go/parser: fix type switch scoping

The variable declared by a TypeSwitchGuard must be
visible in each TypeCaseClause and must not conflict
with other variables declared by the initial SimpleStmt
of a type switch.

Also:
- explicitly detect type switches (as opposed to detecting
  regular (expression switches) and then do extra testing
  for type switches
- fix all outstanding TODOs in parser.go

R=rsc
CC=golang-dev
https://golang.org/cl/4896053

diff --git a/src/cmd/gotype/testdata/test1.go b/src/cmd/gotype/testdata/test1.go
index 0bd46568d69a7ad8c88ed570b9f4a2a5619fae68..a3298e6e5fe61d55ae4b18b3c72e2d28c00fe25b 100644 (file)
--- a/src/cmd/gotype/testdata/test1.go
+++ b/src/cmd/gotype/testdata/test1.go
@@ -4,3 +4,20 @@ func _() {
        // the scope of a local type declaration starts immediately after the type name
        type T struct{ _ *T }
 }
+
+func _(x interface{}) {
+       // the variable defined by a TypeSwitchGuard is declared in each TypeCaseClause
+       switch t := x.(type) {
+       case int:
+               _ = t
+       case float32:
+               _ = t
+       default:
+               _ = t
+       }
+
+       // the variable defined by a TypeSwitchGuard must not conflict with other
+       // variables declared in the initial simple statement
+       switch t := 0; t := x.(type) {
+       }
+}

diff --git a/src/pkg/go/parser/parser.go b/src/pkg/go/parser/parser.go
index 9c14d16673242fb2973924abd1f7c34a66bb703a..1c2afe6413ea67473998023143840edca7f21257 100644 (file)
--- a/src/pkg/go/parser/parser.go
+++ b/src/pkg/go/parser/parser.go
@@ -587,7 +587,6 @@ func (p *parser) parseStructType() *ast.StructType {
        }
        rbrace := p.expect(token.RBRACE)
 
-       // TODO(gri): store struct scope in AST
        return &ast.StructType{pos, &ast.FieldList{lbrace, list, rbrace}, false}
 }
 
@@ -800,7 +799,6 @@ func (p *parser) parseInterfaceType() *ast.InterfaceType {
        }
        rbrace := p.expect(token.RBRACE)
 
-       // TODO(gri): store interface scope in AST
        return &ast.InterfaceType{pos, &ast.FieldList{lbrace, list, rbrace}, false}
 }
 
@@ -1436,14 +1434,14 @@ func (p *parser) parseSimpleStmt(mode int) (ast.Stmt, bool) {
        case token.ARROW:
                // send statement
                arrow := p.pos
-               p.next() // consume "<-"
+               p.next()
                y := p.parseRhs()
                return &ast.SendStmt{x[0], arrow, y}, false
 
        case token.INC, token.DEC:
                // increment or decrement
                s := &ast.IncDecStmt{x[0], p.pos, p.tok}
-               p.next() // consume "++" or "--"
+               p.next()
                return s, false
        }
 
@@ -1591,7 +1589,7 @@ func (p *parser) parseTypeList() (list []ast.Expr) {
        return
 }
 
-func (p *parser) parseCaseClause(exprSwitch bool) *ast.CaseClause {
+func (p *parser) parseCaseClause(varName string) *ast.CaseClause {
        if p.trace {
                defer un(trace(p, "CaseClause"))
        }
@@ -1600,10 +1598,12 @@ func (p *parser) parseCaseClause(exprSwitch bool) *ast.CaseClause {
        var list []ast.Expr
        if p.tok == token.CASE {
                p.next()
-               if exprSwitch {
-                       list = p.parseRhsList()
-               } else {
+               if varName != "" {
+                       // type switch
                        list = p.parseTypeList()
+               } else {
+                       // expression switch
+                       list = p.parseRhsList()
                }
        } else {
                p.expect(token.DEFAULT)
@@ -1611,21 +1611,31 @@ func (p *parser) parseCaseClause(exprSwitch bool) *ast.CaseClause {
 
        colon := p.expect(token.COLON)
        p.openScope()
+       // If we have a type switch declaring a variable in its TypeSwitchGuard,
+       // declare that variable in each of the TypeCaseClauses.
+       if varName != "" && varName != "_" {
+               ident := &ast.Ident{Name: varName} // dummy identifier
+               p.declare(nil, nil, p.topScope, ast.Var, ident)
+       }
        body := p.parseStmtList()
        p.closeScope()
 
        return &ast.CaseClause{pos, list, colon, body}
 }
 
-func isExprSwitch(s ast.Stmt) bool {
-       if s == nil {
-               return true
-       }
-       if e, ok := s.(*ast.ExprStmt); ok {
-               if a, ok := e.X.(*ast.TypeAssertExpr); ok {
-                       return a.Type != nil // regular type assertion
-               }
-               return true
+func isTypeSwitchAssert(x ast.Expr) bool {
+       a, ok := x.(*ast.TypeAssertExpr)
+       return ok && a.Type == nil
+}
+
+func isTypeSwitchGuard(s ast.Stmt) bool {
+       switch t := s.(type) {
+       case *ast.ExprStmt:
+               // x.(nil)
+               return isTypeSwitchAssert(t.X)
+       case *ast.AssignStmt:
+               // v := x.(nil)
+               return len(t.Lhs) == 1 && t.Tok == token.DEFINE && len(t.Rhs) == 1 && isTypeSwitchAssert(t.Rhs[0])
        }
        return false
 }
@@ -1640,39 +1650,65 @@ func (p *parser) parseSwitchStmt() ast.Stmt {
        defer p.closeScope()
 
        var s1, s2 ast.Stmt
+       var scope *ast.Scope // scope for variable declared in TypeSwitchGuard, if any
        if p.tok != token.LBRACE {
                prevLev := p.exprLev
                p.exprLev = -1
                if p.tok != token.SEMICOLON {
                        s2, _ = p.parseSimpleStmt(basic)
+                       scope = p.topScope
                }
                if p.tok == token.SEMICOLON {
                        p.next()
                        s1 = s2
                        s2 = nil
                        if p.tok != token.LBRACE {
+                               // A TypeSwitchGuard may declare a variable in addition
+                               // to the variable declared in the initial SimpleStmt.
+                               // Put it into an extra scope to avoid redeclaration
+                               // errors, as in:
+                               //
+                               //      switch t := 0; t := x.(T) { ... }
+                               //
+                               // (this code is still not valid Go because the first t
+                               // will never be used - but the extra scope is needed
+                               // for the correct error message).
+                               //
+                               // If we don't have a type switch, s2 must be an expression.
+                               // Having the extra (empty) scope here won't affect it.
+                               p.openScope()
                                s2, _ = p.parseSimpleStmt(basic)
+                               scope = p.topScope
+                               p.closeScope()
                        }
                }
                p.exprLev = prevLev
        }
 
-       exprSwitch := isExprSwitch(s2)
+       // If varName != "", we have a type switch. If varName != "_" it
+       // is the name of the variable declared by the TypeSwitchGuard.
+       varName := ""
+       if isTypeSwitchGuard(s2) {
+               varName = "_"
+               // If s2 declared a variable, there is exactly one.
+               for varName = range scope.Objects {
+               }
+       }
+
        lbrace := p.expect(token.LBRACE)
        var list []ast.Stmt
        for p.tok == token.CASE || p.tok == token.DEFAULT {
-               list = append(list, p.parseCaseClause(exprSwitch))
+               list = append(list, p.parseCaseClause(varName))
        }
        rbrace := p.expect(token.RBRACE)
        p.expectSemi()
        body := &ast.BlockStmt{lbrace, list, rbrace}
 
-       if exprSwitch {
-               return &ast.SwitchStmt{pos, s1, p.makeExpr(s2), body}
+       if varName != "" {
+               return &ast.TypeSwitchStmt{pos, s1, s2, body}
        }
-       // type switch
-       // TODO(gri): do all the checks!
-       return &ast.TypeSwitchStmt{pos, s1, s2, body}
+
+       return &ast.SwitchStmt{pos, s1, p.makeExpr(s2), body}
 }
 
 func (p *parser) parseCommClause() *ast.CommClause {
@@ -2001,14 +2037,12 @@ func (p *parser) parseReceiver(scope *ast.Scope) *ast.FieldList {
                defer un(trace(p, "Receiver"))
        }
 
-       pos := p.pos
        par := p.parseParameters(scope, false)
 
        // must have exactly one receiver
        if par.NumFields() != 1 {
-               p.errorExpected(pos, "exactly one receiver")
-               // TODO determine a better range for BadExpr below
-               par.List = []*ast.Field{&ast.Field{Type: &ast.BadExpr{pos, pos}}}
+               p.errorExpected(par.Opening, "exactly one receiver")
+               par.List = []*ast.Field{&ast.Field{Type: &ast.BadExpr{par.Opening, par.Closing + 1}}}
                return par
        }
 

diff --git a/src/pkg/go/parser/parser_test.go b/src/pkg/go/parser/parser_test.go
index 39a78e5156e79b8e64babc4fdf1c16777652aaf3..fb91dd1e7c1a1c0c2841f12bd32d2781bb7d3f83 100644 (file)
--- a/src/pkg/go/parser/parser_test.go
+++ b/src/pkg/go/parser/parser_test.go
@@ -26,6 +26,9 @@ var illegalInputs = []interface{}{
        `package p; func f() { for _ = range x ; ; {} };`,
        `package p; func f() { for ; ; _ = range x {} };`,
        `package p; func f() { for ; _ = range x ; {} };`,
+       `package p; func f() { switch t = t.(type) {} };`,
+       `package p; func f() { switch t, t = t.(type) {} };`,
+       `package p; func f() { switch t = t.(type), t {} };`,
        `package p; var a = [1]int; /* illegal expression */`,
        `package p; var a = [...]int; /* illegal expression */`,
        `package p; var a = struct{} /* illegal expression */`,