From c5f3a8b10797258cf527601a44bfdfa63d5ef1a7 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 11 Jan 2018 15:09:38 -0800 Subject: [PATCH] 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 --- src/go/parser/parser.go | 90 ++++++++++++++++++++------- src/go/parser/short_test.go | 10 +-- src/go/parser/testdata/issue11377.src | 27 ++++++++ 3 files changed, 98 insertions(+), 29 deletions(-) create mode 100644 src/go/parser/testdata/issue11377.src diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go index 2b58724521..88a5eb67d2 100644 --- 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 6f8ef6b0f7..49bb681e09 100644 --- 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 index 0000000000..1c438003eb --- /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() {} -- 2.50.0