go/printer: align expression list elements containing tabs

A user noticed that, given the input

	{
		S:       "Hello World",
		Integer: 42,
	},
	{
		S:       "	", // an actual <tab>
		Integer: 42,
	},

gofmt would incorrectly format the code as

	{
		S:       "Hello World",
		Integer: 42,
	},
	{
		S: "	", // an actual <tab>
		Integer: 42,
	},

The problem was in the nodeSize method, used to get the printed length
of a node before it's actually printed to the final buffer.
The exprList method calls nodeSize to see if one expression in a list
changes too drastically in size from the previous, which means the
vertical alignment should be broken.

It is worth noting that nodeSize only reports valid lengths if the node
fits into a single line; otherwise, it returns a large number, larger
than an "infinity" currently set to 1e6.

However, the "does it fit in a single line" logic was broken;
it checked if any of the to-be-printed characters is less than ' ',
which does include '\n' and '\f' (the latter used by tabwriter as well),
but also includes '\t', which would make nodeSize incorrectly conclude
that our key-value expression with a tab does not fit into a single line.

While here, make the testdata test cases run as sub-tests,
as I used "-run TestRewrite/tabs.input" to help debug this.

Fixes #51910.

Change-Id: Ib7936e02652bc58f99772b06384ae271fddf09e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/404955
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Daniel Martí 2022-05-08 23:29:23 +01:00
parent 1284cc2495
commit bf68170c63
4 changed files with 81 additions and 11 deletions

View file

@ -144,15 +144,18 @@ func TestRewrite(t *testing.T) {
match = append(match, "gofmt.go", "gofmt_test.go")
for _, in := range match {
out := in // for files where input and output are identical
if strings.HasSuffix(in, ".input") {
out = in[:len(in)-len(".input")] + ".golden"
}
runTest(t, in, out)
if in != out {
// Check idempotence.
runTest(t, out, out)
}
name := filepath.Base(in)
t.Run(name, func(t *testing.T) {
out := in // for files where input and output are identical
if strings.HasSuffix(in, ".input") {
out = in[:len(in)-len(".input")] + ".golden"
}
runTest(t, in, out)
if in != out && !t.Failed() {
// Check idempotence.
runTest(t, out, out)
}
})
}
}

33
src/cmd/gofmt/testdata/tabs.golden vendored Normal file
View file

@ -0,0 +1,33 @@
// Copyright 2022 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.
//gofmt
package main
var _ = []struct {
S string
Integer int
}{
{
S: "Hello World",
Integer: 42,
},
{
S: "\t",
Integer: 42,
},
{
S: " ", // an actual <tab>
Integer: 42,
},
{
S: ` `, // an actual <tab>
Integer: 42,
},
{
S: "\u0009",
Integer: 42,
},
}

33
src/cmd/gofmt/testdata/tabs.input vendored Normal file
View file

@ -0,0 +1,33 @@
// Copyright 2022 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.
//gofmt
package main
var _ = []struct{
S string
Integer int
}{
{
S: "Hello World",
Integer: 42,
},
{
S: "\t",
Integer: 42,
},
{
S: " ", // an actual <tab>
Integer: 42,
},
{
S: ` `, // an actual <tab>
Integer: 42,
},
{
S: "\u0009",
Integer: 42,
},
}

View file

@ -1746,8 +1746,9 @@ func (p *printer) nodeSize(n ast.Node, maxSize int) (size int) {
}
if buf.Len() <= maxSize {
for _, ch := range buf.Bytes() {
if ch < ' ' {
return
switch ch {
case '\n', '\f':
return // does not fit in a single line
}
}
size = buf.Len() // n fits