Skip to content

Commit 47d27a8

Browse files
elibenodeke-em
authored andcommitted
cmd/gofmt: fix computation of function header size
Function sizes are computed to determine whether a function can be kept on one line or should be split to several lines. Part of the computation is the function header from the FUNC token and until the opening { token. Prior to this change, the function header size used distance from the original source position of the current token, which led to issues when the source between FUNC and the original source position was rewritten (such as whitespace being collapsed). Now we take the current output position into account, so that header size represents the reformatted source rather than the original source. The following files in the Go repository are reformatted with this change: * strings/strings_test.go * cmd/compile/internal/gc/fmt.go In both cases the reformatting is minor and seems to be correct given the heuristic to single-line functions longer than 100 columns to multiple lines. Fixes #28082 Change-Id: Ib737f6933e09b79e83715211421d5262b366ec93 Reviewed-on: https://go-review.googlesource.com/c/go/+/188818 Run-TryBot: Emmanuel Odeke <[email protected]> Reviewed-by: Daniel Martí <[email protected]> Reviewed-by: Emmanuel Odeke <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 904fdb3 commit 47d27a8

File tree

5 files changed

+53
-18
lines changed

5 files changed

+53
-18
lines changed

src/cmd/compile/internal/gc/fmt.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,11 @@ func (o fmtOpTypeId) Format(s fmt.State, verb rune) { Op(o).format(s, verb,
275275
func (o fmtOpTypeIdName) Format(s fmt.State, verb rune) { Op(o).format(s, verb, FTypeIdName) }
276276
func (o Op) Format(s fmt.State, verb rune) { o.format(s, verb, FErr) }
277277

278-
func (t *fmtTypeErr) Format(s fmt.State, verb rune) { typeFormat((*types.Type)(t), s, verb, FErr) }
279-
func (t *fmtTypeDbg) Format(s fmt.State, verb rune) { typeFormat((*types.Type)(t), s, verb, FDbg) }
280-
func (t *fmtTypeTypeId) Format(s fmt.State, verb rune) { typeFormat((*types.Type)(t), s, verb, FTypeId) }
278+
func (t *fmtTypeErr) Format(s fmt.State, verb rune) { typeFormat((*types.Type)(t), s, verb, FErr) }
279+
func (t *fmtTypeDbg) Format(s fmt.State, verb rune) { typeFormat((*types.Type)(t), s, verb, FDbg) }
280+
func (t *fmtTypeTypeId) Format(s fmt.State, verb rune) {
281+
typeFormat((*types.Type)(t), s, verb, FTypeId)
282+
}
281283
func (t *fmtTypeTypeIdName) Format(s fmt.State, verb rune) {
282284
typeFormat((*types.Type)(t), s, verb, FTypeIdName)
283285
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
// testcase for issue #28082
8+
9+
func foo() {}
10+
11+
func main() {}
12+
13+
func bar() {}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
// testcase for issue #28082
8+
9+
func foo( ) {}
10+
11+
func main( ) {}
12+
13+
func bar() {}

src/go/printer/nodes.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -794,8 +794,11 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) {
794794
p.print(x)
795795

796796
case *ast.FuncLit:
797-
p.expr(x.Type)
798-
p.funcBody(p.distanceFrom(x.Type.Pos()), blank, x.Body)
797+
p.print(x.Type.Pos(), token.FUNC)
798+
// See the comment in funcDecl about how the header size is computed.
799+
startCol := p.out.Column - len("func")
800+
p.signature(x.Type.Params, x.Type.Results)
801+
p.funcBody(p.distanceFrom(x.Type.Pos(), startCol), blank, x.Body)
799802

800803
case *ast.ParenExpr:
801804
if _, hasParens := x.X.(*ast.ParenExpr); hasParens {
@@ -1689,28 +1692,30 @@ func (p *printer) funcBody(headerSize int, sep whiteSpace, b *ast.BlockStmt) {
16891692
p.block(b, 1)
16901693
}
16911694

1692-
// distanceFrom returns the column difference between from and p.pos (the current
1693-
// estimated position) if both are on the same line; if they are on different lines
1694-
// (or unknown) the result is infinity.
1695-
func (p *printer) distanceFrom(from token.Pos) int {
1696-
if from.IsValid() && p.pos.IsValid() {
1697-
if f := p.posFor(from); f.Line == p.pos.Line {
1698-
return p.pos.Column - f.Column
1699-
}
1695+
// distanceFrom returns the column difference between p.out (the current output
1696+
// position) and startOutCol. If the start position is on a different line from
1697+
// the current position (or either is unknown), the result is infinity.
1698+
func (p *printer) distanceFrom(startPos token.Pos, startOutCol int) int {
1699+
if startPos.IsValid() && p.pos.IsValid() && p.posFor(startPos).Line == p.pos.Line {
1700+
return p.out.Column - startOutCol
17001701
}
17011702
return infinity
17021703
}
17031704

17041705
func (p *printer) funcDecl(d *ast.FuncDecl) {
17051706
p.setComment(d.Doc)
17061707
p.print(d.Pos(), token.FUNC, blank)
1708+
// We have to save startCol only after emitting FUNC; otherwise it can be on a
1709+
// different line (all whitespace preceding the FUNC is emitted only when the
1710+
// FUNC is emitted).
1711+
startCol := p.out.Column - len("func ")
17071712
if d.Recv != nil {
17081713
p.parameters(d.Recv) // method: print receiver
17091714
p.print(blank)
17101715
}
17111716
p.expr(d.Name)
17121717
p.signature(d.Type.Params, d.Type.Results)
1713-
p.funcBody(p.distanceFrom(d.Pos()), vtab, d.Body)
1718+
p.funcBody(p.distanceFrom(d.Pos(), startCol), vtab, d.Body)
17141719
}
17151720

17161721
func (p *printer) decl(decl ast.Decl) {

src/strings/strings_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,12 @@ func runIndexTests(t *testing.T, f func(s, sep string) int, funcName string, tes
194194
}
195195
}
196196

197-
func TestIndex(t *testing.T) { runIndexTests(t, Index, "Index", indexTests) }
198-
func TestLastIndex(t *testing.T) { runIndexTests(t, LastIndex, "LastIndex", lastIndexTests) }
199-
func TestIndexAny(t *testing.T) { runIndexTests(t, IndexAny, "IndexAny", indexAnyTests) }
200-
func TestLastIndexAny(t *testing.T) { runIndexTests(t, LastIndexAny, "LastIndexAny", lastIndexAnyTests) }
197+
func TestIndex(t *testing.T) { runIndexTests(t, Index, "Index", indexTests) }
198+
func TestLastIndex(t *testing.T) { runIndexTests(t, LastIndex, "LastIndex", lastIndexTests) }
199+
func TestIndexAny(t *testing.T) { runIndexTests(t, IndexAny, "IndexAny", indexAnyTests) }
200+
func TestLastIndexAny(t *testing.T) {
201+
runIndexTests(t, LastIndexAny, "LastIndexAny", lastIndexAnyTests)
202+
}
201203

202204
func TestIndexByte(t *testing.T) {
203205
for _, tt := range indexTests {

0 commit comments

Comments
 (0)