cmd/doc: handle embedded interfaces properly

Changes made:
* Disallow star expression on interfaces as this is not possible.
* Show an embedded "error" in an interface as public similar to
how godoc does it.
* Properly handle selector expressions in both structs and interfaces.
This is possible since a type may refer to something defined in
another package (e.g. io.Reader).

Before:
<<<
$ go doc runtime.Error
type Error interface {

    // RuntimeError is a no-op function but
    // serves to distinguish types that are run time
    // errors from ordinary errors: a type is a
    // run time error if it has a RuntimeError method.
    RuntimeError()
    // Has unexported methods.
}

$ go doc compress/flate Reader
doc: invalid program: unexpected type for embedded field
doc: invalid program: unexpected type for embedded field
type Reader interface {
    io.Reader
    io.ByteReader
}
>>>

After:
<<<
$ go doc runtime.Error
type Error interface {
    error

    // RuntimeError is a no-op function but
    // serves to distinguish types that are run time
    // errors from ordinary errors: a type is a
    // run time error if it has a RuntimeError method.
    RuntimeError()
}

$ go doc compress/flate Reader
type Reader interface {
    io.Reader
    io.ByteReader
}
>>>

Fixes #16567

Change-Id: I272dede971eee9f43173966233eb8810e4a8c907
Reviewed-on: https://go-review.googlesource.com/25365
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/doc/doc_test.go b/src/cmd/doc/doc_test.go
index 5cb1ec990ee3b16a4111cea9bcb9ecefbefa70e4..bfb9099dd2d6eceebbe8e5de80a4a7537b087e3f 100644 (file)
--- a/src/cmd/doc/doc_test.go
+++ b/src/cmd/doc/doc_test.go
@@ -221,6 +221,7 @@ var tests = []test{
                        `func \(ExportedType\) ExportedMethod\(a int\) bool`,
                        `const ExportedTypedConstant ExportedType = iota`, // Must include associated constant.
                        `func ExportedTypeConstructor\(\) \*ExportedType`, // Must include constructor.
+                       `io.Reader.*Comment on line with embedded Reader.`,
                },
                []string{
                        `unexportedField`,                // No unexported field.
@@ -228,6 +229,7 @@ var tests = []test{
                        `Comment about exported method.`, // No comment about exported method.
                        `unexportedMethod`,               // No unexported method.
                        `unexportedTypedConstant`,        // No unexported constant.
+                       `error`,                          // No embedded error.
                },
        },
        // Type -u with unexported fields.
@@ -243,6 +245,8 @@ var tests = []test{
                        `\*ExportedEmbeddedType.*Comment on line with exported embedded \*field.`,
                        `unexportedType.*Comment on line with unexported embedded field.`,
                        `\*unexportedType.*Comment on line with unexported embedded \*field.`,
+                       `io.Reader.*Comment on line with embedded Reader.`,
+                       `error.*Comment on line with embedded error.`,
                        `func \(ExportedType\) unexportedMethod\(a int\) bool`,
                        `unexportedTypedConstant`,
                },
@@ -274,6 +278,8 @@ var tests = []test{
                        `type ExportedInterface interface`, // Interface definition.
                        `Comment before exported method.*\n.*ExportedMethod\(\)` +
                                `.*Comment on line with exported method`,
+                       `io.Reader.*Comment on line with embedded Reader.`,
+                       `error.*Comment on line with embedded error.`,
                        `Has unexported methods`,
                },
                []string{
@@ -293,6 +299,8 @@ var tests = []test{
                        `Comment before exported method.*\n.*ExportedMethod\(\)` +
                                `.*Comment on line with exported method`,
                        `unexportedMethod\(\).*Comment on line with unexported method.`,
+                       `io.Reader.*Comment on line with embedded Reader.`,
+                       `error.*Comment on line with embedded error.`,
                },
                []string{
                        `Has unexported methods`,

diff --git a/src/cmd/doc/pkg.go b/src/cmd/doc/pkg.go
index efd681d5149f92cdfe09977965fbafbff80e157d..eec9f1e803e837d7655c83c3345ba778f2cdf524 100644 (file)
--- a/src/cmd/doc/pkg.go
+++ b/src/cmd/doc/pkg.go
@@ -494,14 +494,19 @@ func trimUnexportedElems(spec *ast.TypeSpec) {
        }
        switch typ := spec.Type.(type) {
        case *ast.StructType:
-               typ.Fields = trimUnexportedFields(typ.Fields, "fields")
+               typ.Fields = trimUnexportedFields(typ.Fields, false)
        case *ast.InterfaceType:
-               typ.Methods = trimUnexportedFields(typ.Methods, "methods")
+               typ.Methods = trimUnexportedFields(typ.Methods, true)
        }
 }
 
 // trimUnexportedFields returns the field list trimmed of unexported fields.
-func trimUnexportedFields(fields *ast.FieldList, what string) *ast.FieldList {
+func trimUnexportedFields(fields *ast.FieldList, isInterface bool) *ast.FieldList {
+       what := "methods"
+       if !isInterface {
+               what = "fields"
+       }
+
        trimmed := false
        list := make([]*ast.Field, 0, len(fields.List))
        for _, field := range fields.List {
@@ -511,12 +516,23 @@ func trimUnexportedFields(fields *ast.FieldList, what string) *ast.FieldList {
                        // Nothing else is allowed.
                        switch ident := field.Type.(type) {
                        case *ast.Ident:
+                               if isInterface && ident.Name == "error" && ident.Obj == nil {
+                                       // For documentation purposes, we consider the builtin error
+                                       // type special when embedded in an interface, such that it
+                                       // always gets shown publicly.
+                                       list = append(list, field)
+                                       continue
+                               }
                                names = []*ast.Ident{ident}
                        case *ast.StarExpr:
                                // Must have the form *identifier.
-                               if ident, ok := ident.X.(*ast.Ident); ok {
+                               // This is only valid on embedded types in structs.
+                               if ident, ok := ident.X.(*ast.Ident); ok && !isInterface {
                                        names = []*ast.Ident{ident}
                                }
+                       case *ast.SelectorExpr:
+                               // An embedded type may refer to a type in another package.
+                               names = []*ast.Ident{ident.Sel}
                        }
                        if names == nil {
                                // Can only happen if AST is incorrect. Safe to continue with a nil list.

diff --git a/src/cmd/doc/testdata/pkg.go b/src/cmd/doc/testdata/pkg.go
index 5f79414b33579361b958ba053367a3c44722a773..9c5cf8f55764c3cc261324dd823766628a35bc18 100644 (file)
--- a/src/cmd/doc/testdata/pkg.go
+++ b/src/cmd/doc/testdata/pkg.go
@@ -66,6 +66,8 @@ type ExportedType struct {
        *ExportedEmbeddedType     // Comment on line with exported embedded *field.
        unexportedType            // Comment on line with unexported embedded field.
        *unexportedType           // Comment on line with unexported embedded *field.
+       io.Reader                 // Comment on line with embedded Reader.
+       error                     // Comment on line with embedded error.
 }
 
 // Comment about exported method.
@@ -96,6 +98,8 @@ type ExportedInterface interface {
        // Comment before exported method.
        ExportedMethod()   // Comment on line with exported method.
        unexportedMethod() // Comment on line with unexported method.
+       io.Reader          // Comment on line with embedded Reader.
+       error              // Comment on line with embedded error.
 }
 
 // Comment about unexported type.