cmd/compile/internal/syntax: better error for malformed 'if' statements

Use distinction between explicit and automatically inserted semicolons
to provide a better error message if the condition in an 'if' statement
is missing.

For #18747.

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

diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go
index 79d6c8c14dd3047ad81db9be37137a486a7afd7c..e4aaa12ae50e4ae0b81ff749f05090a032cd2683 100644 (file)
--- a/src/cmd/compile/internal/syntax/parser.go
+++ b/src/cmd/compile/internal/syntax/parser.go
@@ -1662,7 +1662,7 @@ func (p *parser) forStmt() Stmt {
        s.init(p)
 
        p.want(_For)
-       s.Init, s.Cond, s.Post = p.header(true)
+       s.Init, s.Cond, s.Post = p.header(_For)
        if gcCompat {
                s.init(p)
        }
@@ -1688,10 +1688,13 @@ func (p *parser) stmtBody(context string) []Stmt {
        return body
 }
 
-var dummyCond = &Name{Value: "false"}
+func (p *parser) header(keyword token) (init SimpleStmt, cond Expr, post SimpleStmt) {
+       // TODO(gri) move caller's p.want(keyword) here, once we removed gcCompat
 
-func (p *parser) header(forStmt bool) (init SimpleStmt, cond Expr, post SimpleStmt) {
        if p.tok == _Lbrace {
+               if keyword == _If {
+                       p.syntax_error("missing condition in if statement")
+               }
                return
        }
 
@@ -1700,11 +1703,11 @@ func (p *parser) header(forStmt bool) (init SimpleStmt, cond Expr, post SimpleSt
 
        if p.tok != _Semi {
                // accept potential varDecl but complain
-               if forStmt && p.got(_Var) {
+               if keyword == _For && p.got(_Var) {
                        p.syntax_error("var declaration not allowed in for initializer")
                }
-               init = p.simpleStmt(nil, forStmt)
-               // If we have a range clause, we are done.
+               init = p.simpleStmt(nil, keyword == _For)
+               // If we have a range clause, we are done (can only happen for keyword == _For).
                if _, ok := init.(*RangeClause); ok {
                        p.xnest = outer
                        return
@@ -1712,8 +1715,15 @@ func (p *parser) header(forStmt bool) (init SimpleStmt, cond Expr, post SimpleSt
        }
 
        var condStmt SimpleStmt
-       if p.got(_Semi) {
-               if forStmt {
+       var semi struct {
+               pos src.Pos
+               lit string
+       }
+       if p.tok == _Semi {
+               semi.pos = p.pos()
+               semi.lit = p.lit
+               p.next()
+               if keyword == _For {
                        if p.tok != _Semi {
                                condStmt = p.simpleStmt(nil, false)
                        }
@@ -1732,12 +1742,17 @@ func (p *parser) header(forStmt bool) (init SimpleStmt, cond Expr, post SimpleSt
        // unpack condStmt
        switch s := condStmt.(type) {
        case nil:
-               // nothing to do
+               if keyword == _If {
+                       if semi.lit != "semicolon" {
+                               p.syntax_error_at(semi.pos, fmt.Sprintf("unexpected %s, expecting { after if clause", semi.lit))
+                       } else {
+                               p.syntax_error_at(semi.pos, "missing condition in if statement")
+                       }
+               }
        case *ExprStmt:
                cond = s.X
        default:
                p.syntax_error(fmt.Sprintf("%s used as value", String(s)))
-               cond = dummyCond // avoid follow-up error for if statements
        }
 
        p.xnest = outer
@@ -1753,10 +1768,7 @@ func (p *parser) ifStmt() *IfStmt {
        s.init(p)
 
        p.want(_If)
-       s.Init, s.Cond, _ = p.header(false)
-       if s.Cond == nil {
-               p.syntax_error("missing condition in if statement")
-       }
+       s.Init, s.Cond, _ = p.header(_If)
 
        if gcCompat {
                s.init(p)
@@ -1788,7 +1800,7 @@ func (p *parser) switchStmt() *SwitchStmt {
        s := new(SwitchStmt)
        s.init(p)
 
-       s.Init, s.Tag, _ = p.header(false)
+       s.Init, s.Tag, _ = p.header(_Switch)
 
        if !p.got(_Lbrace) {
                p.syntax_error("missing { after switch clause")

diff --git a/test/fixedbugs/issue18747.go b/test/fixedbugs/issue18747.go
new file mode 100644 (file)
index 0000000..fb8331f
--- /dev/null
+++ b/test/fixedbugs/issue18747.go
@@ -0,0 +1,28 @@
+// errorcheck
+
+// Copyright 2017 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.
+
+package p
+
+func _ () {
+       if {} // ERROR "missing condition in if statement"
+
+       if
+       {} // ERROR "missing condition in if statement"
+
+       if ; {} // ERROR "missing condition in if statement"
+
+       if foo; {} // ERROR "missing condition in if statement"
+
+       if foo; // ERROR "missing condition in if statement"
+       {}
+
+       if foo {}
+
+       if ; foo {}
+
+       if foo // ERROR "unexpected newline, expecting { after if clause"
+       {}
+}