Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gofmt: not idempotent on line with multiple comments #24472

Open
ALTree opened this issue Mar 21, 2018 · 3 comments

Comments

@ALTree
Copy link
Member

commented Mar 21, 2018

$ gotip version
go version devel +5f0a9ba134 Tue Mar 20 22:46:00 2018 +0000 linux/amd64

Note as the second gofmt invocation changes the code again:

$ cat test.go
package p

func f() {
	foo(); /* one */ fooooo(); /* two */
}

$ cat test.go | gofmt
package p

func f() {
	foo() /* one */
	fooooo() /* two */
}

$ cat test.go | gofmt | gofmt
package p

func f() {
	foo()    /* one */
	fooooo() /* two */

@ALTree ALTree added this to the Go1.11 milestone Mar 21, 2018

@ALTree ALTree changed the title go/fmt: not idempotent on line with multiple comments gofmt: not idempotent on line with multiple comments Mar 21, 2018

@griesemer griesemer removed this from the Go1.11 milestone Mar 21, 2018

@griesemer griesemer self-assigned this Mar 21, 2018

@griesemer griesemer added this to the Unplanned milestone Mar 21, 2018

@ysmolsky

This comment has been minimized.

Copy link
Member

commented May 13, 2018

FYI, this break following test:

% go test cmd/gofmt
--- FAIL: TestAll (13.14s)
	long_test.go:88: gofmt /Users/thorn/golang/test/fixedbugs/issue22662.go not idempotent
FAIL
FAIL	cmd/gofmt	13.182s
@gopherbot

This comment has been minimized.

Copy link

commented Aug 21, 2018

Change https://golang.org/cl/130377 mentions this issue: cmd/gofmt: skip gofmt idempotency check on known issue

gopherbot pushed a commit that referenced this issue Aug 21, 2018
cmd/gofmt: skip gofmt idempotency check on known issue
gofmt's TestAll runs gofmt on all the go files in the tree and checks,
among other things, that gofmt is idempotent (i.e. that a second
invocation does not change the input again).

There's a known bug of gofmt not being idempotent (Issue #24472), and
unfortunately the fixedbugs/issue22662.go file triggers it. We can't
just gofmt the file, because it tests the effect of various line
directives inside weirdly-placed comments, and gofmt moves those
comments, making the test useless.

Instead, just skip the idempotency check when gofmt-ing the
problematic file.

This fixes go test on the cmd/gofmt package, and a failure seen on the
longtest builder.

Updates #24472

Change-Id: Ib06300977cd8fce6c609e688b222e9b2186f5aa7
Reviewed-on: https://go-review.googlesource.com/130377
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@ALTree

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2018

The bad file tested fixedbugs/issue22662.go (which was causing failures in the std library test suite in TestAll when run without -short) is now skipped (CL 130377). The check should be re-enabled on that file when this issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.