]> Cypherpunks.ru repositories - gostls13.git/commitdiff
go/parser: better error messages for incorrect type parameter list
authorRobert Griesemer <gri@golang.org>
Tue, 31 Oct 2023 21:22:05 +0000 (14:22 -0700)
committerGopher Robot <gobot@golang.org>
Thu, 2 Nov 2023 12:56:53 +0000 (12:56 +0000)
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>

14 files changed:
src/cmd/compile/internal/syntax/parser.go
src/go/parser/parser.go
src/go/parser/parser_test.go
src/go/parser/resolver.go
src/go/parser/short_test.go
src/go/parser/testdata/issue11377.src
src/go/parser/testdata/issue23434.src
src/go/parser/testdata/issue3106.src
src/go/parser/testdata/issue34946.src
src/go/parser/testdata/issue44504.src
src/go/parser/testdata/issue49175.go2
src/go/parser/testdata/resolution/typeparams.go2
src/go/parser/testdata/tparams.go2
src/go/parser/testdata/typeset.go2

index 140f00537a832f0a62a7ece59b2a1118c4ea85ea..1569b5e9872e1eab970c56ff81cd3675898319dc 100644 (file)
@@ -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
-               var pos Pos  // left-most error position (or unknown)
-               var typ Expr // current type (from right to left)
+               // some named or we're in a type parameter list => all must be named
+               var errPos Pos // left-most error position (or unknown)
+               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)
-                                       par.Name = NewName(pos, "_")
+                                       errPos = StartPos(typ)
+                                       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)
                }
        }
 
index 7d8f727b0c93e2ae09621b3c0f1b6142ca826450..a28960523eda818937d1c89c8ffe2d72785ee24d 100644 (file)
@@ -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
-       if name0 != nil {
-               pos = name0.Pos()
-       }
+       // Note: The code below matches the corresponding code in the syntax
+       //       parser closely. Changes must be reflected in either parser.
+       //       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
-       //           can be combined and made more efficient
-
-       // distribute parameter types
-       if named == 0 {
-               // all unnamed => found names are type names
+       // distribute parameter types (len(list) > 0)
+       if named == 0 && !tparams {
+               // all unnamed and we're not in a type parameter list => 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
-               ok := true
-               var typ ast.Expr
-               missingName := pos
+               // some named or we're in a type parameter list => all must be named
+               var errPos token.Pos // left-most error position (or invalid)
+               var typ ast.Expr     // current type (from right to left)
                for i := len(list) - 1; i >= 0; i-- {
                        if par := &list[i]; par.typ != nil {
                                typ = par.typ
                                if par.name == nil {
-                                       ok = false
-                                       missingName = par.typ.Pos()
+                                       errPos = 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
-                               missingName = par.name.Pos()
-                               par.typ = &ast.BadExpr{From: par.name.Pos(), To: p.pos}
+                               errPos = par.name.Pos()
+                               par.typ = &ast.BadExpr{From: errPos, 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
index 0848aca22a3e8bf2378070fc7e4d981f9f0c68e7..e72c03a3d47daeb55de03c6a5002e8ab03aca7e1 100644 (file)
@@ -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 {} }",
index 1539dcd5c7122e455b04d11ac07d46b45a06d4aa..d1e1834e53c6d5233badb909784a4bd31e9a1b76 100644 (file)
@@ -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 {
index f9575e1d0f2ffaa03bb6c4b41696c7c47b455fdf..3a34e8c216bf96f61b5ab59413f9145fa450701c 100644 (file)
@@ -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 "type parameters must be named" */ ,] struct{ A }`,
+       `package p; type _[A,] /* ERROR "missing type constraint" */ struct{ A }`,
 
        `package p; func _[type /* ERROR "found 'type'" */ P, *Q interface{}]()`,
 
index 1c438003eb8e069cc74027c3fbccafd9fb0ab3b9..a19b86e7abc844a338889e548a3466294a7e7c92 100644 (file)
@@ -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
index 24a0832347e20d78ff63c4fc0473ca7c1c3b15cb..f04ca1da894dd610b8ee06d24c4378c25d0e4a34 100644 (file)
@@ -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.
 
index 2db10be23549b96b55c4d5a14c71fe484cb59a45..37dfb2a52f5a5a444ec574a415d620e762794932 100644 (file)
@@ -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
index 6bb15e10c771b221cfb5df2475a4c854a6dd28c8..87b703d0c946a3ff05b016023aac4753331a7373 100644 (file)
@@ -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.
 
index 7791f4a8093660a0636c4e1e688037e8548cafdc..c46c79f7da9335e2119f4478cb357a8726ec0378 100644 (file)
@@ -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
index cf1c83c63398eac7462e12db2bd9543cb21d9811..df303ada406cf580fba295532f25e446bad1ca85 100644 (file)
@@ -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
index 7395ca2a34493cfaf93e9c4dd8795410f9fa5242..0f894d7c9391c96486dff6794d79219c0b8d96a9 100644 (file)
@@ -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)
index 1a9a6c635de53542e29c348b427cbc18944da34c..3293b559bb70abd06dd4559bb277be3a5909da45 100644 (file)
@@ -4,8 +4,8 @@
 
 package p
 
-type _[a /* ERROR "type parameters must be named" */, b] struct{}
-type _[a t, b t, c /* ERROR "type parameters must be named" */ ] struct{}
+type _[a, b] /* ERROR "missing type constraint" */ 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 t, b t, c /* ERROR "type parameters must be named" */ ]()
+func _[a, b ] /* ERROR "missing type constraint" */ ()
+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
+)
index 7844c22212298a306a5347b24078069cdc266e9e..3d90d766393cd599484aec29eff0e0d467128a32 100644 (file)
@@ -61,12 +61,12 @@ type (
         _[~t|~t] t
 )
 
-type _[_ t, t /* ERROR "type parameters must be named" */ ] t
-type _[_ ~t, t /* ERROR "type parameters must be named" */ ] t
-type _[_ t, ~ /* ERROR "type parameters must be named" */ t] t
-type _[_ ~t, ~ /* ERROR "type parameters must be named" */ t] t
+type _[_ t, t] /* ERROR "missing type constraint" */ t
+type _[_ ~t, t] /* ERROR "missing type constraint" */ t
+type _[_ t, ~ /* ERROR "missing type parameter name" */ 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 "type parameters must be named" */ |t] t
-type _[_ t|t, ~ /* ERROR "type parameters must be named" */ t|t] t
-type _[_ ~t|t, ~ /* ERROR "type parameters must be named" */ t|t] t
+type _[_ t|t, t /* ERROR "missing type parameter name" */ |t] t
+type _[_ ~t|t, t /* ERROR "missing type parameter name" */ |t] t
+type _[_ t|t, ~ /* ERROR "missing type parameter name" */ t|t] t
+type _[_ ~t|t, ~ /* ERROR "missing type parameter name" */ t|t] t