go/parser: accept all valid type parameter lists

This is a port of CL 402256 from the syntax package to go/parser
with adjustments because of the different AST structure, and
excluding any necessary go/printer changes (separate CL).

Type parameter lists starting with the form [name *T|...] or
[name (X)|...] may look like an array length expression [x].
Only after parsing the entire initial expression and checking
whether the expression contains type elements or is followed
by a comma can we make the final decision.

This change simplifies the existing parsing strategy: instead
of trying to make an upfront decision with limited information
(which is insufficient), the parser now parses the start of a
type parameter list or array length specification as expression.
In a second step, if the expression can be split into a name
followed by a type element, or a name followed by an ordinary
expression which is succeeded by a comma, we assume a type
parameter list (because it can't be an array length).
In all other cases we assume an array length specification.

Fixes #52559.

Change-Id: I11ab6e62b073b78b2331bb6063cf74d2a9eaa236
Reviewed-on: https://go-review.googlesource.com/c/go/+/403937
Run-TryBot: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

Author: griesemer

src/go/parser/parser.go

@@ -785,9 +785,9 @@ func (p *parser) parseParamDecl(name *ast.Ident, typeSetsOK bool) (f field) {
 		return // don't allow ...type "|" ...
 
 	default:
-		// TODO(rfindley): this looks incorrect in the case of type parameter
-		// lists.
-		p.errorExpected(p.pos, ")")
+		// TODO(rfindley): this is incorrect in the case of type parameter lists
+		//                 (should be "']'" in that case)
+		p.errorExpected(p.pos, "')'")
 		p.advance(exprEnd)
 	}
 
@@ -2592,10 +2592,12 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
 		defer un(trace(p, "TypeSpec"))
 	}
 
-	ident := p.parseIdent()
-	spec := &ast.TypeSpec{Doc: doc, Name: ident}
+	name := p.parseIdent()
+	spec := &ast.TypeSpec{Doc: doc, Name: name}
 
 	if p.tok == token.LBRACK && p.allowGenerics() {
+		// spec.Name "[" ...
+		// array/slice type or type parameter list
 		lbrack := p.pos
 		p.next()
 		if p.tok == token.IDENT {
@@ -2608,14 +2610,12 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
 			// with a "[" as in: P []E. In that case, simply parsing
 			// an expression would lead to an error: P[] is invalid.
 			// But since index or slice expressions are never constant
-			// and thus invalid array length expressions, if we see a
-			// "[" following a name it must be the start of an array
-			// or slice constraint. Only if we don't see a "[" do we
-			// need to parse a full expression.
-
-			// Index or slice expressions are never constant and thus invalid
-			// array length expressions. Thus, if we see a "[" following name
-			// we can safely assume that "[" name starts a type parameter list.
+			// and thus invalid array length expressions, if the name
+			// is followed by "[" it must be the start of an array or
+			// slice constraint. Only if we don't see a "[" do we
+			// need to parse a full expression. Notably, name <- x
+			// is not a concern because name <- x is a statement and
+			// not an expression.
 			var x ast.Expr = p.parseIdent()
 			if p.tok != token.LBRACK {
 				// To parse the expression starting with name, expand
@@ -2626,58 +2626,21 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
 				x = p.parseBinaryExpr(lhs, token.LowestPrec+1, false)
 				p.exprLev--
 			}
-
-			// analyze the cases
-			var pname *ast.Ident // pname != nil means pname is the type parameter name
-			var ptype ast.Expr   // ptype != nil means ptype is the type parameter type; pname != nil in this case
-
-			switch t := x.(type) {
-			case *ast.Ident:
-				// Unless we see a "]", we are at the start of a type parameter list.
-				if p.tok != token.RBRACK {
-					// d.Name "[" name ...
-					pname = t
-					// no ptype
-				}
-			case *ast.BinaryExpr:
-				// If we have an expression of the form name*T, and T is a (possibly
-				// parenthesized) type literal or the next token is a comma, we are
-				// at the start of a type parameter list.
-				if name, _ := t.X.(*ast.Ident); name != nil {
-					if t.Op == token.MUL && (isTypeLit(t.Y) || p.tok == token.COMMA) {
-						// d.Name "[" name "*" t.Y
-						// d.Name "[" name "*" t.Y ","
-						// convert t into unary *t.Y
-						pname = name
-						ptype = &ast.StarExpr{Star: t.OpPos, X: t.Y}
-					}
-				}
-				if pname == nil {
-					// A normal binary expression. Since we passed check=false, we must
-					// now check its operands.
-					p.checkBinaryExpr(t)
-				}
-			case *ast.CallExpr:
-				// If we have an expression of the form name(T), and T is a (possibly
-				// parenthesized) type literal or the next token is a comma, we are
-				// at the start of a type parameter list.
-				if name, _ := t.Fun.(*ast.Ident); name != nil {
-					if len(t.Args) == 1 && !t.Ellipsis.IsValid() && (isTypeLit(t.Args[0]) || p.tok == token.COMMA) {
-						// d.Name "[" name "(" t.ArgList[0] ")"
-						// d.Name "[" name "(" t.ArgList[0] ")" ","
-						pname = name
-						ptype = t.Args[0]
-					}
-				}
-			}
-
-			if pname != nil {
-				// d.Name "[" pname ...
-				// d.Name "[" pname ptype ...
-				// d.Name "[" pname ptype "," ...
-				p.parseGenericType(spec, lbrack, pname, ptype)
+			// Analyze expression x. If we can split x into a type parameter
+			// name, possibly followed by a type parameter type, we consider
+			// this the start of a type parameter list, with some caveats:
+			// a single name followed by "]" tilts the decision towards an
+			// array declaration; a type parameter type that could also be
+			// an ordinary expression but which is followed by a comma tilts
+			// the decision towards a type parameter list.
+			if pname, ptype := extractName(x, p.tok == token.COMMA); pname != nil && (ptype != nil || p.tok != token.RBRACK) {
+				// spec.Name "[" pname ...
+				// spec.Name "[" pname ptype ...
+				// spec.Name "[" pname ptype "," ...
+				p.parseGenericType(spec, lbrack, pname, ptype) // ptype may be nil
 			} else {
-				// d.Name "[" x ...
+				// spec.Name "[" pname "]" ...
+				// spec.Name "[" x ...
 				spec.Type = p.parseArrayType(lbrack, x)
 			}
 		} else {
@@ -2700,17 +2663,66 @@ func (p *parser) parseTypeSpec(doc *ast.CommentGroup, _ token.Pos, _ token.Token
 	return spec
 }
 
-// isTypeLit reports whether x is a (possibly parenthesized) type literal.
-func isTypeLit(x ast.Expr) bool {
+// extractName splits the expression x into (name, expr) if syntactically
+// x can be written as name expr. The split only happens if expr is a type
+// element (per the isTypeElem predicate) or if force is set.
+// If x is just a name, the result is (name, nil). If the split succeeds,
+// the result is (name, expr). Otherwise the result is (nil, x).
+// Examples:
+//
+//	x           force    name    expr
+//	------------------------------------
+//	P*[]int     T/F      P       *[]int
+//	P*E         T        P       *E
+//	P*E         F        nil     P*E
+//	P([]int)    T/F      P       []int
+//	P(E)        T        P       E
+//	P(E)        F        nil     P(E)
+//	P*E|F|~G    T/F      P       *E|F|~G
+//	P*E|F|G     T        P       *E|F|G
+//	P*E|F|G     F        nil     P*E|F|G
+func extractName(x ast.Expr, force bool) (*ast.Ident, ast.Expr) {
+	switch x := x.(type) {
+	case *ast.Ident:
+		return x, nil
+	case *ast.BinaryExpr:
+		switch x.Op {
+		case token.MUL:
+			if name, _ := x.X.(*ast.Ident); name != nil && (force || isTypeElem(x.Y)) {
+				// x = name *x.Y
+				return name, &ast.StarExpr{Star: x.OpPos, X: x.Y}
+			}
+		case token.OR:
+			if name, lhs := extractName(x.X, force || isTypeElem(x.Y)); name != nil && lhs != nil {
+				// x = name lhs|x.Y
+				op := *x
+				op.X = lhs
+				return name, &op
+			}
+		}
+	case *ast.CallExpr:
+		if name, _ := x.Fun.(*ast.Ident); name != nil {
+			if len(x.Args) == 1 && x.Ellipsis == token.NoPos && (force || isTypeElem(x.Args[0])) {
+				// x = name "(" x.ArgList[0] ")"
+				return name, x.Args[0]
+			}
+		}
+	}
+	return nil, x
+}
+
+// isTypeElem reports whether x is a (possibly parenthesized) type element expression.
+// The result is false if x could be a type element OR an ordinary (value) expression.
+func isTypeElem(x ast.Expr) bool {
 	switch x := x.(type) {
 	case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, *ast.MapType, *ast.ChanType:
 		return true
-	case *ast.StarExpr:
-		// *T may be a pointer dereferenciation.
-		// Only consider *T as type literal if T is a type literal.
-		return isTypeLit(x.X)
+	case *ast.BinaryExpr:
+		return isTypeElem(x.X) || isTypeElem(x.Y)
+	case *ast.UnaryExpr:
+		return x.Op == token.TILDE
 	case *ast.ParenExpr:
-		return isTypeLit(x.X)
+		return isTypeElem(x.X)
 	}
 	return false
 }

src/go/parser/testdata/issue49482.go2

@@ -29,7 +29,6 @@ type (
         _ [P*T-T, /* ERROR "unexpected comma" */ ]struct{}
         _ [10, /* ERROR "unexpected comma" */ ]struct{}
 
-        // These should be parsed as generic type declarations.
-        _[P *struct /* ERROR "expected expression" */ {}|int] struct{}
-        _[P *struct /* ERROR "expected expression" */ {}|int|string] struct{}
+        _[P *struct{}|int] struct{}
+        _[P *struct{}|int|string] struct{}
 )

src/go/parser/testdata/tparams.go2

@@ -0,0 +1,47 @@
+// Copyright 2020 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
+
+type _[a /* ERROR "all type parameters must be named" */, b] struct{}
+type _[a t, b t, c /* ERROR "all type parameters must be named" */ ] struct{}
+type _ struct {
+	t [n]byte
+	t[a]
+	t[a, b]
+}
+type _ interface {
+	t[a]
+	m[ /* ERROR "method must have no type parameters" */ _ _, /* ERROR mixed */ _]()
+	t[a, b]
+}
+
+func _[] /* ERROR "empty type parameter list" */ ()
+func _[a /* ERROR "all type parameters must be named" */, b ]()
+func _[a t, b t, c /* ERROR "all type parameters must be named" */ ]()
+
+// TODO(rfindley) incorrect error message (see existing TODO in parser)
+func f[a b, 0 /* ERROR "expected '\)', found 0" */ ] ()
+
+// issue #49482
+type (
+	_[a *[]int] struct{}
+	_[a *t,] struct{}
+	_[a *t|[]int] struct{}
+	_[a *t|t,] struct{}
+	_[a *t|~t,] struct{}
+	_[a *struct{}|t] struct{}
+	_[a *t|struct{}] struct{}
+	_[a *struct{}|~t] struct{}
+)
+
+// issue #51488
+type (
+	_[a *t|t,] struct{}
+	_[a *t|t, b t] struct{}
+	_[a *t|t] struct{}
+	_[a *[]t|t] struct{}
+	_[a ([]t)] struct{}
+	_[a ([]t)|t] struct{}
+)

src/go/printer/testdata/generics.golden

@@ -48,7 +48,7 @@ type _[P T] struct{}
 type _[P T, _ any] struct{}
 
 type _[P *struct{}] struct{}
-type _[P *struct{}] struct{}
+type _ [P(*struct{})]struct{}
 type _[P []int] struct{}
 
 // array type declarations

src/go/types/testdata/fixedbugs/issue49482.go

@@ -2,9 +2,6 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// This file is tested when running "go test -run Manual"
-// without source arguments. Use for one-off debugging.
-
 package p
 
 // The following is OK, per the special handling for type literals discussed in issue #49482.
@@ -14,12 +11,16 @@ type _[P (*int),] int
 
 const P = 2 // declare P to avoid noisy 'undeclared name' errors below.
 
-// The following parse as invalid array types.
-type _[P *int /* ERROR "int \(type\) is not an expression" */ ] int
-type _[P /* ERROR non-function P */ (*int)] int
+// The following parse as invalid array types due to parsing ambiguitiues.
+type _ [P *int /* ERROR "int \(type\) is not an expression" */ ]int
+type _ [P /* ERROR non-function P */ (*int)]int
 
-// The following should be parsed as a generic type, but is instead parsed as an array type.
-type _[P *struct /* ERROR "expected expression" */ {}| int /* ERROR "not an expression" */ ] struct{}
+// Adding a trailing comma or an enclosing interface resolves the ambiguity.
+type _[P *int,] int
+type _[P (*int),] int
+type _[P interface{*int}] int
+type _[P interface{(*int)}] int
 
-// The following fails to parse, due to the '~'
-type _[P *struct /* ERROR "expected expression" */ {}|~int /* ERROR "not an expression" */ ] struct{}
+// The following parse correctly as valid generic types.
+type _[P *struct{} | int] struct{}
+type _[P *struct{} | ~int] struct{}