go/printer: do not treat comments inside a ast.Decl as godoc

This change makes sure that we do not format comments
as doc comments inside of a declaration and makes the
go doc formatter idempotent:

Previously:

	// test comment
	//go:directive2
	// test comment
func main() {
}

was formatted to:

// test comment
//go:directive2
// test comment
func main() {
}

after another formatting, it got formatted with doc rules into:

// test comment
// test comment
//
//go:directive2
func main() {
}

With this change it gets directly to the correct form (last one).

Change-Id: Id7d8f03e43474357cd714e0672e886652c3fce86
GitHub-Last-Rev: 9833b87536c29f8be0e74c232936c5e6c5a9b78b
GitHub-Pull-Request: golang/go#69134
Reviewed-on: https://go-review.googlesource.com/c/go/+/609077
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>

diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go
index 495ec22031a3bce0bd364dc59565229e36ee513e..780d58ec5c141f2685b4f8c9f9760334d04d8a27 100644 (file)
--- a/src/go/printer/nodes.go
+++ b/src/go/printer/nodes.go
@@ -1737,6 +1737,9 @@ func (p *printer) genDecl(d *ast.GenDecl) {
        p.setPos(d.Pos())
        p.print(d.Tok, blank)
 
+       defer func(d bool) { p.inDecl = d }(p.inDecl)
+       p.inDecl = true
+
        if d.Lparen.IsValid() || len(d.Specs) != 1 {
                // group of parenthesized declarations
                p.setPos(d.Lparen)
@@ -1921,6 +1924,10 @@ func (p *printer) funcDecl(d *ast.FuncDecl) {
        p.setComment(d.Doc)
        p.setPos(d.Pos())
        p.print(token.FUNC, blank)
+
+       defer func(d bool) { p.inDecl = d }(p.inDecl)
+       p.inDecl = true
+
        // We have to save startCol only after emitting FUNC; otherwise it can be on a
        // different line (all whitespace preceding the FUNC is emitted only when the
        // FUNC is emitted).

diff --git a/src/go/printer/printer.go b/src/go/printer/printer.go
index 5a6127c6b40dd7a018e3bfd8a575af22a8b83225..3aaf8947be72f8715be9da4d58ec78605ad2d7eb 100644 (file)
--- a/src/go/printer/printer.go
+++ b/src/go/printer/printer.go
@@ -63,6 +63,7 @@ type printer struct {
        mode         pmode        // current printer mode
        endAlignment bool         // if set, terminate alignment immediately
        impliedSemi  bool         // if set, a linebreak implies a semicolon
+       inDecl       bool         // if set, printer is inside declaration (after first token)
        lastTok      token.Token  // last token printed (token.ILLEGAL if it's whitespace)
        prevOpen     token.Token  // previous non-brace "open" token (, [, or token.ILLEGAL
        wsbuf        []whiteSpace // delayed white space
@@ -739,8 +740,9 @@ func (p *printer) intersperseComments(next token.Position, tok token.Token) (wro
        for p.commentBefore(next) {
                list := p.comment.List
                changed := false
-               if p.lastTok != token.IMPORT && // do not rewrite cgo's import "C" comments
-                       p.posFor(p.comment.Pos()).Column == 1 &&
+               if !p.inDecl &&
+                       p.lastTok != token.IMPORT && // do not rewrite cgo's import "C" comments
+                       p.posFor(p.comment.Pos()).Line != p.last.Line &&
                        p.posFor(p.comment.End()+1) == next {
                        // Unindented comment abutting next token position:
                        // a top-level doc comment.

diff --git a/src/go/printer/printer_test.go b/src/go/printer/printer_test.go
index 4d5d3eabedca06f58816463cc639fdb8d9c7b778..00bb842cc89b85ab218985e7a24901a51f1e5bb7 100644 (file)
--- a/src/go/printer/printer_test.go
+++ b/src/go/printer/printer_test.go
@@ -16,6 +16,7 @@ import (
        "io"
        "os"
        "path/filepath"
+       "strings"
        "testing"
        "time"
 )
@@ -863,3 +864,192 @@ func TestEmptyDecl(t *testing.T) { // issue 63566
                }
        }
 }
+
+func TestDocFormat(t *testing.T) {
+       cases := []struct {
+               src  string
+               want string
+       }{
+               {
+                       src: `package main
+
+func main() {
+//
+//go:directive
+// test
+//
+}
+`,
+                       want: `package main
+
+func main() {
+       //
+       //go:directive
+       // test
+       //
+}
+`,
+               },
+               {
+                       src: `package main
+
+func main() {
+       //go:directive
+       // test
+       type a struct{}
+
+//go:directive
+// test
+test()
+}
+`,
+                       want: `package main
+
+func main() {
+       //go:directive
+       // test
+       type a struct{}
+
+       //go:directive
+       // test
+       test()
+}
+`,
+               },
+               {
+                       src: `package main
+
+func main() {
+//go:directive
+// test
+type a struct{}
+}
+`,
+                       want: `package main
+
+func main() {
+       //go:directive
+       // test
+       type a struct{}
+}
+`,
+               },
+               {
+                       src: `package main
+
+func a() {
+//line a:5:1
+       //
+}
+`,
+                       want: `package main
+
+func a() {
+//line a:5:1
+       //
+}
+`,
+               },
+
+               {
+                       src: `package main
+
+// test comment
+//go:directive2
+// test comment
+func main() {
+}
+`,
+                       want: `package main
+
+// test comment
+// test comment
+//
+//go:directive2
+func main() {
+}
+`,
+               },
+               {
+                       src: `package main
+
+       // test comment
+       //go:directive2
+       // test comment
+func main() {
+}
+`,
+                       want: `package main
+
+// test comment
+// test comment
+//
+//go:directive2
+func main() {
+}
+`,
+               },
+               {
+                       src: `package main
+
+/* test
+ */ // test comment
+//go:directive2
+// test comment
+func main() {
+}
+`,
+                       want: `package main
+
+/* test
+ */ // test comment
+//go:directive2
+// test comment
+func main() {
+}
+`,
+               },
+
+               {
+                       src: `package main  //comment
+var a int = 4 //comment
+func a() {
+}
+`,
+                       want: `package main  //comment
+var a int = 4 //comment
+func a() {
+}
+`,
+               },
+
+               // Edge case found by a fuzzer, not a real-world example.
+               {
+                       src:  "package A\n\nimport(\"\f\"\n//\n\"\")",
+                       want: "package A\n\nimport (\n\t\"\f\" //\n\t\"\"\n)\n",
+               },
+               {
+                       src:  "package A\n\nimport(`\f`\n//\n\"\")",
+                       want: "package A\n\nimport (\n\t`\f` //\n\t\"\"\n)\n",
+               },
+       }
+
+       for _, tt := range cases {
+               fset := token.NewFileSet()
+               f, err := parser.ParseFile(fset, "test.go", tt.src, parser.ParseComments|parser.SkipObjectResolution)
+               if err != nil {
+                       t.Fatal(err)
+               }
+
+               var buf strings.Builder
+               cfg := Config{Tabwidth: 8, Mode: UseSpaces | TabIndent}
+               if err := cfg.Fprint(&buf, fset, f); err != nil {
+                       t.Fatal(err)
+               }
+
+               got := buf.String()
+               if got != tt.want {
+                       t.Errorf("source\n%v\nformatted as:\n%v\nwant formatted as:\n%v", tt.src, got, tt.want)
+               }
+       }
+}