go/printer: printer should always print a compilable program even in the absence of correct position information

[griesemer]

Here's a simple test case exhibiting the problem:


package main

import (
    "go/parser"
    "go/printer"
    "go/token"
    "os"
)

const src = `// This is a comment
//
package main
import "fmt"
func main() {
    fmt.Printf("hello, world\n")
}
`

func main() {
    ast, _ := parser.ParseFile(token.NewFileSet(), "", src, parser.ParseComments)
    printer.Fprint(os.Stdout, token.NewFileSet(), ast)
}

Running this code produces (after fix for issue #1503):

package main

import "fmt"

func main() {
    fmt// This is a comment
    //
    .Printf("hello, world\n")
}

Automatic semicolon insertion inserts a semicolon after the "fmt" identifier
and thus breaks the program.

[griesemer]

Comment 1:

The current implementation of go/printer is from a time when Go did not have automatic
semicolon insertion. Consequently the implementation does not try to ensure that no line
breaks are inserted where they may cause extra semicolons; instead it now relies
critically on correct line information.
This makes go/printer less useful for printing generated ASTs where there is no line
information (or where it is at least difficult to compute "reasonable" line information).

[jimmyfrasche]

Comment 2:

I ran into this today working on a program to inject nodes into the AST.
Since I was generating temporary files to be burned after the compiler read them, I
tried to just walk the AST and strip all the documentation (had to parse it with the
documentation to read declarations I was putting there) and in the extremely limited
test cases I've run that seems to do the trick.
I did not try injecting after parsing without keeping comments. 
I could be wrong and there are other cases that manifest this issue*, but so far this
has worked for me (in my extremly limited and simple usages). I've attached the file I
boxed the workaround into.
*if I run into them I'll reply again

Attachments:

1. issue1505.go (774 bytes)

[rsc]

Comment 3:

Labels changed: added priority-later, removed priority-medium.

[griesemer]

Comment 4:

Issue #2636 has been merged into this issue.

[griesemer]

Comment 5:

Labels changed: added priority-go1, removed priority-later.

[griesemer]

Comment 6:

A comment re: the original program reported: That program is incorrect because it uses a
different (new) file set when printing instead of the one used for parsing. The
following program works correctly:
package main
import (
    "go/parser"
    "go/printer"
    "go/token"
    "os"
)
const src = `// This is a comment
//
package main
import "fmt"
func main() {
    fmt.Printf("hello, world\n")
}
`
func main() {
    fset := token.NewFileSet()
    ast, _ := parser.ParseFile(fset, "", src, parser.ParseComments)
    printer.Fprint(os.Stdout, fset, ast)
}
That said, the go/printer should not insert a //-style comment at a place where there is
no line break following in the source as it will invalidate the program. In general
however, an AST with incorrect position information (position information that is valid
but wrong) can always be created which will result in an incorrect Go program.
The go/printer fix can only concentrate on situations where no position information is
present.

[robpike]

Comment 7:

Owner changed to [email protected].

[griesemer]

Comment 8:

Issue #2750 has been merged into this issue.

[rsc]

Comment 9:

Labels changed: added go1-must.

[griesemer]

Comment 10:

Owner changed to @griesemer.

Status changed to Started.

[griesemer]

Comment 11:

Status update: Have working solution for existing and new test. Approach needs some
cleanup, tuning, and justification.
http://golang.org/cl/5598054

[griesemer]

Comment 12:

Issue #2898 has been merged into this issue.

[griesemer]

Comment 13:

This issue was closed by revision 3d6b368.

Status changed to Fixed.