go/parser: more robust error handling for 'if' headers

R=go1.11

To fix this, this CL borrows code from the new syntax
package which has a better tuned parser at this point.

Fixes #11377.

Change-Id: Ib9212c945903d6f62abcc59ef5a5767d4ef36981
Reviewed-on: https://go-review.googlesource.com/87495
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go
index 2b58724521fb41cec65f9f3d8e88da50ab80e74d..88a5eb67d227113dcfbb659b20e1c8156c77e7d0 100644 (file)
--- a/src/go/parser/parser.go
+++ b/src/go/parser/parser.go
@@ -1808,48 +1808,90 @@ func (p *parser) parseBranchStmt(tok token.Token) *ast.BranchStmt {
        return &ast.BranchStmt{TokPos: pos, Tok: tok, Label: label}
 }
 
-func (p *parser) makeExpr(s ast.Stmt, kind string) ast.Expr {
+func (p *parser) makeExpr(s ast.Stmt, want string) ast.Expr {
        if s == nil {
                return nil
        }
        if es, isExpr := s.(*ast.ExprStmt); isExpr {
                return p.checkExpr(es.X)
        }
-       p.error(s.Pos(), fmt.Sprintf("expected %s, found simple statement (missing parentheses around composite literal?)", kind))
+       found := "simple statement"
+       if _, isAss := s.(*ast.AssignStmt); isAss {
+               found = "assignment"
+       }
+       p.error(s.Pos(), fmt.Sprintf("expected %s, found %s (missing parentheses around composite literal?)", want, found))
        return &ast.BadExpr{From: s.Pos(), To: p.safePos(s.End())}
 }
 
-func (p *parser) parseIfStmt() *ast.IfStmt {
-       if p.trace {
-               defer un(trace(p, "IfStmt"))
+// parseIfHeader is an adjusted version of parser.header
+// in cmd/compile/internal/syntax/parser.go, which has
+// been tuned for better error handling.
+func (p *parser) parseIfHeader() (init ast.Stmt, cond ast.Expr) {
+       if p.tok == token.LBRACE {
+               p.error(p.pos, "missing condition in if statement")
+               return
        }
+       // p.tok != token.LBRACE
 
-       pos := p.expect(token.IF)
-       p.openScope()
-       defer p.closeScope()
+       outer := p.exprLev
+       p.exprLev = -1
 
-       var s ast.Stmt
-       var x ast.Expr
-       {
-               prevLev := p.exprLev
-               p.exprLev = -1
+       if p.tok != token.SEMICOLON {
+               // accept potential variable declaration but complain
+               if p.tok == token.VAR {
+                       p.next()
+                       p.error(p.pos, fmt.Sprintf("var declaration not allowed in 'IF' initializer"))
+               }
+               init, _ = p.parseSimpleStmt(basic)
+       }
+
+       var condStmt ast.Stmt
+       var semi struct {
+               pos token.Pos
+               lit string // ";" or "\n"; valid if pos.IsValid()
+       }
+       if p.tok != token.LBRACE {
                if p.tok == token.SEMICOLON {
+                       semi.pos = p.pos
+                       semi.lit = p.lit
                        p.next()
-                       x = p.parseRhs()
                } else {
-                       s, _ = p.parseSimpleStmt(basic)
-                       if p.tok == token.SEMICOLON {
-                               p.next()
-                               x = p.parseRhs()
-                       } else {
-                               x = p.makeExpr(s, "boolean expression")
-                               s = nil
-                       }
+                       p.expect(token.SEMICOLON)
                }
-               p.exprLev = prevLev
+               if p.tok != token.LBRACE {
+                       condStmt, _ = p.parseSimpleStmt(basic)
+               }
+       } else {
+               condStmt = init
+               init = nil
+       }
+
+       if condStmt != nil {
+               cond = p.makeExpr(condStmt, "boolean expression")
+       } else if semi.pos.IsValid() {
+               if semi.lit == "\n" {
+                       p.error(semi.pos, "unexpected newline, expecting { after if clause")
+               } else {
+                       p.error(semi.pos, "missing condition in if statement")
+               }
+       }
+
+       p.exprLev = outer
+       return
+}
+
+func (p *parser) parseIfStmt() *ast.IfStmt {
+       if p.trace {
+               defer un(trace(p, "IfStmt"))
        }
 
+       pos := p.expect(token.IF)
+       p.openScope()
+       defer p.closeScope()
+
+       init, cond := p.parseIfHeader()
        body := p.parseBlockStmt()
+
        var else_ ast.Stmt
        if p.tok == token.ELSE {
                p.next()
@@ -1867,7 +1909,7 @@ func (p *parser) parseIfStmt() *ast.IfStmt {
                p.expectSemi()
        }
 
-       return &ast.IfStmt{If: pos, Init: s, Cond: x, Body: body, Else: else_}
+       return &ast.IfStmt{If: pos, Init: init, Cond: cond, Body: body, Else: else_}
 }
 
 func (p *parser) parseTypeList() (list []ast.Expr) {

diff --git a/src/go/parser/short_test.go b/src/go/parser/short_test.go
index 6f8ef6b0f775d98c918510fd4c878f28a11f64cb..49bb681e09e66ea275934c5d85dd2f95925fb4b4 100644 (file)
--- a/src/go/parser/short_test.go
+++ b/src/go/parser/short_test.go
@@ -58,10 +58,10 @@ func TestValid(t *testing.T) {
 
 var invalids = []string{
        `foo /* ERROR "expected 'package'" */ !`,
-       `package p; func f() { if { /* ERROR "expected operand" */ } };`,
-       `package p; func f() { if ; { /* ERROR "expected operand" */ } };`,
-       `package p; func f() { if f(); { /* ERROR "expected operand" */ } };`,
-       `package p; func f() { if _ /* ERROR "expected boolean expression" */ = range x; true {} };`,
+       `package p; func f() { if { /* ERROR "missing condition" */ } };`,
+       `package p; func f() { if ; /* ERROR "missing condition" */ {} };`,
+       `package p; func f() { if f(); /* ERROR "missing condition" */ {} };`,
+       `package p; func f() { if _ = range /* ERROR "expected operand" */ x; true {} };`,
        `package p; func f() { switch _ /* ERROR "expected switch expression" */ = range x; true {} };`,
        `package p; func f() { for _ = range x ; /* ERROR "expected '{'" */ ; {} };`,
        `package p; func f() { for ; ; _ = range /* ERROR "expected operand" */ x {} };`,
@@ -85,7 +85,7 @@ var invalids = []string{
        `package p; func f() { _ = (<-<- /* ERROR "expected 'chan'" */ chan int)(nil) };`,
        `package p; func f() { _ = (<-chan<-chan<-chan<-chan<-chan<- /* ERROR "expected channel type" */ int)(nil) };`,
        `package p; func f() { var t []int; t /* ERROR "expected identifier on left side of :=" */ [0] := 0 };`,
-       `package p; func f() { if x := g(); x = /* ERROR "expected '=='" */ 0 {}};`,
+       `package p; func f() { if x := g(); x /* ERROR "expected boolean expression" */ = 0 {}};`,
        `package p; func f() { _ = x = /* ERROR "expected '=='" */ 0 {}};`,
        `package p; func f() { _ = 1 == func()int { var x bool; x = x = /* ERROR "expected '=='" */ true; return x }() };`,
        `package p; func f() { var s []int; _ = s[] /* ERROR "expected operand" */ };`,

diff --git a/src/go/parser/testdata/issue11377.src b/src/go/parser/testdata/issue11377.src
new file mode 100644 (file)
index 0000000..1c43800
--- /dev/null
+++ b/src/go/parser/testdata/issue11377.src
@@ -0,0 +1,27 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test case for issue 11377: Better synchronization of
+// parser after certain syntax errors.
+
+package p
+
+func bad1() {
+    if f()) /* ERROR "expected ';', found '\)'" */ {
+        return
+    }
+}
+
+// There shouldn't be any errors down below.
+
+func F1() {}
+func F2() {}
+func F3() {}
+func F4() {}
+func F5() {}
+func F6() {}
+func F7() {}
+func F8() {}
+func F9() {}
+func F10() {}