]> Cypherpunks repositories - gostls13.git/commitdiff
go/printer: align expression list elements containing tabs
authorDaniel Martí <mvdan@mvdan.cc>
Sun, 8 May 2022 22:29:23 +0000 (23:29 +0100)
committerDaniel Martí <mvdan@mvdan.cc>
Tue, 10 May 2022 19:22:49 +0000 (19:22 +0000)
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>
src/cmd/gofmt/gofmt_test.go
src/cmd/gofmt/testdata/tabs.golden [new file with mode: 0644]
src/cmd/gofmt/testdata/tabs.input [new file with mode: 0644]
src/go/printer/nodes.go

index 641e0ea415f7ca7a5d7598305489accf91797930..6b80673af148f58a2dea38d71536d8c5755c1e6e 100644 (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)
+                       }
+               })
        }
 }
 
diff --git a/src/cmd/gofmt/testdata/tabs.golden b/src/cmd/gofmt/testdata/tabs.golden
new file mode 100644 (file)
index 0000000..287678c
--- /dev/null
@@ -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,
+       },
+}
diff --git a/src/cmd/gofmt/testdata/tabs.input b/src/cmd/gofmt/testdata/tabs.input
new file mode 100644 (file)
index 0000000..635be79
--- /dev/null
@@ -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,
+       },
+}
index 2cc84dc6a9b2c5e6ac6535770c12ff1f0d3509f1..c7cab46f17fc9c5c28cea877d03b6a2833e1506e 100644 (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