go/doc: enable AllMethods flag (and fix logic)

- enable AllMethods flag (default: not set)
- fix logic determining which methods to show
- added respective test case in testdata/e.go for AllMethods = false
- added test case set for AllMethods = true

The critical changes/files to look at are:
- testdata/e{0,1,2}.golden: T4.M should only show up as method of T5 in e2.golden
- reader.go: always include top-level methods, and negate former logic for embedded methods
  (rewrote as a switch for better comprehensability)

Fixes #2791.

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

src/cmd/api/goapi.go

@@ -264,7 +264,7 @@ func (w *Walker) WalkPackage(name string) {
 	// (functions and methods). This is done here because
 	// go/doc is destructive.  We can't use the
 	// *ast.Package after this.
-	dpkg := doc.New(apkg, name, 0)
+	dpkg := doc.New(apkg, name, doc.AllMethods)
 
 	for _, t := range dpkg.Types {
 		// Move funcs up to the top-level, not hiding in the Types.

src/pkg/go/doc/doc.go

@@ -78,7 +78,6 @@ const (
 // 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()

src/pkg/go/doc/doc_test.go

@@ -118,4 +118,5 @@ func test(t *testing.T, mode Mode) {
 func Test(t *testing.T) {
 	test(t, 0)
 	test(t, AllDecls)
+	test(t, AllMethods)
 }

src/pkg/go/doc/exports.go

@@ -141,6 +141,10 @@ func (r *reader) filterDecl(decl ast.Decl) bool {
 		d.Specs = r.filterSpecList(d.Specs)
 		return len(d.Specs) > 0
 	case *ast.FuncDecl:
+		// ok to filter these methods early because any
+		// conflicting method will be filtered here, too -
+		// thus, removing these methods early will not lead
+		// to the false removal of possible conflicts
 		return ast.IsExported(d.Name.Name)
 	}
 	return false

src/pkg/go/doc/reader.go

@@ -708,8 +708,13 @@ 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 && (allMethods || ast.IsExported(removeStar(m.Orig))) {
+		// determine which methods to include
+		switch {
+		case m.Decl == nil:
+			// exclude conflict entry
+		case allMethods, m.Level == 0, !ast.IsExported(removeStar(m.Orig)):
+			// forced inclusion, method not embedded, or method
+			// embedded but original receiver type not exported
 			list[i] = m
 			i++
 		}

src/pkg/go/doc/testdata/a.2.golden

@@ -0,0 +1,13 @@
+// comment 0  comment 1 
+PACKAGE a
+
+IMPORTPATH
+	testdata/a
+
+FILENAMES
+	testdata/a0.go
+	testdata/a1.go
+
+BUGS
+	// bug0 
+	// bug1 

src/pkg/go/doc/testdata/b.2.golden

@@ -0,0 +1,43 @@
+// 
+PACKAGE b
+
+IMPORTPATH
+	testdata/b
+
+IMPORTS
+	a
+
+FILENAMES
+	testdata/b.go
+
+CONSTANTS
+	// 
+	const Pi = 3.14	// Pi
+
+
+VARIABLES
+	// 
+	var MaxInt int	// MaxInt
+
+
+FUNCTIONS
+	// 
+	func F(x int) int
+
+	// Always under the package functions list. 
+	func NotAFactory() int
+
+	// Associated with uint type if AllDecls is set. 
+	func UintFactory() uint
+
+
+TYPES
+	// 
+	type T struct{}	// T
+
+	// 
+	var V T	// v
+
+	// 
+	func (x *T) M()
+

src/pkg/go/doc/testdata/c.2.golden

@@ -0,0 +1,48 @@
+// 
+PACKAGE c
+
+IMPORTPATH
+	testdata/c
+
+IMPORTS
+	a
+
+FILENAMES
+	testdata/c.go
+
+TYPES
+	// A (should see this) 
+	type A struct{}
+
+	// B (should see this) 
+	type B struct{}
+
+	// C (should see this) 
+	type C struct{}
+
+	// D (should see this) 
+	type D struct{}
+
+	// E1 (should see this) 
+	type E1 struct{}
+
+	// E (should see this for E2 and E3) 
+	type E2 struct{}
+
+	// E (should see this for E2 and E3) 
+	type E3 struct{}
+
+	// E4 (should see this) 
+	type E4 struct{}
+
+	// 
+	type T1 struct{}
+
+	// 
+	func (t1 *T1) M()
+
+	// T2 must not show methods of local T1 
+	type T2 struct {
+		a.T1	// not the same as locally declared T1
+	}
+

src/pkg/go/doc/testdata/d.2.golden

@@ -0,0 +1,104 @@
+// 
+PACKAGE d
+
+IMPORTPATH
+	testdata/d
+
+FILENAMES
+	testdata/d1.go
+	testdata/d2.go
+
+CONSTANTS
+	// CBx constants should appear before CAx constants. 
+	const (
+		CB2	= iota	// before CB1
+		CB1		// before CB0
+		CB0		// at end
+	)
+
+	// CAx constants should appear after CBx constants. 
+	const (
+		CA2	= iota	// before CA1
+		CA1		// before CA0
+		CA0		// at end
+	)
+
+	// C0 should be first. 
+	const C0 = 0
+
+	// C1 should be second. 
+	const C1 = 1
+
+	// C2 should be third. 
+	const C2 = 2
+
+	// 
+	const (
+		// Single const declarations inside ()'s are considered ungrouped
+		// and show up in sorted order.
+		Cungrouped = 0
+	)
+
+
+VARIABLES
+	// VBx variables should appear before VAx variables. 
+	var (
+		VB2	int	// before VB1
+		VB1	int	// before VB0
+		VB0	int	// at end
+	)
+
+	// VAx variables should appear after VBx variables. 
+	var (
+		VA2	int	// before VA1
+		VA1	int	// before VA0
+		VA0	int	// at end
+	)
+
+	// V0 should be first. 
+	var V0 uintptr
+
+	// V1 should be second. 
+	var V1 uint
+
+	// V2 should be third. 
+	var V2 int
+
+	// 
+	var (
+		// Single var declarations inside ()'s are considered ungrouped
+		// and show up in sorted order.
+		Vungrouped = 0
+	)
+
+
+FUNCTIONS
+	// F0 should be first. 
+	func F0()
+
+	// F1 should be second. 
+	func F1()
+
+	// F2 should be third. 
+	func F2()
+
+
+TYPES
+	// T0 should be first. 
+	type T0 struct{}
+
+	// T1 should be second. 
+	type T1 struct{}
+
+	// T2 should be third. 
+	type T2 struct{}
+
+	// TG0 should be first. 
+	type TG0 struct{}
+
+	// TG1 should be second. 
+	type TG1 struct{}
+
+	// TG2 should be third. 
+	type TG2 struct{}
+

src/pkg/go/doc/testdata/e.0.golden

@@ -29,9 +29,14 @@ TYPES
 	// T3.M should appear as method of T3. 
 	func (T3) M()
 
-	// T1 has no embedded (level 1) M method due to conflict. 
-	type T4 struct {
-		T2
-		// contains filtered or unexported fields
+	// 
+	type T4 struct{}
+
+	// T4.M should appear as method of T5 only if AllMethods is set. 
+	func (*T4) M()
+
+	// 
+	type T5 struct {
+		T4
 	}
 

src/pkg/go/doc/testdata/e.1.golden

@@ -31,10 +31,15 @@ TYPES
 	// T3.M should appear as method of T3. 
 	func (T3) M()
 
-	// T1 has no embedded (level 1) M method due to conflict. 
-	type T4 struct {
-		t2
-		T2
+	// 
+	type T4 struct{}
+
+	// T4.M should appear as method of T5 only if AllMethods is set. 
+	func (*T4) M()
+
+	// 
+	type T5 struct {
+		T4
 	}
 
 	// 

src/pkg/go/doc/testdata/e.2.golden

@@ -0,0 +1,45 @@
+// The package e is a go/doc test for embedded methods. 
+PACKAGE e
+
+IMPORTPATH
+	testdata/e
+
+FILENAMES
+	testdata/e.go
+
+TYPES
+	// T1 has no embedded (level 1) M method due to conflict. 
+	type T1 struct {
+		// contains filtered or unexported fields
+	}
+
+	// T2 has only M as top-level method. 
+	type T2 struct {
+		// contains filtered or unexported fields
+	}
+
+	// T2.M should appear as method of T2. 
+	func (T2) M()
+
+	// T3 has only M as top-level method. 
+	type T3 struct {
+		// contains filtered or unexported fields
+	}
+
+	// T3.M should appear as method of T3. 
+	func (T3) M()
+
+	// 
+	type T4 struct{}
+
+	// T4.M should appear as method of T5 only if AllMethods is set. 
+	func (*T4) M()
+
+	// 
+	type T5 struct {
+		T4
+	}
+
+	// T4.M should appear as method of T5 only if AllMethods is set. 
+	func (*T5) M()
+

src/pkg/go/doc/testdata/e.go

@@ -64,3 +64,16 @@ type T4 struct {
 	t2
 	T2
 }
+
+// ----------------------------------------------------------------------------
+// Don't show embedded methods of exported anonymous fields unless AllMethods
+// is set.
+
+type T4 struct{}
+
+// T4.M should appear as method of T5 only if AllMethods is set.
+func (*T4) M() {}
+
+type T5 struct {
+	T4
+}