Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HandleFuncs can now be Type Methods as well. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions doc/parse/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,36 @@ func IsFuncInPkg(longFnName string) bool {
return doc != nil
}

// getShortFnName returns the name of the function, given
// longFnName of the form:
// github.com/adams-sarah/test2doc/example.GetWidget
// getShortFnName returns the name of the function
// without the package name so:
// github.com/user/project/package.method
// becomes
// method
// and
// github.com/user/project/package.(*type).method
// becomes
// type.method
func getShortFnName(longFnName string) string {
splitName := strings.Split(longFnName, ".")
return splitName[len(splitName)-1]

// drop anything before the last '/'
slashed := strings.Split(longFnName, "/")
last := slashed[len(slashed)-1]

// split the final part by period
dotted := strings.Split(last, ".")

// drop the first part which is the package name
dotted = dotted[1:]

// loop over and drop pointer references (*v) => v
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment as to why you are dropping the * (so that it matches the funcsMap key)

for i, p := range dotted {
if len(p) > 3 {
if p[0:2] == "(*" && p[len(p)-1] == ')' {
p = p[2 : len(p)-1]
}
}
dotted[i] = p
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be done much clearer with regular expressions i think.

Here's a replacement implementation of this func using regex:

    methodRE := regexp.MustCompile(`/(.*)\.(.*)\.(.*)`)
    funcRE := regexp.MustCompile(`/(.*)\.(.*)`)

    matches := methodRE.FindStringSubmatch(longFnName)
    if len(matches) > 0 {
        fnName := strings.Join(matches[len(matches)-2:], ".")
        fnName = strings.Replace(fnName, "(*", "", -1)
        return strings.Replace(fnName, ")", "", -1)
    }

    matches = funcRE.FindStringSubmatch(v)
    if len(matches) > 0 {
        return matches[len(matches)-1]
    }

    return ""


return strings.Join(dotted, ".")
}
23 changes: 22 additions & 1 deletion doc/parse/package.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package parse

import (
"fmt"
"go/doc"
"go/parser"
"go/token"
Expand All @@ -13,6 +14,7 @@ var (

// go/doc stores package 'Funcs' as a slice
// - we need to look up documentation by func name

funcsMap map[string]*doc.Func
)

Expand All @@ -31,10 +33,29 @@ func NewPackageDoc(dir string) (*doc.Package, error) {
}

func setDocFuncsMap(pkgDoc *doc.Package) {
funcsMap = make(map[string]*doc.Func, len(pkgDoc.Funcs))
typeFuncsMap := getPkgTypesFunctions(pkgDoc)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename this to methodsMap

funcsMap = make(map[string]*doc.Func, len(pkgDoc.Funcs)+len(typeFuncsMap))

for _, fn := range pkgDoc.Funcs {
funcsMap[fn.Name] = fn
}

for k, fn := range typeFuncsMap {
funcsMap[k] = fn
}
}

func getPkgTypesFunctions(pkgDoc *doc.Package) map[string]*doc.Func {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename to getPkgMethods

result := make(map[string]*doc.Func)
for _, t := range pkgDoc.Types {
for _, f := range t.Methods {
if f.Doc != "" {
result[fmt.Sprintf("%s.%s", t.Name, f.Name)] = f
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh i see now why you dropped the*.
For the future, some high-level description of your pull request would have been helpful.

You don't want to only add the function name if the doc string is present, you should always add it.
So remove line 52 :)

}
}
}

return result
}

func getPackageDoc(dir string) (*doc.Package, error) {
Expand Down
3 changes: 1 addition & 2 deletions test/responsewriter.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package test

import (
"github.com/adams-sarah/test2doc/doc/parse"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should go back at the bottom with a newline separating it from std lib imports

"log"
"net/http"
"net/http/httptest"
"runtime"

"github.com/adams-sarah/test2doc/doc/parse"
)

type ResponseWriter struct {
Expand Down