From c3f16307b3ff28578c915b8513f6369ad9f5e8f3 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Wed, 4 Sep 2024 07:05:17 +0000 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Griesemer Auto-Submit: Robert Griesemer --- src/go/printer/nodes.go | 7 ++ src/go/printer/printer.go | 6 +- src/go/printer/printer_test.go | 190 +++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+), 2 deletions(-) diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go index 495ec22031a3b..780d58ec5c141 100644 --- 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 5a6127c6b40dd..3aaf8947be72f 100644 --- 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 4d5d3eabedca0..00bb842cc89b8 100644 --- 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) + } + } +}