go/parser: better error position for non-invoked gp/defer functions

Added test cases and expanded test harness to handle token end
positions.

Also: Make sure token end positions are never outside the valid
      position range, as was possible in case of parse errors.

Fixes #7458.

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/70190046

src/pkg/go/parser/error_test.go

@@ -59,8 +59,11 @@ func getPos(filename string, offset int) token.Pos {
 
 // ERROR comments must be of the form /* ERROR "rx" */ and rx is
 // a regular expression that matches the expected error message.
+// The special form /* ERROR HERE "rx" */ must be used for error
+// messages that appear immediately after a token, rather than at
+// a token's position.
 //
-var errRx = regexp.MustCompile(`^/\* *ERROR *"([^"]*)" *\*/$`)
+var errRx = regexp.MustCompile(`^/\* *ERROR *(HERE)? *"([^"]*)" *\*/$`)
 
 // expectedErrors collects the regular expressions of ERROR comments found
 // in files and returns them as a map of error positions to error messages.
@@ -74,6 +77,7 @@ func expectedErrors(t *testing.T, filename string, src []byte) map[token.Pos]str
 	// not match the position information collected by the parser
 	s.Init(getFile(filename), src, nil, scanner.ScanComments)
 	var prev token.Pos // position of last non-comment, non-semicolon token
+	var here token.Pos // position immediately after the token at position prev
 
 	for {
 		pos, tok, lit := s.Scan()
@@ -82,11 +86,22 @@ func expectedErrors(t *testing.T, filename string, src []byte) map[token.Pos]str
 			return errors
 		case token.COMMENT:
 			s := errRx.FindStringSubmatch(lit)
-			if len(s) == 2 {
-				errors[prev] = string(s[1])
+			if len(s) == 3 {
+				pos := prev
+				if s[1] == "HERE" {
+					pos = here
+				}
+				errors[pos] = string(s[2])
 			}
 		default:
 			prev = pos
+			var l int // token length
+			if tok.IsLiteral() {
+				l = len(lit)
+			} else {
+				l = len(tok.String())
+			}
+			here = prev + token.Pos(l)
 		}
 	}
 }

src/pkg/go/parser/parser.go

@@ -492,6 +492,26 @@ func syncDecl(p *parser) {
 	}
 }
 
+// safePos returns a valid file position for a given position: If pos
+// is valid to begin with, safePos returns pos. If pos is out-of-range,
+// safePos returns the EOF position.
+//
+// This is hack to work around "artifical" end positions in the AST which
+// are computed by adding 1 to (presumably valid) token positions. If the
+// token positions are invalid due to parse errors, the resulting end position
+// may be past the file's EOF position, which would lead to panics if used
+// later on.
+//
+func (p *parser) safePos(pos token.Pos) (res token.Pos) {
+	defer func() {
+		if recover() != nil {
+			res = token.Pos(p.file.Base() + p.file.Size()) // EOF position
+		}
+	}()
+	_ = p.file.Offset(pos) // trigger a panic if position is out-of-range
+	return pos
+}
+
 // ----------------------------------------------------------------------------
 // Identifiers
 
@@ -679,7 +699,7 @@ func (p *parser) parseFieldDecl(scope *ast.Scope) *ast.Field {
 		if n := len(list); n > 1 || !isTypeName(deref(typ)) {
 			pos := typ.Pos()
 			p.errorExpected(pos, "anonymous field")
-			typ = &ast.BadExpr{From: pos, To: list[n-1].End()}
+			typ = &ast.BadExpr{From: pos, To: p.safePos(list[n-1].End())}
 		}
 	}
 
@@ -1337,7 +1357,7 @@ func (p *parser) checkExpr(x ast.Expr) ast.Expr {
 	default:
 		// all other nodes are not proper expressions
 		p.errorExpected(x.Pos(), "expression")
-		x = &ast.BadExpr{From: x.Pos(), To: x.End()}
+		x = &ast.BadExpr{From: x.Pos(), To: p.safePos(x.End())}
 	}
 	return x
 }
@@ -1400,7 +1420,7 @@ func (p *parser) checkExprOrType(x ast.Expr) ast.Expr {
 	case *ast.ArrayType:
 		if len, isEllipsis := t.Len.(*ast.Ellipsis); isEllipsis {
 			p.error(len.Pos(), "expected array length, found '...'")
-			x = &ast.BadExpr{From: x.Pos(), To: x.End()}
+			x = &ast.BadExpr{From: x.Pos(), To: p.safePos(x.End())}
 		}
 	}
 
@@ -1686,14 +1706,14 @@ func (p *parser) parseSimpleStmt(mode int) (ast.Stmt, bool) {
 	return &ast.ExprStmt{X: x[0]}, false
 }
 
-func (p *parser) parseCallExpr() *ast.CallExpr {
+func (p *parser) parseCallExpr(callType string) *ast.CallExpr {
 	x := p.parseRhsOrType() // could be a conversion: (some type)(x)
 	if call, isCall := x.(*ast.CallExpr); isCall {
 		return call
 	}
 	if _, isBad := x.(*ast.BadExpr); !isBad {
 		// only report error if it's a new one
-		p.errorExpected(x.Pos(), "function/method call")
+		p.error(p.safePos(x.End()), fmt.Sprintf("function must be invoked in %s statement", callType))
 	}
 	return nil
 }
@@ -1704,7 +1724,7 @@ func (p *parser) parseGoStmt() ast.Stmt {
 	}
 
 	pos := p.expect(token.GO)
-	call := p.parseCallExpr()
+	call := p.parseCallExpr("go")
 	p.expectSemi()
 	if call == nil {
 		return &ast.BadStmt{From: pos, To: pos + 2} // len("go")
@@ -1719,7 +1739,7 @@ func (p *parser) parseDeferStmt() ast.Stmt {
 	}
 
 	pos := p.expect(token.DEFER)
-	call := p.parseCallExpr()
+	call := p.parseCallExpr("defer")
 	p.expectSemi()
 	if call == nil {
 		return &ast.BadStmt{From: pos, To: pos + 5} // len("defer")
@@ -1770,7 +1790,7 @@ func (p *parser) makeExpr(s ast.Stmt, kind string) ast.Expr {
 		return p.checkExpr(es.X)
 	}
 	p.error(s.Pos(), fmt.Sprintf("expected %s, found simple statement (missing parentheses around composite literal?)", kind))
-	return &ast.BadExpr{From: s.Pos(), To: s.End()}
+	return &ast.BadExpr{From: s.Pos(), To: p.safePos(s.End())}
 }
 
 func (p *parser) parseIfStmt() *ast.IfStmt {
@@ -2052,7 +2072,7 @@ func (p *parser) parseForStmt() ast.Stmt {
 			key = as.Lhs[0]
 		default:
 			p.errorExpected(as.Lhs[0].Pos(), "1 or 2 expressions")
-			return &ast.BadStmt{From: pos, To: body.End()}
+			return &ast.BadStmt{From: pos, To: p.safePos(body.End())}
 		}
 		// parseSimpleStmt returned a right-hand side that
 		// is a single unary expression of the form "range x"
@@ -2299,7 +2319,7 @@ func (p *parser) parseReceiver(scope *ast.Scope) *ast.FieldList {
 			p.errorExpected(base.Pos(), "(unqualified) identifier")
 		}
 		par.List = []*ast.Field{
-			{Type: &ast.BadExpr{From: recv.Pos(), To: recv.End()}},
+			{Type: &ast.BadExpr{From: recv.Pos(), To: p.safePos(recv.End())}},
 		}
 	}
 

src/pkg/go/parser/short_test.go

@@ -86,6 +86,9 @@ var invalids = []string{
 	`package p; func f() { for x /* ERROR "boolean or range expression" */ := []string {} }`,
 	`package p; func f() { for i /* ERROR "boolean or range expression" */ , x = []string {} }`,
 	`package p; func f() { for i /* ERROR "boolean or range expression" */ , x := []string {} }`,
+	`package p; func f() { go f /* ERROR HERE "function must be invoked" */ }`,
+	`package p; func f() { defer func() {} /* ERROR HERE "function must be invoked" */ }`,
+	`package p; func f() { go func() { func() { f(x func /* ERROR "expected '\)'" */ (){}) } } }`,
 }
 
 func TestInvalid(t *testing.T) {