Skip to content

Commit

Permalink
format: ignore //line directives when computing positions
Browse files Browse the repository at this point in the history
Otherwise, if the line directives give us line numbers
which don't make any sense given the real file's line numbers,
our use of MergeLines can cause a panic or just bad formatting.

Fixes #288.
  • Loading branch information
mvdan committed Jan 28, 2024
1 parent 9e77a5f commit 37e0463
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 7 deletions.
26 changes: 19 additions & 7 deletions format/format.go
Expand Up @@ -106,7 +106,7 @@ func File(fset *token.FileSet, file *ast.File, opts Options) {
}
opts.LangVersion = "v" + m[2] + "." + m[3]
f := &fumpter{
File: fset.File(file.Pos()),
file: fset.File(file.Pos()),
fset: fset,
astFile: file,
Options: opts,
Expand Down Expand Up @@ -179,7 +179,7 @@ var rxOctalInteger = regexp.MustCompile(`\A0[0-7_]+\z`)
type fumpter struct {
Options

*token.File
file *token.File
fset *token.FileSet

astFile *ast.File
Expand Down Expand Up @@ -227,7 +227,7 @@ func (f *fumpter) addNewline(at token.Pos) {
offset := f.Offset(at)

// TODO: replace with the new Lines method once we require Go 1.21 or later
field := reflect.ValueOf(f.File).Elem().FieldByName("lines")
field := reflect.ValueOf(f.file).Elem().FieldByName("lines")
n := field.Len()
lines := make([]int, 0, n+1)
for i := 0; i < n; i++ {
Expand All @@ -246,7 +246,7 @@ func (f *fumpter) addNewline(at token.Pos) {
if offset >= 0 {
lines = append(lines, offset)
}
if !f.SetLines(lines) {
if !f.file.SetLines(lines) {
panic(fmt.Sprintf("could not set lines to %v", lines))
}
}
Expand All @@ -255,7 +255,7 @@ func (f *fumpter) addNewline(at token.Pos) {
// up on the same line.
func (f *fumpter) removeLines(fromLine, toLine int) {
for fromLine < toLine {
f.MergeLine(fromLine)
f.file.MergeLine(fromLine)
toLine--
}
}
Expand All @@ -266,6 +266,18 @@ func (f *fumpter) removeLinesBetween(from, to token.Pos) {
f.removeLines(f.Line(from)+1, f.Line(to))
}

func (f *fumpter) Position(p token.Pos) token.Position {
return f.file.PositionFor(p, false)
}

func (f *fumpter) Line(p token.Pos) int {
return f.Position(p).Line
}

func (f *fumpter) Offset(p token.Pos) int {
return f.file.Offset(p)
}

type byteCounter int

func (b *byteCounter) Write(p []byte) (n int, err error) {
Expand Down Expand Up @@ -295,14 +307,14 @@ func (f *fumpter) lineEnd(line int) token.Pos {
if line < 1 {
panic("illegal line number")
}
total := f.LineCount()
total := f.file.LineCount()
if line > total {
panic("illegal line number")
}
if line == total {
return f.astFile.End()
}
return f.LineStart(line+1) - 1
return f.file.LineStart(line+1) - 1
}

// rxCommentDirective covers all common Go comment directives:
Expand Down
173 changes: 173 additions & 0 deletions testdata/script/linedirectives.txtar
@@ -0,0 +1,173 @@
# Line directives can throw off our use of MergeLines.
# We should ignore them entirely when calculating line numbers.
# The file below is borrowed from Go's test/dwarf/linedirectives.go.

exec gofumpt -w foo.go
cmp foo.go foo.go.golden

-- foo.go --
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//line foo/bar.y:4
package main
//line foo/bar.y:60
func main() {
//line foo/bar.y:297
f, l := 0, 0
//line yacctab:1
f, l = 1, 1
//line yaccpar:1
f, l = 2, 1
//line foo/bar.y:82
f, l = 3, 82
//line foo/bar.y:90
f, l = 3, 90
//line foo/bar.y:92
f, l = 3, 92
//line foo/bar.y:100
f, l = 3, 100
//line foo/bar.y:104
l = 104
//line foo/bar.y:112
l = 112
//line foo/bar.y:117
l = 117
//line foo/bar.y:121
l = 121
//line foo/bar.y:125
l = 125
//line foo/bar.y:133
l = 133
//line foo/bar.y:146
l = 146
//line foo/bar.y:148
//line foo/bar.y:153
//line foo/bar.y:155
l = 155
//line foo/bar.y:160

//line foo/bar.y:164
//line foo/bar.y:173

//line foo/bar.y:178
//line foo/bar.y:180
//line foo/bar.y:185
//line foo/bar.y:195
//line foo/bar.y:197
//line foo/bar.y:202
//line foo/bar.y:204
//line foo/bar.y:208
//line foo/bar.y:211
//line foo/bar.y:213
//line foo/bar.y:215
//line foo/bar.y:217
//line foo/bar.y:221
//line foo/bar.y:229
//line foo/bar.y:236
//line foo/bar.y:238
//line foo/bar.y:240
//line foo/bar.y:244
//line foo/bar.y:249
//line foo/bar.y:253
//line foo/bar.y:257
//line foo/bar.y:262
//line foo/bar.y:267
//line foo/bar.y:272
if l == f {
//line foo/bar.y:277
panic("aie!")
//line foo/bar.y:281
}
//line foo/bar.y:285
return
//line foo/bar.y:288
//line foo/bar.y:290
}
//line foo/bar.y:293
//line foo/bar.y:295
-- foo.go.golden --
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//line foo/bar.y:4
package main

//line foo/bar.y:60
func main() {
//line foo/bar.y:297
f, l := 0, 0
//line yacctab:1
f, l = 1, 1
//line yaccpar:1
f, l = 2, 1
//line foo/bar.y:82
f, l = 3, 82
//line foo/bar.y:90
f, l = 3, 90
//line foo/bar.y:92
f, l = 3, 92
//line foo/bar.y:100
f, l = 3, 100
//line foo/bar.y:104
l = 104
//line foo/bar.y:112
l = 112
//line foo/bar.y:117
l = 117
//line foo/bar.y:121
l = 121
//line foo/bar.y:125
l = 125
//line foo/bar.y:133
l = 133
//line foo/bar.y:146
l = 146
//line foo/bar.y:148
//line foo/bar.y:153
//line foo/bar.y:155
l = 155
//line foo/bar.y:160

//line foo/bar.y:164
//line foo/bar.y:173

//line foo/bar.y:178
//line foo/bar.y:180
//line foo/bar.y:185
//line foo/bar.y:195
//line foo/bar.y:197
//line foo/bar.y:202
//line foo/bar.y:204
//line foo/bar.y:208
//line foo/bar.y:211
//line foo/bar.y:213
//line foo/bar.y:215
//line foo/bar.y:217
//line foo/bar.y:221
//line foo/bar.y:229
//line foo/bar.y:236
//line foo/bar.y:238
//line foo/bar.y:240
//line foo/bar.y:244
//line foo/bar.y:249
//line foo/bar.y:253
//line foo/bar.y:257
//line foo/bar.y:262
//line foo/bar.y:267
//line foo/bar.y:272
if l == f {
//line foo/bar.y:277
panic("aie!")
//line foo/bar.y:281
}
//line foo/bar.y:285
return
//line foo/bar.y:288
//line foo/bar.y:290
}

//line foo/bar.y:293
//line foo/bar.y:295

0 comments on commit 37e0463

Please sign in to comment.