Repository navigation
go/printer: can print an invalid return statement depending on position info #32854
Description
What version of Go are you using (go version)?
go version devel +2e0cd2aef5 Fri Jun 28 20:10:05 2019 +0000 linux/amd64
Does this issue reproduce with the latest release?
Yes.
What did you do?
https://play.golang.org/p/oxljScGL89r
What did you expect to see?
A valid program, like:
func f() {
return call()
}
What did you see instead?
The return call() being printed separately, which means two statements instead of one.
func f() {
return
call()
}
To make this more obvious, gofmt formats the last piece of code as:
func f() {
return
call()
}
I encountered this while writing a program to modify source code. Surprisingly, I ended up with code that wouldn't compile, and figured out it was because the printer wasn't doing the right thing.
I assume this is a small bug that can be fixed, such as not allowing a newline in this case. I'll investigate a bit. /cc @griesemer
Activity
mvdan commentedon Jun 29, 2019
I realise that this could qualify as "abusing" the syntax tree nodes. However, I don't see why
go/printershould ever write incorrect or different code. The only alternative would be to write lots of code to always make sure that the positions are OK, which I'm not sure is possible.av86743 commentedon Jun 29, 2019
Conceivably, your updated AST is not a valid one. Perhaps you can verify this using one of the AST library predicates/validators?
mvdan commentedon Jun 29, 2019
I'd disagree. Even if you print a syntax tree with an empty fileset, the printer should still print a valid program, even if the style ends up looking weird.
For example, other similar statements like
goanddeferdon't have this same bug, so I think this is simply human error. The fix is also trivial - I'm finishing the test now.av86743 commentedon Jun 29, 2019
So you are unable to verify whether your generated AST is valid?
gopherbot commentedon Jun 29, 2019
Change https://golang.org/cl/184121 mentions this issue:
go/printer: never print a newline before the returned resultsmvdan commentedon Jun 29, 2019
Sorry, I don't know what you mean. The syntax tree is perfectly valid. The position information obtained via the fileset is a bit weird, but still, I don't think that should lead to the printer producing incorrect output and no error.
I also don't know what validators you're talking about. You can validate the syntax tree example I gave above, if that helps.
temporarily work around a go/printer ReturnStmt bug
griesemer commentedon Jun 30, 2019
@mvdan I agree this is a bug. Unfortunately, the
go/printercan produce incorrect formatting in some cases.mvdan commentedon Jul 1, 2019
I understand these edge cases are super rare, primarily because one needs to craft a syntax tree manually. It's impossible for
gofmtto run into this bug, I think. In any case, I'm happy to send patches like this one if you're happy to review them.griesemer commentedon Jul 1, 2019
@mvdan I can review them but it's not a high priority. The shorter the better, of course :-)