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
cmd/gofmt: inconsistent formatting of empty methods #28082
Comments
An interesting behavior. For anyone confused why it formats instead of printing an error like |
This patch solves this specific issue: diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go
index 1de7cd81b2..38b50f2596 100644
--- a/src/go/printer/nodes.go
+++ b/src/go/printer/nodes.go
@@ -1693,7 +1693,9 @@ func (p *printer) funcBody(headerSize int, sep whiteSpace, b *ast.BlockStmt) {
func (p *printer) distanceFrom(from token.Pos) int {
if from.IsValid() && p.pos.IsValid() {
if f := p.posFor(from); f.Line == p.pos.Line {
- return p.pos.Column - f.Column
+ // Use actual (formatted, i.e., p.out) column information
+ // to get an accurate column distance. See issue #28082.
+ return p.out.Column - f.Column
}
}
return infinity But it leads to cmd/gofmt test failures. Haven't investigated. Not urgent. If somebody wants to analyze this a bit more, please go ahead and keep me in the loop, thanks. |
I looked into why @griesemer 's proposed patch causes test failures. The failures are in tests that format fragments of code that don't have a function wrapping them. For example
In this case,
When the printer sees this code, it starts by splitting off
I found a workaround that makes the whole test suite pass. It basically assumes that this discrepancy ( Even though all tests pass and the original snippets by @neild are formatted correctly, this doesn’t feel robust. While figuring out the workaround I encountered several other failing tests that are sensitive to changes in A more robust fix could be to remember the last emitted func header in the state, and use that to measure its size. |
The most accurate measure of the actual header length would be to count how many bytes were emitted by the header printer. |
@eliben Worth a try. |
Change https://golang.org/cl/188818 mentions this issue: |
What version of Go are you using (
go version
)?go version go1.11rc2 linux/amd64
Does this issue reproduce with the latest release?
yes
What did you do?
Format https://play.golang.org/p/VW-uvEb5Mro
What did you expect to see?
https://play.golang.org/p/34L1Aw0cZ1D
What did you see instead?
Looks like gofmt is deciding whether to turn
{}
into{\n}
based on the original length of the line, rather than the post-formatting length. This is harmless, but surprising.The text was updated successfully, but these errors were encountered: