Skip to content

Commit 670642d

Browse files
committed
go/parser, go/types: report invalid else branch in if statements
- Only accept valid if statement syntax in go/parser. - Check AST again in go/types since it may have been modified and the AST doesn't preclude other statements in the else branch of an if statement. - Removed a test from gofmt which verified that old-style if statements permitting any statement in the else branch were correctly reformatted. It's been years since we switched to the current syntax; no need to support this anymore. - Added a comment to go/printer. Fixes #13475. Change-Id: Id2c8fbcc68b719cd511027d0412a37266cceed6b Reviewed-on: https://go-review.googlesource.com/17408 Reviewed-by: Russ Cox <[email protected]>
1 parent c5aa53c commit 670642d

File tree

6 files changed

+25
-19
lines changed

6 files changed

+25
-19
lines changed

src/cmd/gofmt/testdata/old.golden

Lines changed: 0 additions & 9 deletions
This file was deleted.

src/cmd/gofmt/testdata/old.input

Lines changed: 0 additions & 8 deletions
This file was deleted.

src/go/parser/parser.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1857,7 +1857,16 @@ func (p *parser) parseIfStmt() *ast.IfStmt {
18571857
var else_ ast.Stmt
18581858
if p.tok == token.ELSE {
18591859
p.next()
1860-
else_ = p.parseStmt()
1860+
switch p.tok {
1861+
case token.IF:
1862+
else_ = p.parseIfStmt()
1863+
case token.LBRACE:
1864+
else_ = p.parseBlockStmt()
1865+
p.expectSemi()
1866+
default:
1867+
p.errorExpected(p.pos, "if statement or block")
1868+
else_ = &ast.BadStmt{From: p.pos, To: p.pos}
1869+
}
18611870
} else {
18621871
p.expectSemi()
18631872
}

src/go/parser/short_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ var invalids = []string{
121121
`package p; type _ struct { ( /* ERROR "expected anonymous field" */ int) };`,
122122
`package p; func _()(x, y, z ... /* ERROR "expected '\)', found '...'" */ int){}`,
123123
`package p; func _()(... /* ERROR "expected type, found '...'" */ int){}`,
124+
125+
// issue 13475
126+
`package p; func f() { if true {} else ; /* ERROR "expected if statement or block" */ }`,
127+
`package p; func f() { if true {} else defer /* ERROR "expected if statement or block" */ f() }`,
124128
}
125129

126130
func TestInvalid(t *testing.T) {

src/go/printer/nodes.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,6 +1185,9 @@ func (p *printer) stmt(stmt ast.Stmt, nextIsRBrace bool) {
11851185
case *ast.BlockStmt, *ast.IfStmt:
11861186
p.stmt(s.Else, nextIsRBrace)
11871187
default:
1188+
// This can only happen with an incorrectly
1189+
// constructed AST. Permit it but print so
1190+
// that it can be parsed without errors.
11881191
p.print(token.LBRACE, indent, formfeed)
11891192
p.stmt(s.Else, true)
11901193
p.print(unindent, formfeed, token.RBRACE)

src/go/types/stmt.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,15 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
457457
check.error(s.Cond.Pos(), "non-boolean condition in if statement")
458458
}
459459
check.stmt(inner, s.Body)
460-
if s.Else != nil {
460+
// The parser produces a correct AST but if it was modified
461+
// elsewhere the else branch may be invalid. Check again.
462+
switch s.Else.(type) {
463+
case nil, *ast.BadStmt:
464+
// valid or error already reported
465+
case *ast.IfStmt, *ast.BlockStmt:
461466
check.stmt(inner, s.Else)
467+
default:
468+
check.error(s.Else.Pos(), "invalid else branch in if statement")
462469
}
463470

464471
case *ast.SwitchStmt:

0 commit comments

Comments
 (0)