cmd/doc: ensure summaries truly are only one line

The documentation for doc says:
> Doc prints the documentation comments associated with the item identified by its
> arguments (a package, const, func, type, var, or method) followed by a one-line
> summary of each of the first-level items "under" that item (package-level
> declarations for a package, methods for a type, etc.).

Certain variables (and constants, functions, and types) have value specifications
that are multiple lines long. Prior to this change, doc would print out all of the
lines necessary to display the value. This is inconsistent with the documented
behavior, which guarantees a one-line summary for all first-level items.
We fix this here by writing a general oneLineNode method that always returns
a one-line summary (guaranteed!) of any input node.

Packages like image/color/palette and unicode now become much
more readable since large slices are now a single line.

$ go doc image/color/palette
<<<
// Before:
var Plan9 = []color.Color{
	color.RGBA{0x00, 0x00, 0x00, 0xff},
	color.RGBA{0x00, 0x00, 0x44, 0xff},
	color.RGBA{0x00, 0x00, 0x88, 0xff},
	... // Hundreds of more lines!
}
var WebSafe = []color.Color{
	color.RGBA{0x00, 0x00, 0x00, 0xff},
	color.RGBA{0x00, 0x00, 0x33, 0xff},
	color.RGBA{0x00, 0x00, 0x66, 0xff},
	... // Hundreds of more lines!
}

// After:
var Plan9 = []color.Color{ ... }
var WebSafe = []color.Color{ ... }
>>>

In order to test this, I ran `go doc` and `go doc -u` on all of the
standard library packages and diff'd the output with and without the
change to ensure that all differences were intended.

Fixes #13072

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

src/cmd/doc/doc_test.go

@@ -55,19 +55,22 @@ var tests = []test{
 		[]string{p},
 		[]string{
 			`Package comment`,
-			`const ExportedConstant = 1`,                            // Simple constant.
+			`const ExportedConstant = 1`,                                   // Simple constant.
-			`const ConstOne = 1`,                                    // First entry in constant block.
+			`const ConstOne = 1`,                                           // First entry in constant block.
-			`const ConstFive ...`,                                   // From block starting with unexported constant.
+			`const ConstFive ...`,                                          // From block starting with unexported constant.
-			`var ExportedVariable = 1`,                              // Simple variable.
+			`var ExportedVariable = 1`,                                     // Simple variable.
-			`var VarOne = 1`,                                        // First entry in variable block.
+			`var VarOne = 1`,                                               // First entry in variable block.
-			`func ExportedFunc\(a int\) bool`,                       // Function.
+			`func ExportedFunc\(a int\) bool`,                              // Function.
-			`func ReturnUnexported\(\) unexportedType`,              // Function with unexported return type.
+			`func ReturnUnexported\(\) unexportedType`,                     // Function with unexported return type.
-			`type ExportedType struct { ... }`,                      // Exported type.
+			`type ExportedType struct{ ... }`,                              // Exported type.
-			`const ExportedTypedConstant ExportedType = iota`,       // Typed constant.
+			`const ExportedTypedConstant ExportedType = iota`,              // Typed constant.
-			`const ExportedTypedConstant_unexported unexportedType`, // Typed constant, exported for unexported type.
+			`const ExportedTypedConstant_unexported unexportedType`,        // Typed constant, exported for unexported type.
-			`const ConstLeft2 uint64 ...`,                           // Typed constant using unexported iota.
+			`const ConstLeft2 uint64 ...`,                                  // Typed constant using unexported iota.
-			`const ConstGroup1 unexportedType = iota ...`,           // Typed constant using unexported type.
+			`const ConstGroup1 unexportedType = iota ...`,                  // Typed constant using unexported type.
-			`const ConstGroup4 ExportedType = ExportedType{}`,       // Typed constant using exported type.
+			`const ConstGroup4 ExportedType = ExportedType{}`,              // Typed constant using exported type.
+			`const MultiLineConst = ...`,                                   // Multi line constant.
+			`var MultiLineVar = map\[struct{ ... }\]struct{ ... }{ ... }`,  // Multi line variable.
+			`func MultiLineFunc\(x interface{ ... }\) \(r struct{ ... }\)`, // Multi line function.
 		},
 		[]string{
 			`const internalConstant = 2`,        // No internal constants.
@@ -102,6 +105,7 @@ var tests = []test{
 			`Comment about exported constant`,  // No comment for simple constant.
 			`Comment about block of constants`, // No comment for constant block.
 			`Comment about internal function`,  // No comment for internal function.
+			`MultiLine(String|Method|Field)`,   // No data from multi line portions.
 		},
 	},
 

src/cmd/doc/pkg.go

@@ -194,68 +194,178 @@ func (pkg *Package) emit(comment string, node ast.Node) {
 	}
 }
 
-var formatBuf bytes.Buffer // Reusable to avoid allocation.
+// oneLineNode returns a one-line summary of the given input node.
-
+func (pkg *Package) oneLineNode(node ast.Node) string {
-// formatNode is a helper function for printing.
+	const maxDepth = 10
-func (pkg *Package) formatNode(node ast.Node) []byte {
+	return pkg.oneLineNodeDepth(node, maxDepth)
-	formatBuf.Reset()
-	format.Node(&formatBuf, pkg.fs, node)
-	return formatBuf.Bytes()
 }
 
-// oneLineFunc prints a function declaration as a single line.
+// oneLineNodeDepth returns a one-line summary of the given input node.
-func (pkg *Package) oneLineFunc(decl *ast.FuncDecl) {
+// The depth specifies the maximum depth when traversing the AST.
-	decl.Doc = nil
+func (pkg *Package) oneLineNodeDepth(node ast.Node, depth int) string {
-	decl.Body = nil
+	const dotDotDot = "..."
-	pkg.emit("", decl)
+	if depth == 0 {
-}
+		return dotDotDot
-
-// oneLineValueGenDecl prints a var or const declaration as a single line.
-func (pkg *Package) oneLineValueGenDecl(prefix string, decl *ast.GenDecl) {
-	decl.Doc = nil
-	dotDotDot := ""
-	if len(decl.Specs) > 1 {
-		dotDotDot = " ..."
 	}
-	// Find the first relevant spec.
+	depth--
-	typ := ""
-	for i, spec := range decl.Specs {
-		valueSpec := spec.(*ast.ValueSpec) // Must succeed; we can't mix types in one genDecl.
 
-		// The type name may carry over from a previous specification in the
+	switch n := node.(type) {
-		// case of constants and iota.
+	case nil:
-		if valueSpec.Type != nil {
+		return ""
-			typ = fmt.Sprintf(" %s", pkg.formatNode(valueSpec.Type))
+
-		} else if len(valueSpec.Values) > 0 {
+	case *ast.GenDecl:
-			typ = ""
+		// Formats const and var declarations.
+		trailer := ""
+		if len(n.Specs) > 1 {
+			trailer = " " + dotDotDot
 		}
 
-		if !isExported(valueSpec.Names[0].Name) {
+		// Find the first relevant spec.
-			continue
+		typ := ""
-		}
+		for i, spec := range n.Specs {
-		val := ""
+			valueSpec := spec.(*ast.ValueSpec) // Must succeed; we can't mix types in one GenDecl.
-		if i < len(valueSpec.Values) && valueSpec.Values[i] != nil {
+
-			val = fmt.Sprintf(" = %s", pkg.formatNode(valueSpec.Values[i]))
+			// The type name may carry over from a previous specification in the
-		}
+			// case of constants and iota.
-		pkg.Printf("%s%s %s%s%s%s\n", prefix, decl.Tok, valueSpec.Names[0], typ, val, dotDotDot)
+			if valueSpec.Type != nil {
-		break
+				typ = fmt.Sprintf(" %s", pkg.oneLineNodeDepth(valueSpec.Type, depth))
-	}
+			} else if len(valueSpec.Values) > 0 {
-}
+				typ = ""
+			}
+
+			if !isExported(valueSpec.Names[0].Name) {
+				continue
+			}
+			val := ""
+			if i < len(valueSpec.Values) && valueSpec.Values[i] != nil {
+				val = fmt.Sprintf(" = %s", pkg.oneLineNodeDepth(valueSpec.Values[i], depth))
+			}
+			return fmt.Sprintf("%s %s%s%s%s", n.Tok, valueSpec.Names[0], typ, val, trailer)
+		}
+		return ""
+
+	case *ast.FuncDecl:
+		// Formats func declarations.
+		name := n.Name.Name
+		recv := pkg.oneLineNodeDepth(n.Recv, depth)
+		if len(recv) > 0 {
+			recv = "(" + recv + ") "
+		}
+		fnc := pkg.oneLineNodeDepth(n.Type, depth)
+		if strings.Index(fnc, "func") == 0 {
+			fnc = fnc[4:]
+		}
+		return fmt.Sprintf("func %s%s%s", recv, name, fnc)
+
+	case *ast.TypeSpec:
+		return fmt.Sprintf("type %s %s", n.Name.Name, pkg.oneLineNodeDepth(n.Type, depth))
+
+	case *ast.FuncType:
+		var params []string
+		if n.Params != nil {
+			for _, field := range n.Params.List {
+				params = append(params, pkg.oneLineField(field, depth))
+			}
+		}
+		needParens := false
+		var results []string
+		if n.Results != nil {
+			needParens = needParens || len(n.Results.List) > 1
+			for _, field := range n.Results.List {
+				needParens = needParens || len(field.Names) > 0
+				results = append(results, pkg.oneLineField(field, depth))
+			}
+		}
+
+		param := strings.Join(params, ", ")
+		if len(results) == 0 {
+			return fmt.Sprintf("func(%s)", param)
+		}
+		result := strings.Join(results, ", ")
+		if !needParens {
+			return fmt.Sprintf("func(%s) %s", param, result)
+		}
+		return fmt.Sprintf("func(%s) (%s)", param, result)
 
-// oneLineTypeDecl prints a type declaration as a single line.
-func (pkg *Package) oneLineTypeDecl(spec *ast.TypeSpec) {
-	spec.Doc = nil
-	spec.Comment = nil
-	switch spec.Type.(type) {
-	case *ast.InterfaceType:
-		pkg.Printf("type %s interface { ... }\n", spec.Name)
 	case *ast.StructType:
-		pkg.Printf("type %s struct { ... }\n", spec.Name)
+		if n.Fields == nil || len(n.Fields.List) == 0 {
+			return "struct{}"
+		}
+		return "struct{ ... }"
+
+	case *ast.InterfaceType:
+		if n.Methods == nil || len(n.Methods.List) == 0 {
+			return "interface{}"
+		}
+		return "interface{ ... }"
+
+	case *ast.FieldList:
+		if n == nil || len(n.List) == 0 {
+			return ""
+		}
+		if len(n.List) == 1 {
+			return pkg.oneLineField(n.List[0], depth)
+		}
+		return dotDotDot
+
+	case *ast.FuncLit:
+		return pkg.oneLineNodeDepth(n.Type, depth) + " { ... }"
+
+	case *ast.CompositeLit:
+		typ := pkg.oneLineNodeDepth(n.Type, depth)
+		if len(n.Elts) == 0 {
+			return fmt.Sprintf("%s{}", typ)
+		}
+		return fmt.Sprintf("%s{ %s }", typ, dotDotDot)
+
+	case *ast.ArrayType:
+		length := pkg.oneLineNodeDepth(n.Len, depth)
+		element := pkg.oneLineNodeDepth(n.Elt, depth)
+		return fmt.Sprintf("[%s]%s", length, element)
+
+	case *ast.MapType:
+		key := pkg.oneLineNodeDepth(n.Key, depth)
+		value := pkg.oneLineNodeDepth(n.Value, depth)
+		return fmt.Sprintf("map[%s]%s", key, value)
+
+	case *ast.CallExpr:
+		fnc := pkg.oneLineNodeDepth(n.Fun, depth)
+		var args []string
+		for _, arg := range n.Args {
+			args = append(args, pkg.oneLineNodeDepth(arg, depth))
+		}
+		return fmt.Sprintf("%s(%s)", fnc, strings.Join(args, ", "))
+
+	case *ast.UnaryExpr:
+		return fmt.Sprintf("%s%s", n.Op, pkg.oneLineNodeDepth(n.X, depth))
+
+	case *ast.Ident:
+		return n.Name
+
 	default:
-		pkg.Printf("type %s %s\n", spec.Name, pkg.formatNode(spec.Type))
+		// As a fallback, use default formatter for all unknown node types.
+		buf := new(bytes.Buffer)
+		format.Node(buf, pkg.fs, node)
+		s := buf.String()
+		if strings.Contains(s, "\n") {
+			return dotDotDot
+		}
+		return s
 	}
 }
 
+// oneLineField returns a one-line summary of the field.
+func (pkg *Package) oneLineField(field *ast.Field, depth int) string {
+	var names []string
+	for _, name := range field.Names {
+		names = append(names, name.Name)
+	}
+	if len(names) == 0 {
+		return pkg.oneLineNodeDepth(field.Type, depth)
+	}
+	return strings.Join(names, ", ") + " " + pkg.oneLineNodeDepth(field.Type, depth)
+}
+
 // packageDoc prints the docs for the package (package doc plus one-liners of the rest).
 func (pkg *Package) packageDoc() {
 	defer pkg.flush()
@@ -329,7 +439,9 @@ func (pkg *Package) valueSummary(values []*doc.Value, showGrouped bool) {
 
 	for _, value := range values {
 		if !isGrouped[value] {
-			pkg.oneLineValueGenDecl("", value.Decl)
+			if decl := pkg.oneLineNode(value.Decl); decl != "" {
+				pkg.Printf("%s\n", decl)
+			}
 		}
 	}
 }
@@ -342,20 +454,18 @@ func (pkg *Package) funcSummary(funcs []*doc.Func, showConstructors bool) {
 	if !showConstructors {
 		isConstructor = make(map[*doc.Func]bool)
 		for _, typ := range pkg.doc.Types {
-			if !isExported(typ.Name) {
+			if isExported(typ.Name) {
-				continue
+				for _, f := range typ.Funcs {
-			}
+					isConstructor[f] = true
-			for _, f := range typ.Funcs {
+				}
-				isConstructor[f] = true
 			}
 		}
 	}
 	for _, fun := range funcs {
-		decl := fun.Decl
 		// Exported functions only. The go/doc package does not include methods here.
 		if isExported(fun.Name) {
 			if !isConstructor[fun] {
-				pkg.oneLineFunc(decl)
+				pkg.Printf("%s\n", pkg.oneLineNode(fun.Decl))
 			}
 		}
 	}
@@ -367,18 +477,21 @@ func (pkg *Package) typeSummary() {
 		for _, spec := range typ.Decl.Specs {
 			typeSpec := spec.(*ast.TypeSpec) // Must succeed.
 			if isExported(typeSpec.Name.Name) {
-				pkg.oneLineTypeDecl(typeSpec)
+				pkg.Printf("%s\n", pkg.oneLineNode(typeSpec))
 				// Now print the consts, vars, and constructors.
 				for _, c := range typ.Consts {
-					pkg.oneLineValueGenDecl(indent, c.Decl)
+					if decl := pkg.oneLineNode(c.Decl); decl != "" {
+						pkg.Printf(indent+"%s\n", decl)
+					}
 				}
 				for _, v := range typ.Vars {
-					pkg.oneLineValueGenDecl(indent, v.Decl)
+					if decl := pkg.oneLineNode(v.Decl); decl != "" {
+						pkg.Printf(indent+"%s\n", decl)
+					}
 				}
 				for _, constructor := range typ.Funcs {
 					if isExported(constructor.Name) {
-						pkg.Printf(indent)
+						pkg.Printf(indent+"%s\n", pkg.oneLineNode(constructor.Decl))
-						pkg.oneLineFunc(constructor.Decl)
 					}
 				}
 			}
@@ -486,7 +599,7 @@ func (pkg *Package) symbolDoc(symbol string) bool {
 						// This a standalone identifier, as in the case of iota usage.
 						// Thus, assume the type comes from the previous type.
 						vspec.Type = &ast.Ident{
-							Name:    string(pkg.formatNode(typ)),
+							Name:    string(pkg.oneLineNode(typ)),
 							NamePos: vspec.End() - 1,
 						}
 					}

src/cmd/doc/testdata/pkg.go

@@ -127,6 +127,36 @@ const Casematch = 2
 func ReturnUnexported() unexportedType { return 0 }
 func ReturnExported() ExportedType     { return ExportedType{} }
 
+const MultiLineConst = `
+	MultiLineString1
+	MultiLineString2
+	MultiLineString3
+`
+
+func MultiLineFunc(x interface {
+	MultiLineMethod1() int
+	MultiLineMethod2() int
+	MultiLineMethod3() int
+}) (r struct {
+	MultiLineField1 int
+	MultiLineField2 int
+	MultiLineField3 int
+}) {
+	return r
+}
+
+var MultiLineVar = map[struct {
+	MultiLineField1 string
+	MultiLineField2 uint64
+}]struct {
+	MultiLineField3 error
+	MultiLineField2 error
+}{
+	{"FieldVal1", 1}: {},
+	{"FieldVal2", 2}: {},
+	{"FieldVal3", 3}: {},
+}
+
 const (
 	_, _ uint64 = 2 * iota, 1 << iota
 	constLeft1, constRight1