From bf68170c638e7e69bedcc64fadfd83354fd06c10 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Daniel=20Mart=C3=AD?= Date: Sun, 8 May 2022 23:29:23 +0100 Subject: [PATCH] go/printer: align expression list elements containing tabs MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit A user noticed that, given the input { S: "Hello World", Integer: 42, }, { S: " ", // an actual Integer: 42, }, gofmt would incorrectly format the code as { S: "Hello World", Integer: 42, }, { S: " ", // an actual 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í TryBot-Result: Gopher Robot Reviewed-by: Robert Griesemer Reviewed-by: Heschi Kreinick --- src/cmd/gofmt/gofmt_test.go | 21 +++++++++++-------- src/cmd/gofmt/testdata/tabs.golden | 33 ++++++++++++++++++++++++++++++ src/cmd/gofmt/testdata/tabs.input | 33 ++++++++++++++++++++++++++++++ src/go/printer/nodes.go | 5 +++-- 4 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 src/cmd/gofmt/testdata/tabs.golden create mode 100644 src/cmd/gofmt/testdata/tabs.input diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index 641e0ea415..6b80673af1 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -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) + } + }) } } diff --git a/src/cmd/gofmt/testdata/tabs.golden b/src/cmd/gofmt/testdata/tabs.golden new file mode 100644 index 0000000000..287678cfc9 --- /dev/null +++ b/src/cmd/gofmt/testdata/tabs.golden @@ -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 + Integer: 42, + }, + { + S: ` `, // an actual + Integer: 42, + }, + { + S: "\u0009", + Integer: 42, + }, +} diff --git a/src/cmd/gofmt/testdata/tabs.input b/src/cmd/gofmt/testdata/tabs.input new file mode 100644 index 0000000000..635be797c9 --- /dev/null +++ b/src/cmd/gofmt/testdata/tabs.input @@ -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 + Integer: 42, + }, + { + S: ` `, // an actual + Integer: 42, + }, + { + S: "\u0009", + Integer: 42, + }, +} diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go index 2cc84dc6a9..c7cab46f17 100644 --- a/src/go/printer/nodes.go +++ b/src/go/printer/nodes.go @@ -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 -- 2.48.1