internal/frontend: signal doc: fix signature strings Fix a reversion in which signature strings included param names again. For golang/go#80385. Change-Id: Ia19d3fc7ff81f9917e74d83c330d9fb13a190c5b Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/808663 kokoro-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Ethan Lee <ethanalee@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Auto-Submit: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/internal/frontend/evals.go b/internal/frontend/evals.go index 6ea8cf9..d4987d0 100644 --- a/internal/frontend/evals.go +++ b/internal/frontend/evals.go
@@ -198,7 +198,7 @@ // collectInterfaceMethods walks files looking for top-level exported interface // declarations. For every exported method in those interfaces (including methods // embedded from other interfaces) that has documentation, it records a mapping -// from the method's name to its signature (e.g., "func() string"). +// from the method's name to its signature (e.g., "() string"). // This map is formatted identically to conventionalMethods. func collectInterfaceMethods(files []*ast.File) map[interfaceMethod]bool { // First pass: index all top-level interface declarations in the package by their @@ -257,7 +257,7 @@ // There should only be one name, but just to play it safe, loop. for _, name := range field.Names { if name.IsExported() { - methods[interfaceMethod{name.Name, nodeString(ftype)}] = true + methods[interfaceMethod{name.Name, sigString(ftype)}] = true } } } @@ -322,7 +322,7 @@ if !ast.IsExported(recvName) { continue } - m := interfaceMethod{name: name, signature: nodeString(d.Type)} + m := interfaceMethod{name: name, signature: sigString(d.Type)} if conventionalMethods[m] || ifaceMethods[m] { continue } @@ -397,24 +397,17 @@ // conventionalMethods maps common, standard method names like String and Error // to their signatures. These methods are often undocumented, and that's fine. var conventionalMethods = map[interfaceMethod]bool{ - {"String", "func() string"}: true, // fmt.Stringer - {"Error", "func() string"}: true, // error - {"Unwrap", "func() error"}: true, // for errors.Unwrap - {"Len", "func() int"}: true, // sort.Interface - {"Less", "func(int, int) bool"}: true, // sort.Interface - {"Swap", "func(int, int)"}: true, // sort.Interface - {"Read", "func([]byte) (int, error)"}: true, // io.Reader - {"Close", "func() error"}: true, // io.ReadCloser - {"Write", "func([]byte) (int, error)"}: true, // io.Writer - {"MarshalJSON", "func() ([]byte, error)"}: true, // json.Marshaler - {"UnmarshalJSON", "func([]byte) error"}: true, // json.Unmarshaler -} - -// nodeString returns a string for node. -func nodeString(node ast.Node) string { - var buf bytes.Buffer - printer.Fprint(&buf, token.NewFileSet(), node) - return buf.String() + {"String", "() string"}: true, // fmt.Stringer + {"Error", "() string"}: true, // error + {"Unwrap", "() error"}: true, // for errors.Unwrap + {"Len", "() int"}: true, // sort.Interface + {"Less", "(int, int) bool"}: true, // sort.Interface + {"Swap", "(int, int)"}: true, // sort.Interface + {"Read", "([]byte) (int, error)"}: true, // io.Reader + {"Close", "() error"}: true, // io.ReadCloser + {"Write", "([]byte) (int, error)"}: true, // io.Writer + {"MarshalJSON", "() ([]byte, error)"}: true, // json.Marshaler + {"UnmarshalJSON", "([]byte) error"}: true, // json.Unmarshaler } // sigString returns a string representation of a function signature @@ -459,3 +452,10 @@ } return types } + +// nodeString returns a string for node. +func nodeString(node ast.Node) string { + var buf bytes.Buffer + printer.Fprint(&buf, token.NewFileSet(), node) + return buf.String() +}
diff --git a/internal/frontend/evals_test.go b/internal/frontend/evals_test.go index 2cf4f54..f391e11 100644 --- a/internal/frontend/evals_test.go +++ b/internal/frontend/evals_test.go
@@ -409,6 +409,25 @@ } } +func TestCollectInterfaceMethods(t *testing.T) { + pkg := parseTestPackage(t, readTxtar(t, "collect_interface_methods")) + var files []*ast.File + for _, f := range pkg.Files { + files = append(files, f.AST) + } + + got := collectInterfaceMethods(files) + want := map[interfaceMethod]bool{ + {"DocumentedMethod", "(int) string"}: true, + {"UndocumentedMethod", "() error"}: true, + {"MethodInUnexportedInterface2", "(int) bool"}: true, + } + + if diff := cmp.Diff(want, got, cmp.AllowUnexported(interfaceMethod{})); diff != "" { + t.Errorf("collectInterfaceMethods() mismatch (-want +got):\n%s", diff) + } +} + // readTxtar reads a txtar file into a map from internal txtar filename to contents. // It assumes the file lives in testdata and has a ".txtar" extension. func readTxtar(t *testing.T, txtarName string) map[string]string { @@ -452,22 +471,3 @@ } return decodedPkg } - -func TestCollectInterfaceMethods(t *testing.T) { - pkg := parseTestPackage(t, readTxtar(t, "collect_interface_methods")) - var files []*ast.File - for _, f := range pkg.Files { - files = append(files, f.AST) - } - - got := collectInterfaceMethods(files) - want := map[interfaceMethod]bool{ - {"DocumentedMethod", "func(x int) string"}: true, - {"UndocumentedMethod", "func() error"}: true, - {"MethodInUnexportedInterface2", "func(int) bool"}: true, - } - - if diff := cmp.Diff(want, got, cmp.AllowUnexported(interfaceMethod{})); diff != "" { - t.Errorf("collectInterfaceMethods() mismatch (-want +got):\n%s", diff) - } -}
diff --git a/internal/frontend/testdata/mixed_doc_and_undoc.txtar b/internal/frontend/testdata/mixed_doc_and_undoc.txtar index 6d3b984..061f849 100644 --- a/internal/frontend/testdata/mixed_doc_and_undoc.txtar +++ b/internal/frontend/testdata/mixed_doc_and_undoc.txtar
@@ -39,6 +39,8 @@ func (Config) Len() int { return 0 } +func (Config) UnmarshalJSON(data []byte) error { return nil } + // This one has a conventional name but a different signature // (the "Less" on sort.Interface returns a bool) so it does // count--it should have doc but doesn't.