Skip to content

Commit

Permalink
go/printer: do not treat comments inside a ast.Decl as godoc
Browse files Browse the repository at this point in the history
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: 9833b87
GitHub-Pull-Request: #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>
  • Loading branch information
mateusz834 authored and gopherbot committed Sep 5, 2024
1 parent 89958ab commit c3f1630
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 2 deletions.
7 changes: 7 additions & 0 deletions src/go/printer/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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).
Expand Down
6 changes: 4 additions & 2 deletions src/go/printer/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
190 changes: 190 additions & 0 deletions src/go/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"io"
"os"
"path/filepath"
"strings"
"testing"
"time"
)
Expand Down Expand Up @@ -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)
}
}
}

0 comments on commit c3f1630

Please sign in to comment.