cmd/doc: handle embedded fields properly

The structure of the code meant that an embedded field was never
checked for export status. We need to check the name of the type,
which is either of type T or type *T, and T might be unexported.

Fixes #14356.

Change-Id: I56f468e9b8ae67e9ed7509ed0b91d860507baed2
Reviewed-on: https://go-review.googlesource.com/19701
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/cmd/doc/doc_test.go b/src/cmd/doc/doc_test.go
index ed1d0e7c79044e62a847816f461382f1790f4712..20b61702b43f8befbfca02a9183a984897757fff 100644 (file)
--- a/src/cmd/doc/doc_test.go
+++ b/src/cmd/doc/doc_test.go
@@ -221,6 +221,7 @@ var tests = []test{
                        `type ExportedType struct`,    // Type definition.
                        `Comment before exported field.*\n.*ExportedField +int` +
                                `.*Comment on line with exported field.`,
+                       `ExportedEmbeddedType.*Comment on line with exported embedded field.`,
                        `Has unexported fields`,
                        `func \(ExportedType\) ExportedMethod\(a int\) bool`,
                        `const ExportedTypedConstant ExportedType = iota`, // Must include associated constant.
@@ -228,6 +229,7 @@ var tests = []test{
                },
                []string{
                        `unexportedField`,                // No unexported field.
+                       `int.*embedded`,                  // No unexported embedded field.
                        `Comment about exported method.`, // No comment about exported method.
                        `unexportedMethod`,               // No unexported method.
                        `unexportedTypedConstant`,        // No unexported constant.
@@ -241,7 +243,11 @@ var tests = []test{
                        `Comment about exported type`, // Include comment.
                        `type ExportedType struct`,    // Type definition.
                        `Comment before exported field.*\n.*ExportedField +int`,
-                       `unexportedField int.*Comment on line with unexported field.`,
+                       `unexportedField.*int.*Comment on line with unexported field.`,
+                       `ExportedEmbeddedType.*Comment on line with exported embedded field.`,
+                       `\*ExportedEmbeddedType.*Comment on line with exported embedded \*field.`,
+                       `unexportedType.*Comment on line with unexported embedded field.`,
+                       `\*unexportedType.*Comment on line with unexported embedded \*field.`,
                        `func \(ExportedType\) unexportedMethod\(a int\) bool`,
                        `unexportedTypedConstant`,
                },
@@ -448,7 +454,6 @@ var trimTests = []trimTest{
        {"", "", "", true},
        {"/usr/gopher", "/usr/gopher", "/usr/gopher", true},
        {"/usr/gopher/bar", "/usr/gopher", "bar", true},
-       {"/usr/gopher", "/usr/gopher", "/usr/gopher", true},
        {"/usr/gopherflakes", "/usr/gopher", "/usr/gopherflakes", false},
        {"/usr/gopher/bar", "/usr/zot", "/usr/gopher/bar", false},
 }

diff --git a/src/cmd/doc/pkg.go b/src/cmd/doc/pkg.go
index 0b07f7cc7c6429e2260223735e6bc51242347352..a14ccdb59b549f73c0108b6a87671bc5b5c1e0f0 100644 (file)
--- a/src/cmd/doc/pkg.go
+++ b/src/cmd/doc/pkg.go
@@ -487,9 +487,27 @@ func trimUnexportedFields(fields *ast.FieldList, what string) *ast.FieldList {
        trimmed := false
        list := make([]*ast.Field, 0, len(fields.List))
        for _, field := range fields.List {
+               names := field.Names
+               if len(names) == 0 {
+                       // Embedded type. Use the name of the type. It must be of type ident or *ident.
+                       // Nothing else is allowed.
+                       switch ident := field.Type.(type) {
+                       case *ast.Ident:
+                               names = []*ast.Ident{ident}
+                       case *ast.StarExpr:
+                               // Must have the form *identifier.
+                               if ident, ok := ident.X.(*ast.Ident); ok {
+                                       names = []*ast.Ident{ident}
+                               }
+                       }
+                       if names == nil {
+                               // Can only happen if AST is incorrect. Safe to continue with a nil list.
+                               log.Print("invalid program: unexpected type for embedded field")
+                       }
+               }
                // Trims if any is unexported. Good enough in practice.
                ok := true
-               for _, name := range field.Names {
+               for _, name := range names {
                        if !isExported(name.Name) {
                                trimmed = true
                                ok = false

diff --git a/src/cmd/doc/testdata/pkg.go b/src/cmd/doc/testdata/pkg.go
index 3e7acee50b8719cae5b1d292899b1bbb841235e9..1a673f78d42d01b670ffece564294607f72be224 100644 (file)
--- a/src/cmd/doc/testdata/pkg.go
+++ b/src/cmd/doc/testdata/pkg.go
@@ -60,8 +60,12 @@ func internalFunc(a int) bool
 // Comment about exported type.
 type ExportedType struct {
        // Comment before exported field.
-       ExportedField   int // Comment on line with exported field.
-       unexportedField int // Comment on line with unexported field.
+       ExportedField         int // Comment on line with exported field.
+       unexportedField       int // Comment on line with unexported field.
+       ExportedEmbeddedType      // Comment on line with exported embedded field.
+       *ExportedEmbeddedType     // Comment on line with exported embedded *field.
+       unexportedType            // Comment on line with unexported embedded field.
+       *unexportedType           // Comment on line with unexported embedded *field.
 }
 
 // Comment about exported method.