]> Cypherpunks repositories - gostls13.git/commitdiff
go/doc: don't show methods of exported anonymous fields
authorRobert Griesemer <gri@golang.org>
Fri, 27 Jan 2012 22:45:47 +0000 (14:45 -0800)
committerRobert Griesemer <gri@golang.org>
Fri, 27 Jan 2012 22:45:47 +0000 (14:45 -0800)
Added flag AllMethods: if not set (future default), embedded
methods of exported (and thus visible) embedded fields are not
shown in the final package documentation

The actual change for AllMethods is just in sortedFuncs. All
other changes are simplifications of the existing logic (mostly
deletion of code): Because method conflicts due to embedding
must always be detected, remove any premature elimination of
types and methods. Instead collect all named types and all
methods and do the filtering at the end.

Miscellaneous:
- renamed baseType -> namedType
- streamline logic for recording embedded types
- record embedded types via a map (simpler data structures)

AllMethods is set by default; so the output is unchanged and
the tests pass. The next CL will enable the AllMethods flag
and have adjusted tests (and fix issue 2791).

R=rsc
CC=golang-dev
https://golang.org/cl/5572076

src/pkg/go/doc/doc.go
src/pkg/go/doc/exports.go
src/pkg/go/doc/reader.go

index d4aae8ff05c27a9bbc3410aaa4649bd35235f6e3..42f62641371ea58da9af3d467126c33fc54d7897 100644 (file)
@@ -68,12 +68,17 @@ const (
        // extract documentation for all package-level declarations,
        // not just exported ones
        AllDecls Mode = 1 << iota
+
+       // show all embedded methods, not just the ones of
+       // invisible (unexported) anonymous fields
+       AllMethods
 )
 
 // New computes the package documentation for the given package AST.
 // New takes ownership of the AST pkg and may edit or overwrite it.
 //
 func New(pkg *ast.Package, importPath string, mode Mode) *Package {
+       mode |= AllMethods // TODO(gri) remove this to enable flag
        var r reader
        r.readPackage(pkg, mode)
        r.computeMethodSets()
@@ -86,8 +91,8 @@ func New(pkg *ast.Package, importPath string, mode Mode) *Package {
                Filenames:  r.filenames,
                Bugs:       r.bugs,
                Consts:     sortedValues(r.values, token.CONST),
-               Types:      sortedTypes(r.types),
+               Types:      sortedTypes(r.types, mode&AllMethods != 0),
                Vars:       sortedValues(r.values, token.VAR),
-               Funcs:      sortedFuncs(r.funcs),
+               Funcs:      sortedFuncs(r.funcs, true),
        }
 }
index e6f58ccb3507c9afe6cb5498abb9e5a6f971bbb8..95de2baf4dedc9e465d5d020896d8ab194161f05 100644 (file)
@@ -23,11 +23,11 @@ func filterIdentList(list []*ast.Ident) []*ast.Ident {
 }
 
 // filterFieldList removes unexported fields (field names) from the field list
-// in place and returns true if fields were removed. Removed fields that are
-// anonymous (embedded) fields are added as embedded types to base. filterType
-// is called with the types of all remaining fields.
+// in place and returns true if fields were removed. Anonymous fields are
+// recorded with the parent type. filterType is called with the types of
+// all remaining fields.
 //
-func (r *reader) filterFieldList(base *baseType, fields *ast.FieldList) (removedFields bool) {
+func (r *reader) filterFieldList(parent *namedType, fields *ast.FieldList) (removedFields bool) {
        if fields == nil {
                return
        }
@@ -37,18 +37,9 @@ func (r *reader) filterFieldList(base *baseType, fields *ast.FieldList) (removed
                keepField := false
                if n := len(field.Names); n == 0 {
                        // anonymous field
-                       name, imp := baseTypeName(field.Type)
+                       name := r.recordAnonymousField(parent, field.Type)
                        if ast.IsExported(name) {
-                               // we keep the field - in this case r.readDecl
-                               // will take care of adding the embedded type
                                keepField = true
-                       } else if base != nil && !imp {
-                               // we don't keep the field - add it as an embedded
-                               // type so we won't loose its methods, if any
-                               if embedded := r.lookupType(name); embedded != nil {
-                                       _, ptr := field.Type.(*ast.StarExpr)
-                                       base.addEmbeddedType(embedded, ptr)
-                               }
                        }
                } else {
                        field.Names = filterIdentList(field.Names)
@@ -86,7 +77,7 @@ func (r *reader) filterParamList(fields *ast.FieldList) {
 // in place. If fields (or methods) have been removed, the corresponding
 // struct or interface type has the Incomplete field set to true. 
 //
-func (r *reader) filterType(base *baseType, typ ast.Expr) {
+func (r *reader) filterType(parent *namedType, typ ast.Expr) {
        switch t := typ.(type) {
        case *ast.Ident:
                // nothing to do
@@ -95,14 +86,14 @@ func (r *reader) filterType(base *baseType, typ ast.Expr) {
        case *ast.ArrayType:
                r.filterType(nil, t.Elt)
        case *ast.StructType:
-               if r.filterFieldList(base, t.Fields) {
+               if r.filterFieldList(parent, t.Fields) {
                        t.Incomplete = true
                }
        case *ast.FuncType:
                r.filterParamList(t.Params)
                r.filterParamList(t.Results)
        case *ast.InterfaceType:
-               if r.filterFieldList(base, t.Methods) {
+               if r.filterFieldList(parent, t.Methods) {
                        t.Incomplete = true
                }
        case *ast.MapType:
index 9c6f0816b60f81e864accc1ea477e559d63a9ffd..c6dd351993aef3edf0911ddffb3527383d40b7b6 100644 (file)
@@ -89,7 +89,7 @@ func (mset methodSet) add(m *Func) {
 }
 
 // ----------------------------------------------------------------------------
-// Base types
+// Named types
 
 // baseTypeName returns the name of the base type of x (or "")
 // and whether the type is imported or not.
@@ -110,31 +110,23 @@ func baseTypeName(x ast.Expr) (name string, imported bool) {
        return
 }
 
-// embeddedType describes the type of an anonymous field.
+// A namedType represents a named unqualified (package local, or possibly
+// predeclared) type. The namedType for a type name is always found via
+// reader.lookupType.
 //
-type embeddedType struct {
-       typ *baseType // the corresponding base type
-       ptr bool      // if set, the anonymous field type is a pointer
-}
-
-type baseType struct {
+type namedType struct {
        doc  string       // doc comment for type
-       name string       // local type name (excluding package qualifier)
+       name string       // type name
        decl *ast.GenDecl // nil if declaration hasn't been seen yet
 
+       isEmbedded bool                // true if this type is embedded
+       isStruct   bool                // true if this type is a struct
+       embedded   map[*namedType]bool // true if the embedded type is a pointer
+
        // associated declarations
        values  []*Value // consts and vars
        funcs   methodSet
        methods methodSet
-
-       isEmbedded bool           // true if this type is embedded
-       isStruct   bool           // true if this type is a struct
-       embedded   []embeddedType // list of embedded types
-}
-
-func (typ *baseType) addEmbeddedType(e *baseType, isPtr bool) {
-       e.isEmbedded = true
-       typ.embedded = append(typ.embedded, embeddedType{e, isPtr})
 }
 
 // ----------------------------------------------------------------------------
@@ -158,22 +150,16 @@ type reader struct {
        // declarations
        imports map[string]int
        values  []*Value // consts and vars
-       types   map[string]*baseType
+       types   map[string]*namedType
        funcs   methodSet
 }
 
-// isVisible reports whether name is visible in the documentation.
-//
-func (r *reader) isVisible(name string) bool {
-       return r.mode&AllDecls != 0 || ast.IsExported(name)
-}
-
 // lookupType returns the base type with the given name.
 // If the base type has not been encountered yet, a new
 // type with the given name but no associated declaration
 // is added to the type map.
 //
-func (r *reader) lookupType(name string) *baseType {
+func (r *reader) lookupType(name string) *namedType {
        if name == "" || name == "_" {
                return nil // no type docs for anonymous types
        }
@@ -181,15 +167,34 @@ func (r *reader) lookupType(name string) *baseType {
                return typ
        }
        // type not found - add one without declaration
-       typ := &baseType{
-               name:    name,
-               funcs:   make(methodSet),
-               methods: make(methodSet),
+       typ := &namedType{
+               name:     name,
+               embedded: make(map[*namedType]bool),
+               funcs:    make(methodSet),
+               methods:  make(methodSet),
        }
        r.types[name] = typ
        return typ
 }
 
+// recordAnonymousField registers fieldType as the type of an
+// anonymous field in the parent type. If the field is imported
+// (qualified name) or the parent is nil, the field is ignored.
+// The function returns the field name.
+//
+func (r *reader) recordAnonymousField(parent *namedType, fieldType ast.Expr) (fname string) {
+       fname, imp := baseTypeName(fieldType)
+       if parent == nil || imp {
+               return
+       }
+       if ftype := r.lookupType(fname); ftype != nil {
+               ftype.isEmbedded = true
+               _, ptr := fieldType.(*ast.StarExpr)
+               parent.embedded[ftype] = ptr
+       }
+       return
+}
+
 func (r *reader) readDoc(comment *ast.CommentGroup) {
        // By convention there should be only one package comment
        // but collect all of them if there are more then one.
@@ -232,7 +237,7 @@ func (r *reader) readValue(decl *ast.GenDecl) {
                switch {
                case s.Type != nil:
                        // a type is present; determine its name
-                       if n, imp := baseTypeName(s.Type); !imp && r.isVisible(n) {
+                       if n, imp := baseTypeName(s.Type); !imp {
                                name = n
                        }
                case decl.Tok == token.CONST:
@@ -267,8 +272,7 @@ func (r *reader) readValue(decl *ast.GenDecl) {
        const threshold = 0.75
        if domName != "" && domFreq >= int(float64(len(decl.Specs))*threshold) {
                // typed entries are sufficiently frequent
-               typ := r.lookupType(domName)
-               if typ != nil {
+               if typ := r.lookupType(domName); typ != nil {
                        values = &typ.values // associate with that type
                }
        }
@@ -321,22 +325,14 @@ func (r *reader) readType(decl *ast.GenDecl, spec *ast.TypeSpec) {
        decl.Doc = nil // doc consumed - remove from AST
        typ.doc = doc.Text()
 
-       // look for anonymous fields that might contribute methods
+       // record anonymous fields (they may contribute methods)
+       // (some fields may have been recorded already when filtering
+       // exports, but that's ok)
        var list []*ast.Field
        list, typ.isStruct = fields(spec.Type)
        for _, field := range list {
                if len(field.Names) == 0 {
-                       // anonymous field - add corresponding field type to typ
-                       n, imp := baseTypeName(field.Type)
-                       if imp {
-                               // imported type - we don't handle this case
-                               // at the moment
-                               return
-                       }
-                       if embedded := r.lookupType(n); embedded != nil {
-                               _, ptr := field.Type.(*ast.StarExpr)
-                               typ.addEmbeddedType(embedded, ptr)
-                       }
+                       r.recordAnonymousField(typ, field.Type)
                }
        }
 }
@@ -356,24 +352,10 @@ func (r *reader) readFunc(fun *ast.FuncDecl) {
                        // don't show this method
                        return
                }
-               var typ *baseType
-               if r.isVisible(recvTypeName) {
-                       // visible recv type: if not found, add it to r.types
-                       typ = r.lookupType(recvTypeName)
-               } else {
-                       // invisible recv type: if not found, do not add it
-                       // (invisible embedded types are added before this
-                       // phase, so if the type doesn't exist yet, we don't
-                       // care about this method)
-                       typ = r.types[recvTypeName]
-               }
-               if typ != nil {
-                       // associate method with the type
-                       // (if the type is not exported, it may be embedded
-                       // somewhere so we need to collect the method anyway)
+               if typ := r.lookupType(recvTypeName); typ != nil {
                        typ.methods.set(fun)
                }
-               // otherwise don't show the method
+               // otherwise ignore the method
                // TODO(gri): There may be exported methods of non-exported types
                // that can be called because of exported values (consts, vars, or
                // function results) of that type. Could determine if that is the
@@ -389,7 +371,7 @@ func (r *reader) readFunc(fun *ast.FuncDecl) {
                        // exactly one (named or anonymous) result associated
                        // with the first type in result signature (there may
                        // be more than one result)
-                       if n, imp := baseTypeName(res.Type); !imp && r.isVisible(n) {
+                       if n, imp := baseTypeName(res.Type); !imp {
                                if typ := r.lookupType(n); typ != nil {
                                        // associate Func with typ
                                        typ.funcs.set(fun)
@@ -476,7 +458,7 @@ func (r *reader) readPackage(pkg *ast.Package, mode Mode) {
        r.filenames = make([]string, len(pkg.Files))
        r.imports = make(map[string]int)
        r.mode = mode
-       r.types = make(map[string]*baseType)
+       r.types = make(map[string]*namedType)
        r.funcs = make(methodSet)
 
        // sort package files before reading them so that the
@@ -554,24 +536,23 @@ func customizeRecv(f *Func, recvTypeName string, embeddedIsPtr bool, level int)
        return &newF
 }
 
-// collectEmbeddedMethods collects the embedded methods from
-// all processed embedded types found in info in mset.
+// collectEmbeddedMethods collects the embedded methods of typ in mset.
 //
-func collectEmbeddedMethods(mset methodSet, typ *baseType, recvTypeName string, embeddedIsPtr bool, level int) {
-       for _, e := range typ.embedded {
+func (r *reader) collectEmbeddedMethods(mset methodSet, typ *namedType, recvTypeName string, embeddedIsPtr bool, level int) {
+       for embedded, isPtr := range typ.embedded {
                // Once an embedded type is embedded as a pointer type
                // all embedded types in those types are treated like
                // pointer types for the purpose of the receiver type
                // computation; i.e., embeddedIsPtr is sticky for this
                // embedding hierarchy.
-               thisEmbeddedIsPtr := embeddedIsPtr || e.ptr
-               for _, m := range e.typ.methods {
+               thisEmbeddedIsPtr := embeddedIsPtr || isPtr
+               for _, m := range embedded.methods {
                        // only top-level methods are embedded
                        if m.Level == 0 {
                                mset.add(customizeRecv(m, recvTypeName, thisEmbeddedIsPtr, level))
                        }
                }
-               collectEmbeddedMethods(mset, e.typ, recvTypeName, thisEmbeddedIsPtr, level+1)
+               r.collectEmbeddedMethods(mset, embedded, recvTypeName, thisEmbeddedIsPtr, level+1)
        }
 }
 
@@ -582,7 +563,7 @@ func (r *reader) computeMethodSets() {
                // collect embedded methods for t
                if t.isStruct {
                        // struct
-                       collectEmbeddedMethods(t.methods, t, t.name, false, 1)
+                       r.collectEmbeddedMethods(t.methods, t, t.name, false, 1)
                } else {
                        // interface
                        // TODO(gri) fix this
@@ -597,7 +578,7 @@ func (r *reader) computeMethodSets() {
 // 
 func (r *reader) cleanupTypes() {
        for _, t := range r.types {
-               visible := r.isVisible(t.name)
+               visible := r.mode&AllDecls != 0 || ast.IsExported(t.name)
                if t.decl == nil && (predeclaredTypes[t.name] || t.isEmbedded && visible) {
                        // t.name is a predeclared type (and was not redeclared in this package),
                        // or it was embedded somewhere but its declaration is missing (because
@@ -607,6 +588,8 @@ func (r *reader) cleanupTypes() {
                        r.values = append(r.values, t.values...)
                        // 2) move factory functions
                        for name, f := range t.funcs {
+                               // in a correct AST, package-level function names
+                               // are all different - no need to check for conflicts
                                r.funcs[name] = f
                        }
                        // 3) move methods
@@ -689,7 +672,7 @@ func sortedValues(m []*Value, tok token.Token) []*Value {
        return list
 }
 
-func sortedTypes(m map[string]*baseType) []*Type {
+func sortedTypes(m map[string]*namedType, allMethods bool) []*Type {
        list := make([]*Type, len(m))
        i := 0
        for _, t := range m {
@@ -699,8 +682,8 @@ func sortedTypes(m map[string]*baseType) []*Type {
                        Decl:    t.decl,
                        Consts:  sortedValues(t.values, token.CONST),
                        Vars:    sortedValues(t.values, token.VAR),
-                       Funcs:   sortedFuncs(t.funcs),
-                       Methods: sortedFuncs(t.methods),
+                       Funcs:   sortedFuncs(t.funcs, true),
+                       Methods: sortedFuncs(t.methods, allMethods),
                }
                i++
        }
@@ -714,12 +697,19 @@ func sortedTypes(m map[string]*baseType) []*Type {
        return list
 }
 
-func sortedFuncs(m methodSet) []*Func {
+func removeStar(s string) string {
+       if len(s) > 0 && s[0] == '*' {
+               return s[1:]
+       }
+       return s
+}
+
+func sortedFuncs(m methodSet, allMethods bool) []*Func {
        list := make([]*Func, len(m))
        i := 0
        for _, m := range m {
                // exclude conflict entries
-               if m.Decl != nil {
+               if m.Decl != nil && (allMethods || ast.IsExported(removeStar(m.Orig))) {
                        list[i] = m
                        i++
                }