go/parser: better error messages for incorrect type parameter list

This is a port of CL 538856 from the syntax parser to go/parser.
As part of the port, make more portions of parseParameterList
matching the equivalent paramList method (from the syntax parser).
As a result, this now also produces a better error message in cases
where the missing piece might not be a type parameter name but a
constraint (this fixes a TODO in a test).

Improve comments in the code and adjust the corresponding comments
in the syntax parser.

Change references to issues to use the format go.dev/issue/ddddd.

For #60812.

Change-Id: Ia243bd78161ed8543d3dc5deb20ca4a215c5b1e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/538858
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/syntax/parser.go

@@ -2021,7 +2021,7 @@ func (p *parser) paramList(name *Name, typ Expr, close token, requireNames bool)
 
 	// distribute parameter types (len(list) > 0)
 	if named == 0 && !requireNames {
-		// all unnamed => found names are named types
+		// all unnamed and we're not in a type parameter list => found names are named types
 		for _, par := range list {
 			if typ := par.Name; typ != nil {
 				par.Type = typ
@@ -2029,32 +2029,38 @@ func (p *parser) paramList(name *Name, typ Expr, close token, requireNames bool)
 			}
 		}
 	} else if named != len(list) {
-		// some named => all must have names and types
+		// some named or we're in a type parameter list => all must be named
-		var pos Pos  // left-most error position (or unknown)
+		var errPos Pos // left-most error position (or unknown)
-		var typ Expr // current type (from right to left)
+		var typ Expr   // current type (from right to left)
 		for i := len(list) - 1; i >= 0; i-- {
 			par := list[i]
 			if par.Type != nil {
 				typ = par.Type
 				if par.Name == nil {
-					pos = StartPos(typ)
+					errPos = StartPos(typ)
-					par.Name = NewName(pos, "_")
+					par.Name = NewName(errPos, "_")
 				}
 			} else if typ != nil {
 				par.Type = typ
 			} else {
 				// par.Type == nil && typ == nil => we only have a par.Name
-				pos = par.Name.Pos()
+				errPos = par.Name.Pos()
 				t := p.badExpr()
-				t.pos = pos // correct position
+				t.pos = errPos // correct position
 				par.Type = t
 			}
 		}
-		if pos.IsKnown() {
+		if errPos.IsKnown() {
 			var msg string
 			if requireNames {
+				// Not all parameters are named because named != len(list).
+				// If named == typed we must have parameters that have no types,
+				// and they must be at the end of the parameter list, otherwise
+				// the types would have been filled in by the right-to-left sweep
+				// above and we wouldn't have an error. Since we are in a type
+				// parameter list, the missing types are constraints.
 				if named == typed {
-					pos = end // position error at closing ]
+					errPos = end // position error at closing ]
 					msg = "missing type constraint"
 				} else {
 					msg = "missing type parameter name"
@@ -2066,7 +2072,7 @@ func (p *parser) paramList(name *Name, typ Expr, close token, requireNames bool)
 			} else {
 				msg = "mixed named and unnamed parameters"
 			}
-			p.syntaxErrorAt(pos, msg)
+			p.syntaxErrorAt(errPos, msg)
 		}
 	}
 

src/go/parser/parser.go

@@ -880,26 +880,26 @@ func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing tok
 
 	// Type parameters are the only parameter list closed by ']'.
 	tparams := closing == token.RBRACK
-	// Type set notation is ok in type parameter lists.
-	typeSetsOK := tparams
 
-	pos := p.pos
+	// Note: The code below matches the corresponding code in the syntax
-	if name0 != nil {
+	//       parser closely. Changes must be reflected in either parser.
-		pos = name0.Pos()
+	//       For the code to match, we use the local []field list that
-	}
+	//       corresponds to []syntax.Field. At the end, the list must be
+	//       converted into an []*ast.Field.
 
 	var list []field
 	var named int // number of parameters that have an explicit name and type
+	var typed int // number of parameters that have an explicit type
 
 	for name0 != nil || p.tok != closing && p.tok != token.EOF {
 		var par field
 		if typ0 != nil {
-			if typeSetsOK {
+			if tparams {
 				typ0 = p.embeddedElem(typ0)
 			}
 			par = field{name0, typ0}
 		} else {
-			par = p.parseParamDecl(name0, typeSetsOK)
+			par = p.parseParamDecl(name0, tparams)
 		}
 		name0 = nil // 1st name was consumed if present
 		typ0 = nil  // 1st typ was consumed if present
@@ -908,6 +908,9 @@ func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing tok
 			if par.name != nil && par.typ != nil {
 				named++
 			}
+			if par.typ != nil {
+				typed++
+			}
 		}
 		if !p.atComma("parameter list", closing) {
 			break
@@ -919,12 +922,9 @@ func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing tok
 		return // not uncommon
 	}
 
-	// TODO(gri) parameter distribution and conversion to []*ast.Field
+	// distribute parameter types (len(list) > 0)
-	//           can be combined and made more efficient
+	if named == 0 && !tparams {
-
+		// all unnamed and we're not in a type parameter list => found names are type names
-	// distribute parameter types
-	if named == 0 {
-		// all unnamed => found names are type names
 		for i := 0; i < len(list); i++ {
 			par := &list[i]
 			if typ := par.name; typ != nil {
@@ -932,43 +932,55 @@ func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing tok
 				par.name = nil
 			}
 		}
-		if tparams {
-			p.error(pos, "type parameters must be named")
-		}
 	} else if named != len(list) {
-		// some named => all must be named
+		// some named or we're in a type parameter list => all must be named
-		ok := true
+		var errPos token.Pos // left-most error position (or invalid)
-		var typ ast.Expr
+		var typ ast.Expr     // current type (from right to left)
-		missingName := pos
 		for i := len(list) - 1; i >= 0; i-- {
 			if par := &list[i]; par.typ != nil {
 				typ = par.typ
 				if par.name == nil {
-					ok = false
+					errPos = typ.Pos()
-					missingName = par.typ.Pos()
 					n := ast.NewIdent("_")
-					n.NamePos = typ.Pos() // correct position
+					n.NamePos = errPos // correct position
 					par.name = n
 				}
 			} else if typ != nil {
 				par.typ = typ
 			} else {
 				// par.typ == nil && typ == nil => we only have a par.name
-				ok = false
+				errPos = par.name.Pos()
-				missingName = par.name.Pos()
+				par.typ = &ast.BadExpr{From: errPos, To: p.pos}
-				par.typ = &ast.BadExpr{From: par.name.Pos(), To: p.pos}
 			}
 		}
-		if !ok {
+		if errPos.IsValid() {
+			var msg string
 			if tparams {
-				p.error(missingName, "type parameters must be named")
+				// Not all parameters are named because named != len(list).
+				// If named == typed we must have parameters that have no types,
+				// and they must be at the end of the parameter list, otherwise
+				// the types would have been filled in by the right-to-left sweep
+				// above and we wouldn't have an error. Since we are in a type
+				// parameter list, the missing types are constraints.
+				if named == typed {
+					errPos = p.pos // position error at closing ]
+					msg = "missing type constraint"
+				} else {
+					msg = "missing type parameter name"
+					// go.dev/issue/60812
+					if len(list) == 1 {
+						msg += " or invalid array length"
+					}
+				}
 			} else {
-				p.error(pos, "mixed named and unnamed parameters")
+				msg = "mixed named and unnamed parameters"
 			}
+			p.error(errPos, msg)
 		}
 	}
 
-	// convert list []*ast.Field
+	// Convert list to []*ast.Field.
+	// If list contains types only, each type gets its own ast.Field.
 	if named == 0 {
 		// parameter list consists of types only
 		for _, par := range list {
@@ -978,7 +990,8 @@ func (p *parser) parseParameterList(name0 *ast.Ident, typ0 ast.Expr, closing tok
 		return
 	}
 
-	// parameter list consists of named parameters with types
+	// If the parameter list consists of named parameters with types,
+	// collect all names with the same types into a single ast.Field.
 	var names []*ast.Ident
 	var typ ast.Expr
 	addParams := func() {
@@ -1545,7 +1558,7 @@ func (p *parser) parseIndexOrSliceOrInstance(x ast.Expr) ast.Expr {
 		if ncolons == 2 {
 			slice3 = true
 			// Check presence of middle and final index here rather than during type-checking
-			// to prevent erroneous programs from passing through gofmt (was issue 7305).
+			// to prevent erroneous programs from passing through gofmt (was go.dev/issue/7305).
 			if index[1] == nil {
 				p.error(colons[0], "middle index required in 3-index slice")
 				index[1] = &ast.BadExpr{From: colons[0] + 1, To: colons[1]}
@@ -2534,7 +2547,7 @@ func (p *parser) parseGenericType(spec *ast.TypeSpec, openPos token.Pos, name0 *
 	closePos := p.expect(token.RBRACK)
 	spec.TypeParams = &ast.FieldList{Opening: openPos, List: list, Closing: closePos}
 	// Let the type checker decide whether to accept type parameters on aliases:
-	// see issue #46477.
+	// see go.dev/issue/46477.
 	if p.tok == token.ASSIGN {
 		// type alias
 		spec.Assign = p.pos

src/go/parser/parser_test.go

@@ -319,7 +319,7 @@ const pi = 3.1415
 /* 3a */ // 3b
 /* 3c */ const e = 2.7182
 
-// Example from issue 3139
+// Example from go.dev/issue/3139
 func ExampleCount() {
 	fmt.Println(strings.Count("cheese", "e"))
 	fmt.Println(strings.Count("five", "")) // before & after each rune
@@ -335,7 +335,7 @@ func ExampleCount() {
 		{"/* 1a */", "/* 1b */", "/* 1c */", "// 1d"},
 		{"/* 2a\n*/", "// 2b"},
 		{"/* 3a */", "// 3b", "/* 3c */"},
-		{"// Example from issue 3139"},
+		{"// Example from go.dev/issue/3139"},
 		{"// before & after each rune"},
 		{"// Output:", "// 3", "// 5"},
 	}
@@ -735,7 +735,7 @@ func TestScopeDepthLimit(t *testing.T) {
 	}
 }
 
-// proposal #50429
+// proposal go.dev/issue/50429
 func TestRangePos(t *testing.T) {
 	testcases := []string{
 		"package p; func _() { for range x {} }",

src/go/parser/resolver.go

@@ -136,7 +136,7 @@ func (r *resolver) declare(decl, data any, scope *ast.Scope, kind ast.ObjKind, i
 		obj.Decl = decl
 		obj.Data = data
 		// Identifiers (for receiver type parameters) are written to the scope, but
-		// never set as the resolved object. See issue #50956.
+		// never set as the resolved object. See go.dev/issue/50956.
 		if _, ok := decl.(*ast.Ident); !ok {
 			ident.Obj = obj
 		}
@@ -209,7 +209,7 @@ func (r *resolver) resolve(ident *ast.Ident, collectUnresolved bool) {
 			}
 			assert(obj.Name != "", "obj with no name")
 			// Identifiers (for receiver type parameters) are written to the scope,
-			// but never set as the resolved object. See issue #50956.
+			// but never set as the resolved object. See go.dev/issue/50956.
 			if _, ok := obj.Decl.(*ast.Ident); !ok {
 				ident.Obj = obj
 			}
@@ -285,7 +285,7 @@ func (r *resolver) Visit(node ast.Node) ast.Visitor {
 		}
 		for _, e := range n.Elts {
 			if kv, _ := e.(*ast.KeyValueExpr); kv != nil {
-				// See issue #45160: try to resolve composite lit keys, but don't
+				// See go.dev/issue/45160: try to resolve composite lit keys, but don't
 				// collect them as unresolved if resolution failed. This replicates
 				// existing behavior when resolving during parsing.
 				if ident, _ := kv.Key.(*ast.Ident); ident != nil {

src/go/parser/short_test.go

@@ -43,7 +43,7 @@ var valids = []string{
 	`package p; func _() { map[int]int{}[0]++; map[int]int{}[0] += 1 }`,
 	`package p; func _(x interface{f()}) { interface{f()}(x).f() }`,
 	`package p; func _(x chan int) { chan int(x) <- 0 }`,
-	`package p; const (x = 0; y; z)`, // issue 9639
+	`package p; const (x = 0; y; z)`, // go.dev/issue/9639
 	`package p; var _ = map[P]int{P{}:0, {}:1}`,
 	`package p; var _ = map[*P]int{&P{}:0, {}:1}`,
 	`package p; type T = int`,
@@ -172,21 +172,21 @@ var invalids = []string{
 	`package p; type _ struct{ *( /* ERROR "cannot parenthesize embedded type" */ int) }`,
 	`package p; type _ struct{ *( /* ERROR "cannot parenthesize embedded type" */ []byte) }`,
 
-	// issue 8656
+	// go.dev/issue/8656
 	`package p; func f() (a b string /* ERROR "missing ','" */ , ok bool)`,
 
-	// issue 9639
+	// go.dev/issue/9639
 	`package p; var x, y, z; /* ERROR "expected type" */`,
 
-	// issue 12437
+	// go.dev/issue/12437
 	`package p; var _ = struct { x int, /* ERROR "expected ';', found ','" */ }{};`,
 	`package p; var _ = struct { x int, /* ERROR "expected ';', found ','" */ y float }{};`,
 
-	// issue 11611
+	// go.dev/issue/11611
 	`package p; type _ struct { int, } /* ERROR "expected 'IDENT', found '}'" */ ;`,
 	`package p; type _ struct { int, float } /* ERROR "expected type, found '}'" */ ;`,
 
-	// issue 13475
+	// go.dev/issue/13475
 	`package p; func f() { if true {} else ; /* ERROR "expected if statement or block" */ }`,
 	`package p; func f() { if true {} else defer /* ERROR "expected if statement or block" */ f() }`,
 
@@ -195,8 +195,7 @@ var invalids = []string{
 	`package p; var _ func[ /* ERROR "must have no type parameters" */ T any](T)`,
 	`package p; func _[]/* ERROR "empty type parameter list" */()`,
 
-	// TODO(rfindley) a better location would be after the ']'
+	`package p; type _[A,] /* ERROR "missing type constraint" */ struct{ A }`,
-	`package p; type _[A /* ERROR "type parameters must be named" */ ,] struct{ A }`,
 
 	`package p; func _[type /* ERROR "found 'type'" */ P, *Q interface{}]()`,
 

src/go/parser/testdata/issue11377.src

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Test case for issue 11377: Better synchronization of
+// Test case for go.dev/issue/11377: Better synchronization of
 // parser after certain syntax errors.
 
 package p

src/go/parser/testdata/issue23434.src

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Test case for issue 23434: Better synchronization of
+// Test case for go.dev/issue/23434: Better synchronization of
 // parser after missing type. There should be exactly
 // one error each time, with now follow errors.
 

src/go/parser/testdata/issue3106.src

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Test case for issue 3106: Better synchronization of
+// Test case for go.dev/issue/3106: Better synchronization of
 // parser after certain syntax errors.
 
 package main

src/go/parser/testdata/issue34946.src

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Test case for issue 34946: Better synchronization of
+// Test case for go.dev/issue/34946: Better synchronization of
 // parser for function declarations that start their
 // body's opening { on a new line.
 

src/go/parser/testdata/issue44504.src

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Test case for issue 44504: panic due to duplicate resolution of slice/index
+// Test case for go.dev/issue/44504: panic due to duplicate resolution of slice/index
 // operands. We should not try to resolve a LHS expression with invalid syntax.
 
 package p

src/go/parser/testdata/issue49175.go2

@@ -10,4 +10,4 @@ type _[_ [1]t]t
 func _[_ []t]() {}
 func _[_ [1]t]() {}
 
-type t [t /* ERROR "type parameters must be named" */ [0]]t
+type t [t /* ERROR "missing type parameter name or invalid array length" */ [0]]t

src/go/parser/testdata/resolution/typeparams.go2

@@ -47,5 +47,5 @@ func f /* =@f */[T1 /* =@T1 */ interface{~[]T2 /* @T2 */}, T2 /* =@T2 */ any](
   var t1var /* =@t1var */ T1 /* @T1 */
 }
 
-// From issue #39634
+// From go.dev/issue/39634
 func(*ph1[e, e])h(d)

src/go/parser/testdata/tparams.go2

@@ -4,8 +4,8 @@
 
 package p
 
-type _[a /* ERROR "type parameters must be named" */, b] struct{}
+type _[a, b] /* ERROR "missing type constraint" */ struct{}
-type _[a t, b t, c /* ERROR "type parameters must be named" */ ] struct{}
+type _[a t, b t, c]  /* ERROR "missing type constraint" */ struct{}
 type _ struct {
 	t [n]byte
 	t[a]
@@ -25,13 +25,13 @@ type _ interface {
 }
 
 func _[] /* ERROR "empty type parameter list" */ ()
-func _[a /* ERROR "type parameters must be named" */, b ]()
+func _[a, b ] /* ERROR "missing type constraint" */ ()
-func _[a t, b t, c /* ERROR "type parameters must be named" */ ]()
+func _[a t, b t, c] /* ERROR "missing type constraint" */ ()
 
 // TODO(rfindley) incorrect error message (see existing TODO in parser)
 func f[a b, 0 /* ERROR "expected '\)', found 0" */ ] ()
 
-// issue #49482
+// go.dev/issue/49482
 type (
 	_[a *[]int] struct{}
 	_[a *t,] struct{}
@@ -43,7 +43,7 @@ type (
 	_[a *struct{}|~t] struct{}
 )
 
-// issue #51488
+// go.dev/issue/51488
 type (
 	_[a *t|t,] struct{}
 	_[a *t|t, b t] struct{}
@@ -52,3 +52,14 @@ type (
 	_[a ([]t)] struct{}
 	_[a ([]t)|t] struct{}
 )
+
+// go.dev/issue/60812
+type (
+	_ [t]struct{}
+	_ [[]t]struct{}
+	_ [[t]t]struct{}
+	_ [t /* ERROR "missing type parameter name or invalid array length" */ [t]]struct{}
+	_ [t t[t], t /* ERROR "missing type parameter name" */ [t]]struct{}
+	_ [t /* ERROR "missing type parameter name" */ [t], t t[t]]struct{}
+	_ [t /* ERROR "missing type parameter name" */ [t], t[t]]struct{} // report only first error
+)

src/go/parser/testdata/typeset.go2

@@ -61,12 +61,12 @@ type (
         _[~t|~t] t
 )
 
-type _[_ t, t /* ERROR "type parameters must be named" */ ] t
+type _[_ t, t] /* ERROR "missing type constraint" */ t
-type _[_ ~t, t /* ERROR "type parameters must be named" */ ] t
+type _[_ ~t, t] /* ERROR "missing type constraint" */ t
-type _[_ t, ~ /* ERROR "type parameters must be named" */ t] t
+type _[_ t, ~ /* ERROR "missing type parameter name" */ t] t
-type _[_ ~t, ~ /* ERROR "type parameters must be named" */ t] t
+type _[_ ~t, ~ /* ERROR "missing type parameter name" */ t] t
 
-type _[_ t|t, t /* ERROR "type parameters must be named" */ |t] t
+type _[_ t|t, t /* ERROR "missing type parameter name" */ |t] t
-type _[_ ~t|t, t /* ERROR "type parameters must be named" */ |t] t
+type _[_ ~t|t, t /* ERROR "missing type parameter name" */ |t] t
-type _[_ t|t, ~ /* ERROR "type parameters must be named" */ t|t] t
+type _[_ t|t, ~ /* ERROR "missing type parameter name" */ t|t] t
-type _[_ ~t|t, ~ /* ERROR "type parameters must be named" */ t|t] t
+type _[_ ~t|t, ~ /* ERROR "missing type parameter name" */ t|t] t