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

diff --git a/src/pkg/go/parser/error_test.go b/src/pkg/go/parser/error_test.go
index d4d4f909d3ffd909c5dfa36bae9f6c3e8c745d00..8506077cee626d7464b0b8deb115fc0d1e39e12f 100644 (file)
--- a/src/pkg/go/parser/error_test.go
+++ b/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)
                }
        }
 }

diff --git a/src/pkg/go/parser/parser.go b/src/pkg/go/parser/parser.go
index c3e3ee859ab2f623c47634e0c5296b84054e3198..68eb3cd1e2b42b59a3da0c4bc40f57b75d9d5aa2 100644 (file)
--- a/src/pkg/go/parser/parser.go
+++ b/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())}},
                }
        }
 

diff --git a/src/pkg/go/parser/short_test.go b/src/pkg/go/parser/short_test.go
index 22f79930b372ef46b856b28912b579d54eceb532..b794060998f66de3d7b9ac639a06fcf0d101e3a1 100644 (file)
--- a/src/pkg/go/parser/short_test.go
+++ b/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) {