From: Rob Pike <r@golang.org>
Date: Fri, 3 Apr 2020 02:57:01 +0000 (+1100)
Subject: cmd/doc: don't print package clauses on error
X-Git-Tag: go1.15beta1~516
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=670cb9c37769f07fd8c2aac6270a9f2342d2b970;p=gostls13.git

cmd/doc: don't print package clauses on error

Everybody was deferring a flush when main already
did that, so drop all that nonsense. (Flush was doing
the package clause stuff.) But then make sure we do
get a package clause when there is correctly no output,
as for an empty package. Do that by triggering a
package clause in allDoc and packageDoc.

Slightly tricky but way less intricate than before.

Fixes #37969.

Change-Id: Ia86828436e6c4ab46e6fdaf2c550047f37f353f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/226998
Reviewed-by: Russ Cox <rsc@golang.org>
---

diff --git a/src/cmd/doc/doc_test.go b/src/cmd/doc/doc_test.go
index 80b8cd4d69..f13d917634 100644
--- a/src/cmd/doc/doc_test.go
+++ b/src/cmd/doc/doc_test.go
@@ -1000,6 +1000,25 @@ func TestDotSlashLookup(t *testing.T) {
 	}
 }
 
+// Test that we don't print spurious package clauses
+// when there should be no output at all. Issue 37969.
+func TestNoPackageClauseWhenNoMatch(t *testing.T) {
+	maybeSkip(t)
+	var b bytes.Buffer
+	var flagSet flag.FlagSet
+	err := do(&b, &flagSet, []string{"template.ZZZ"})
+	// Expect an error.
+	if err == nil {
+		t.Error("expect an error for template.zzz")
+	}
+	// And the output should not contain any package clauses.
+	const dontWant = `package template // import `
+	output := b.String()
+	if strings.Contains(output, dontWant) {
+		t.Fatalf("improper package clause printed:\n%s", output)
+	}
+}
+
 type trimTest struct {
 	path   string
 	prefix string
diff --git a/src/cmd/doc/pkg.go b/src/cmd/doc/pkg.go
index 7b8bd1aeb4..ffc302c78c 100644
--- a/src/cmd/doc/pkg.go
+++ b/src/cmd/doc/pkg.go
@@ -214,8 +214,6 @@ func (pkg *Package) Printf(format string, args ...interface{}) {
 }
 
 func (pkg *Package) flush() {
-	// Print the package clause in case it wasn't written already.
-	pkg.buf.packageClause()
 	_, err := pkg.writer.Write(pkg.buf.Bytes())
 	if err != nil {
 		log.Fatal(err)
@@ -451,8 +449,7 @@ func joinStrings(ss []string) string {
 
 // allDoc prints all the docs for the package.
 func (pkg *Package) allDoc() {
-	defer pkg.flush()
-
+	pkg.Printf("") // Trigger the package clause; we know the package exists.
 	doc.ToText(&pkg.buf, pkg.doc.Doc, "", indent, indentedWidth)
 	pkg.newlines(1)
 
@@ -511,8 +508,7 @@ func (pkg *Package) allDoc() {
 
 // packageDoc prints the docs for the package (package doc plus one-liners of the rest).
 func (pkg *Package) packageDoc() {
-	defer pkg.flush()
-
+	pkg.Printf("") // Trigger the package clause; we know the package exists.
 	if !short {
 		doc.ToText(&pkg.buf, pkg.doc.Doc, "", indent, indentedWidth)
 		pkg.newlines(1)
@@ -705,7 +701,6 @@ func (pkg *Package) findTypeSpec(decl *ast.GenDecl, symbol string) *ast.TypeSpec
 // If symbol matches a type, output includes its methods factories and associated constants.
 // If there is no top-level symbol, symbolDoc looks for methods that match.
 func (pkg *Package) symbolDoc(symbol string) bool {
-	defer pkg.flush()
 	found := false
 	// Functions.
 	for _, fun := range pkg.findFuncs(symbol) {
@@ -925,7 +920,6 @@ func trimUnexportedFields(fields *ast.FieldList, isInterface bool) *ast.FieldLis
 // If symbol is empty, it prints all methods for any concrete type
 // that match the name. It reports whether it found any methods.
 func (pkg *Package) printMethodDoc(symbol, method string) bool {
-	defer pkg.flush()
 	types := pkg.findTypes(symbol)
 	if types == nil {
 		if symbol == "" {
@@ -991,7 +985,6 @@ func (pkg *Package) printFieldDoc(symbol, fieldName string) bool {
 	if symbol == "" || fieldName == "" {
 		return false
 	}
-	defer pkg.flush()
 	types := pkg.findTypes(symbol)
 	if types == nil {
 		pkg.Fatalf("symbol %s is not a type in package %s installed in %q", symbol, pkg.name, pkg.build.ImportPath)
@@ -1047,13 +1040,11 @@ func (pkg *Package) printFieldDoc(symbol, fieldName string) bool {
 
 // methodDoc prints the docs for matches of symbol.method.
 func (pkg *Package) methodDoc(symbol, method string) bool {
-	defer pkg.flush()
 	return pkg.printMethodDoc(symbol, method)
 }
 
 // fieldDoc prints the docs for matches of symbol.field.
 func (pkg *Package) fieldDoc(symbol, field string) bool {
-	defer pkg.flush()
 	return pkg.printFieldDoc(symbol, field)
 }