gofmt/go/parser: strengthen syntax checks

- don't allow parenthesized receiver base types or anonymous fields
- fixed a couple of other omissions
- adjusted gofmt test script
- removed several TODOs

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

2 files changed, 66 insertions, 64 deletions

diff --git a/src/cmd/gofmt/test.sh b/src/cmd/gofmt/test.sh
index d2b7752c72..a8309421a7 100755
--- a/src/cmd/gofmt/test.sh
+++ b/src/cmd/gofmt/test.sh
@@ -41,7 +41,7 @@ apply1() {
 	bug106.go | bug121.go | bug125.go | bug133.go | bug160.go | \
 	bug163.go | bug166.go | bug169.go | bug217.go | bug222.go | \
 	bug226.go | bug228.go | bug248.go | bug274.go | bug280.go | \
-	bug282.go | bug287.go ) return ;;
+	bug282.go | bug287.go | bug298.go | bug299.go | bug300.go ) return ;;
 	esac
 	# the following directories are skipped because they contain test
 	# cases for syntax errors and thus won't parse in the first place:
diff --git a/src/pkg/go/parser/parser.go b/src/pkg/go/parser/parser.go
index 56096013c1..a492e738f7 100644
--- a/src/pkg/go/parser/parser.go
+++ b/src/pkg/go/parser/parser.go
@@ -508,19 +508,8 @@ func (p *parser) parseFieldDecl() *ast.Field {
 
 	doc := p.leadComment
 
-	// a list of identifiers looks like a list of type names
-	var list vector.Vector
-	for {
-		// TODO(gri): do not allow ()'s here
-		list.Push(p.parseType())
-		if p.tok != token.COMMA {
-			break
-		}
-		p.next()
-	}
-
-	// if we had a list of identifiers, it must be followed by a type
-	typ := p.tryType()
+	// fields
+	list, typ := p.parseVarList(false)
 
 	// optional tag
 	var tag *ast.BasicLit
@@ -533,15 +522,14 @@ func (p *parser) parseFieldDecl() *ast.Field {
 	var idents []*ast.Ident
 	if typ != nil {
 		// IdentifierList Type
-		idents = p.makeIdentList(&list)
+		idents = p.makeIdentList(list)
 	} else {
-		// Type (anonymous field)
-		if len(list) == 1 {
-			// TODO(gri): check that this looks like a type
-			typ = list.At(0).(ast.Expr)
-		} else {
-			p.errorExpected(p.pos, "anonymous field")
-			typ = &ast.BadExpr{p.pos}
+		// ["*"] TypeName (AnonymousField)
+		typ = (*list)[0].(ast.Expr) // we always have at least one element
+		if len(*list) > 1 || !isTypeName(deref(typ)) {
+			pos := typ.Pos()
+			p.errorExpected(pos, "anonymous field")
+			typ = &ast.BadExpr{pos}
 		}
 	}
 
@@ -559,7 +547,10 @@ func (p *parser) parseStructType() *ast.StructType {
 	pos := p.expect(token.STRUCT)
 	lbrace := p.expect(token.LBRACE)
 	var list vector.Vector
-	for p.tok == token.IDENT || p.tok == token.MUL {
+	for p.tok == token.IDENT || p.tok == token.MUL || p.tok == token.LPAREN {
+		// a field declaration cannot start with a '(' but we accept
+		// it here for more robust parsing and better error messages
+		// (parseFieldDecl will check and complain if necessary)
 		list.Push(p.parseFieldDecl())
 	}
 	rbrace := p.expect(token.RBRACE)
@@ -589,8 +580,8 @@ func (p *parser) parsePointerType() *ast.StarExpr {
 }
 
 
-func (p *parser) tryParameterType(ellipsisOk bool) ast.Expr {
-	if ellipsisOk && p.tok == token.ELLIPSIS {
+func (p *parser) tryVarType(isParam bool) ast.Expr {
+	if isParam && p.tok == token.ELLIPSIS {
 		pos := p.pos
 		p.next()
 		typ := p.tryType() // don't use parseType so we can provide better error message
@@ -607,8 +598,8 @@ func (p *parser) tryParameterType(ellipsisOk bool) ast.Expr {
 }
 
 
-func (p *parser) parseParameterType(ellipsisOk bool) ast.Expr {
-	typ := p.tryParameterType(ellipsisOk)
+func (p *parser) parseVarType(isParam bool) ast.Expr {
+	typ := p.tryVarType(isParam)
 	if typ == nil {
 		p.errorExpected(p.pos, "type")
 		p.next() // make progress
@@ -618,16 +609,19 @@ func (p *parser) parseParameterType(ellipsisOk bool) ast.Expr {
 }
 
 
-func (p *parser) parseParameterDecl(ellipsisOk bool) (*vector.Vector, ast.Expr) {
+func (p *parser) parseVarList(isParam bool) (*vector.Vector, ast.Expr) {
 	if p.trace {
-		defer un(trace(p, "ParameterDecl"))
+		defer un(trace(p, "VarList"))
 	}
 
 	// a list of identifiers looks like a list of type names
 	var list vector.Vector
 	for {
-		// TODO(gri): do not allow ()'s here
-		list.Push(p.parseParameterType(ellipsisOk))
+		// parseVarType accepts any type (including parenthesized ones)
+		// even though the syntax does not permit them here: we
+		// accept them all for more robust parsing and complain
+		// afterwards
+		list.Push(p.parseVarType(isParam))
 		if p.tok != token.COMMA {
 			break
 		}
@@ -635,7 +629,7 @@ func (p *parser) parseParameterDecl(ellipsisOk bool) (*vector.Vector, ast.Expr)
 	}
 
 	// if we had a list of identifiers, it must be followed by a type
-	typ := p.tryParameterType(ellipsisOk)
+	typ := p.tryVarType(isParam)
 
 	return &list, typ
 }
@@ -646,7 +640,7 @@ func (p *parser) parseParameterList(ellipsisOk bool) []*ast.Field {
 		defer un(trace(p, "ParameterList"))
 	}
 
-	list, typ := p.parseParameterDecl(ellipsisOk)
+	list, typ := p.parseVarList(ellipsisOk)
 	if typ != nil {
 		// IdentifierList Type
 		idents := p.makeIdentList(list)
@@ -658,7 +652,7 @@ func (p *parser) parseParameterList(ellipsisOk bool) []*ast.Field {
 
 		for p.tok != token.RPAREN && p.tok != token.EOF {
 			idents := p.parseIdentList(ast.Var)
-			typ := p.parseParameterType(ellipsisOk)
+			typ := p.parseVarType(ellipsisOk)
 			list.Push(&ast.Field{nil, idents, typ, nil, nil})
 			if p.tok != token.COMMA {
 				break
@@ -1119,21 +1113,16 @@ func (p *parser) parseCompositeLit(typ ast.Expr) ast.Expr {
 }
 
 
-// TODO(gri): Consider different approach to checking syntax after parsing:
-//            Provide a arguments (set of flags) to parsing functions
-//            restricting what they are supposed to accept depending
-//            on context.
-
 // checkExpr checks that x is an expression (and not a type).
 func (p *parser) checkExpr(x ast.Expr) ast.Expr {
-	// TODO(gri): should provide predicate in AST nodes
-	switch t := x.(type) {
+	switch t := unparen(x).(type) {
 	case *ast.BadExpr:
 	case *ast.Ident:
 	case *ast.BasicLit:
 	case *ast.FuncLit:
 	case *ast.CompositeLit:
 	case *ast.ParenExpr:
+		panic("unreachable")
 	case *ast.SelectorExpr:
 	case *ast.IndexExpr:
 	case *ast.SliceExpr:
@@ -1161,16 +1150,14 @@ func (p *parser) checkExpr(x ast.Expr) ast.Expr {
 }
 
 
-// isTypeName returns true iff x is type name.
+// isTypeName returns true iff x is a (qualified) TypeName.
 func isTypeName(x ast.Expr) bool {
-	// TODO(gri): should provide predicate in AST nodes
 	switch t := x.(type) {
 	case *ast.BadExpr:
 	case *ast.Ident:
-	case *ast.ParenExpr:
-		return isTypeName(t.X) // TODO(gri): should (TypeName) be illegal?
 	case *ast.SelectorExpr:
-		return isTypeName(t.X)
+		_, isIdent := t.X.(*ast.Ident)
+		return isIdent
 	default:
 		return false // all other nodes are not type names
 	}
@@ -1178,16 +1165,14 @@ func isTypeName(x ast.Expr) bool {
 }
 
 
-// isCompositeLitType returns true iff x is a legal composite literal type.
-func isCompositeLitType(x ast.Expr) bool {
-	// TODO(gri): should provide predicate in AST nodes
+// isLiteralType returns true iff x is a legal composite literal type.
+func isLiteralType(x ast.Expr) bool {
 	switch t := x.(type) {
 	case *ast.BadExpr:
 	case *ast.Ident:
-	case *ast.ParenExpr:
-		return isCompositeLitType(t.X)
 	case *ast.SelectorExpr:
-		return isTypeName(t.X)
+		_, isIdent := t.X.(*ast.Ident)
+		return isIdent
 	case *ast.ArrayType:
 	case *ast.StructType:
 	case *ast.MapType:
@@ -1198,12 +1183,31 @@ func isCompositeLitType(x ast.Expr) bool {
 }
 
 
+// If x is of the form *T, deref returns T, otherwise it returns x.
+func deref(x ast.Expr) ast.Expr {
+	if p, isPtr := x.(*ast.StarExpr); isPtr {
+		x = p.X
+	}
+	return x
+}
+
+
+// If x is of the form (T), unparen returns unparen(T), otherwise it returns x.
+func unparen(x ast.Expr) ast.Expr {
+	if p, isParen := x.(*ast.ParenExpr); isParen {
+		x = unparen(p.X)
+	}
+	return x
+}
+
+
 // checkExprOrType checks that x is an expression or a type
 // (and not a raw type such as [...]T).
 //
 func (p *parser) checkExprOrType(x ast.Expr) ast.Expr {
-	// TODO(gri): should provide predicate in AST nodes
-	switch t := x.(type) {
+	switch t := unparen(x).(type) {
+	case *ast.ParenExpr:
+		panic("unreachable")
 	case *ast.UnaryExpr:
 		if t.Op == token.RANGE {
 			// the range operator is only allowed at the top of a for statement
@@ -1238,7 +1242,7 @@ L:
 		case token.LPAREN:
 			x = p.parseCallOrConversion(p.checkExprOrType(x))
 		case token.LBRACE:
-			if isCompositeLitType(x) && (p.exprLev >= 0 || !isTypeName(x)) {
+			if isLiteralType(x) && (p.exprLev >= 0 || !isTypeName(x)) {
 				x = p.parseCompositeLit(x)
 			} else {
 				break L
@@ -1919,7 +1923,7 @@ func parseVarSpec(p *parser, doc *ast.CommentGroup) ast.Spec {
 
 func (p *parser) parseGenDecl(keyword token.Token, f parseSpecFunction) *ast.GenDecl {
 	if p.trace {
-		defer un(trace(p, keyword.String()+"Decl"))
+		defer un(trace(p, "GenDecl("+keyword.String()+")"))
 	}
 
 	doc := p.leadComment
@@ -1960,17 +1964,15 @@ func (p *parser) parseReceiver(scope *ast.Scope) *ast.FieldList {
 	if par.NumFields() != 1 {
 		p.errorExpected(pos, "exactly one receiver")
 		par.List = []*ast.Field{&ast.Field{Type: &ast.BadExpr{noPos}}}
+		return par
 	}
 
+	// recv type must be of the form ["*"] identifier
 	recv := par.List[0]
-
-	// recv type must be TypeName or *TypeName
-	base := recv.Type
-	if ptr, isPtr := base.(*ast.StarExpr); isPtr {
-		base = ptr.X
-	}
-	if !isTypeName(base) {
-		p.errorExpected(base.Pos(), "type name")
+	base := deref(recv.Type)
+	if _, isIdent := base.(*ast.Ident); !isIdent {
+		p.errorExpected(base.Pos(), "(unqualified) identifier")
+		par.List = []*ast.Field{&ast.Field{Type: &ast.BadExpr{recv.Pos()}}}
 	}
 
 	return par