go/printer, gofmt: don't print incorrect programs

Be careful when printing line comments with incorrect
position information. Maintain additional state
impliedSemi: when set, a comment containing a newline
would imply a semicolon and thus placement must be
delayed.

Precompute state information pertaining to the next
comment for faster checks (the printer is marginally
faster now despite additional checks for each comment).

No effect on existing src, misc sources.

Fixes #1505.

R=rsc
CC=golang-dev
https://golang.org/cl/5598054

Author: griesemer

src/cmd/fix/timefileinfo_test.go

@@ -156,6 +156,32 @@ func main() {
 	t2 := time.Now()
 	dt := t2.Sub(t1)
 }
+`,
+	},
+	{
+		Name: "timefileinfo.5", // test for issues 1505, 2636
+		In: `package main
+
+import (
+	"fmt"
+	"time"
+)
+
+func main() {
+	fmt.Println(time.SecondsToUTC(now)) // this comment must not introduce an illegal linebreak
+}
+`,
+		Out: `package main
+
+import (
+	"fmt"
+	"time"
+)
+
+func main() {
+	fmt.Println(time.Unix(now, 0).UTC( // this comment must not introduce an illegal linebreak
+	))
+}
 `,
 	},
 }

src/pkg/go/printer/nodes.go

@@ -76,6 +76,7 @@ func (p *printer) setComment(g *ast.CommentGroup) {
 	}
 	p.comments[0] = g
 	p.cindex = 0
+	p.nextComment() // get comment ready for use
 }
 
 type exprListMode uint

src/pkg/go/printer/printer.go

@@ -12,7 +12,6 @@ import (
 	"go/token"
 	"io"
 	"os"
-	"path/filepath"
 	"strconv"
 	"strings"
 	"text/tabwriter"
@@ -52,11 +51,12 @@ type printer struct {
 	fset *token.FileSet
 
 	// Current state
-	output  bytes.Buffer // raw printer result
-	indent  int          // current indentation
-	mode    pmode        // current printer mode
-	lastTok token.Token  // the last token printed (token.ILLEGAL if it's whitespace)
-	wsbuf   []whiteSpace // delayed white space
+	output      bytes.Buffer // raw printer result
+	indent      int          // current indentation
+	mode        pmode        // current printer mode
+	impliedSemi bool         // if set, a linebreak implies a semicolon
+	lastTok     token.Token  // the last token printed (token.ILLEGAL if it's whitespace)
+	wsbuf       []whiteSpace // delayed white space
 
 	// The (possibly estimated) position in the generated output;
 	// in AST space (i.e., pos is set whenever a token position is
@@ -73,6 +73,11 @@ type printer struct {
 	cindex          int                 // current comment index
 	useNodeComments bool                // if not set, ignore lead and line comments of nodes
 
+	// Information about p.comments[p.cindex]; set up by nextComment.
+	comment        *ast.CommentGroup // = p.comments[p.cindex]; or nil
+	commentOffset  int               // = p.posFor(p.comments[p.cindex].List[0].Pos()).Offset; or infinity
+	commentNewline bool              // true if the comment group contains newlines
+
 	// Cache of already computed node sizes.
 	nodeSizes map[ast.Node]int
 
@@ -89,6 +94,42 @@ func (p *printer) init(cfg *Config, fset *token.FileSet, nodeSizes map[ast.Node]
 	p.cachedPos = -1
 }
 
+// commentsHaveNewline reports whether a list of comments belonging to
+// an *ast.CommentGroup contains newlines. Because the position information
+// may only be partially correct, we also have to read the comment text.
+func (p *printer) commentsHaveNewline(list []*ast.Comment) bool {
+	// len(list) > 0
+	line := p.lineFor(list[0].Pos())
+	for i, c := range list {
+		if i > 0 && p.lineFor(list[i].Pos()) != line {
+			// not all comments on the same line
+			return true
+		}
+		if t := c.Text; len(t) >= 2 && (t[1] == '/' || strings.Contains(t, "\n")) {
+			return true
+		}
+	}
+	_ = line
+	return false
+}
+
+func (p *printer) nextComment() {
+	for p.cindex < len(p.comments) {
+		c := p.comments[p.cindex]
+		p.cindex++
+		if list := c.List; len(list) > 0 {
+			p.comment = c
+			p.commentOffset = p.posFor(list[0].Pos()).Offset
+			p.commentNewline = p.commentsHaveNewline(list)
+			return
+		}
+		// we should not reach here (correct ASTs don't have empty
+		// ast.CommentGroup nodes), but be conservative and try again
+	}
+	// no more comments
+	p.commentOffset = infinity
+}
+
 func (p *printer) internalError(msg ...interface{}) {
 	if debug {
 		fmt.Print(p.pos.String() + ": ")
@@ -204,8 +245,7 @@ func (p *printer) writeItem(pos token.Position, data string, isLit bool) {
 	}
 	if debug {
 		// do not update p.pos - use write0
-		_, filename := filepath.Split(pos.Filename)
-		fmt.Fprintf(&p.output, "[%s:%d:%d]", filename, pos.Line, pos.Column)
+		fmt.Fprintf(&p.output, "/*%s*/", pos)
 	}
 	p.writeString(data, isLit)
 	p.last = p.pos
@@ -618,12 +658,13 @@ func (p *printer) writeCommentSuffix(needsLinebreak bool) (wroteNewline, dropped
 //
 func (p *printer) intersperseComments(next token.Position, tok token.Token) (wroteNewline, droppedFF bool) {
 	var last *ast.Comment
-	for ; p.commentBefore(next); p.cindex++ {
-		for _, c := range p.comments[p.cindex].List {
+	for p.commentBefore(next) {
+		for _, c := range p.comment.List {
 			p.writeCommentPrefix(p.posFor(c.Pos()), next, last, c, tok.IsKeyword())
 			p.writeComment(c)
 			last = c
 		}
+		p.nextComment()
 	}
 
 	if last != nil {
@@ -735,22 +776,24 @@ func mayCombine(prev token.Token, next byte) (b bool) {
 // printed, followed by the actual token.
 //
 func (p *printer) print(args ...interface{}) {
-	for _, f := range args {
-		next := p.pos // estimated position of next item
-		data := ""
-		isLit := false
-		var tok token.Token
+	for _, arg := range args {
+		// information about the current arg
+		var data string
+		var isLit bool
+		var impliedSemi bool // value for p.impliedSemi after this arg
 
-		switch x := f.(type) {
+		switch x := arg.(type) {
 		case pmode:
 			// toggle printer mode
 			p.mode ^= x
+			continue
+
 		case whiteSpace:
 			if x == ignore {
 				// don't add ignore's to the buffer; they
 				// may screw up "correcting" unindents (see
 				// LabeledStmt)
-				break
+				continue
 			}
 			i := len(p.wsbuf)
 			if i == cap(p.wsbuf) {
@@ -762,13 +805,27 @@ func (p *printer) print(args ...interface{}) {
 			}
 			p.wsbuf = p.wsbuf[0 : i+1]
 			p.wsbuf[i] = x
+			if x == newline || x == formfeed {
+				// newlines affect the current state (p.impliedSemi)
+				// and not the state after printing arg (impliedSemi)
+				// because comments can be interspersed before the arg
+				// in this case
+				p.impliedSemi = false
+			}
+			p.lastTok = token.ILLEGAL
+			continue
+
 		case *ast.Ident:
 			data = x.Name
-			tok = token.IDENT
+			impliedSemi = true
+			p.lastTok = token.IDENT
+
 		case *ast.BasicLit:
 			data = x.Value
 			isLit = true
-			tok = x.Kind
+			impliedSemi = true
+			p.lastTok = x.Kind
+
 		case token.Token:
 			s := x.String()
 			if mayCombine(p.lastTok, s[0]) {
@@ -785,30 +842,40 @@ func (p *printer) print(args ...interface{}) {
 				p.wsbuf[0] = ' '
 			}
 			data = s
-			tok = x
+			// some keywords followed by a newline imply a semicolon
+			switch x {
+			case token.BREAK, token.CONTINUE, token.FALLTHROUGH, token.RETURN,
+				token.INC, token.DEC, token.RPAREN, token.RBRACK, token.RBRACE:
+				impliedSemi = true
+			}
+			p.lastTok = x
+
 		case token.Pos:
 			if x.IsValid() {
-				next = p.posFor(x) // accurate position of next item
+				p.pos = p.posFor(x) // accurate position of next item
 			}
-			tok = p.lastTok
+			continue
+
 		case string:
 			// incorrect AST - print error message
 			data = x
 			isLit = true
-			tok = token.STRING
+			impliedSemi = true
+			p.lastTok = token.STRING
+
 		default:
-			fmt.Fprintf(os.Stderr, "print: unsupported argument %v (%T)\n", f, f)
+			fmt.Fprintf(os.Stderr, "print: unsupported argument %v (%T)\n", arg, arg)
 			panic("go/printer type")
 		}
-		p.lastTok = tok
-		p.pos = next
+		// data != ""
 
-		if data != "" {
-			wroteNewline, droppedFF := p.flush(next, tok)
+		next := p.pos // estimated/accurate position of next item
+		wroteNewline, droppedFF := p.flush(next, p.lastTok)
 
-			// intersperse extra newlines if present in the source
-			// (don't do this in flush as it will cause extra newlines
-			// at the end of a file)
+		// intersperse extra newlines if present in the source and
+		// if they don't cause extra semicolons (don't do this in
+		// flush as it will cause extra newlines at the end of a file)
+		if !p.impliedSemi {
 			n := nlimit(next.Line - p.pos.Line)
 			// don't exceed maxNewlines if we already wrote one
 			if wroteNewline && n == maxNewlines {
@@ -820,22 +887,25 @@ func (p *printer) print(args ...interface{}) {
 					ch = '\f' // use formfeed since we dropped one before
 				}
 				p.writeByteN(ch, n)
+				impliedSemi = false
 			}
-
-			p.writeItem(next, data, isLit)
 		}
+
+		p.writeItem(next, data, isLit)
+		p.impliedSemi = impliedSemi
 	}
 }
 
-// commentBefore returns true iff the current comment occurs
-// before the next position in the source code.
+// commentBefore returns true iff the current comment group occurs
+// before the next position in the source code and printing it does
+// not introduce implicit semicolons.
 //
-func (p *printer) commentBefore(next token.Position) bool {
-	return p.cindex < len(p.comments) && p.posFor(p.comments[p.cindex].List[0].Pos()).Offset < next.Offset
+func (p *printer) commentBefore(next token.Position) (result bool) {
+	return p.commentOffset < next.Offset && (!p.impliedSemi || !p.commentNewline)
 }
 
-// Flush prints any pending comments and whitespace occurring textually
-// before the position of the next token tok. The Flush result indicates
+// flush prints any pending comments and whitespace occurring textually
+// before the position of the next token tok. The flush result indicates
 // if a newline was written or if a formfeed was dropped from the whitespace
 // buffer.
 //
@@ -915,6 +985,9 @@ func (p *printer) printNode(node interface{}) error {
 	// if there are no comments, use node comments
 	p.useNodeComments = p.comments == nil
 
+	// get comments ready for use
+	p.nextComment()
+
 	// format node
 	switch n := node.(type) {
 	case ast.Expr:
@@ -1068,6 +1141,8 @@ func (cfg *Config) fprint(output io.Writer, fset *token.FileSet, node interface{
 	if err = p.printNode(node); err != nil {
 		return
 	}
+	// print outstanding comments
+	p.impliedSemi = false // EOF acts like a newline
 	p.flush(token.Position{Offset: infinity, Line: infinity}, token.EOF)
 
 	// redirect output through a trimmer to eliminate trailing whitespace