go/analysis/passes/composite: update for generics
Warn about unkeyed composite literals via literals of type parameter
type.
Updates golang/go#48704
Change-Id: Ia75139b56cb73288c133bd547d71664464015813
Reviewed-on: https://go-review.googlesource.com/c/tools/+/357756
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go
index 4c3ac66..025952e 100644
--- a/go/analysis/passes/composite/composite.go
+++ b/go/analysis/passes/composite/composite.go
@@ -14,6 +14,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/internal/typeparams"
)
const Doc = `check for unkeyed composite literals
@@ -67,41 +68,52 @@
// skip whitelisted types
return
}
- under := typ.Underlying()
- for {
- ptr, ok := under.(*types.Pointer)
- if !ok {
- break
+ terms, err := typeparams.StructuralTerms(typ)
+ if err != nil {
+ return // invalid type
+ }
+ for _, term := range terms {
+ under := deref(term.Type().Underlying())
+ if _, ok := under.(*types.Struct); !ok {
+ // skip non-struct composite literals
+ continue
}
- under = ptr.Elem().Underlying()
- }
- if _, ok := under.(*types.Struct); !ok {
- // skip non-struct composite literals
- return
- }
- if isLocalType(pass, typ) {
- // allow unkeyed locally defined composite literal
- return
- }
-
- // check if the CompositeLit contains an unkeyed field
- allKeyValue := true
- for _, e := range cl.Elts {
- if _, ok := e.(*ast.KeyValueExpr); !ok {
- allKeyValue = false
- break
+ if isLocalType(pass, term.Type()) {
+ // allow unkeyed locally defined composite literal
+ continue
}
- }
- if allKeyValue {
- // all the composite literal fields are keyed
+
+ // check if the CompositeLit contains an unkeyed field
+ allKeyValue := true
+ for _, e := range cl.Elts {
+ if _, ok := e.(*ast.KeyValueExpr); !ok {
+ allKeyValue = false
+ break
+ }
+ }
+ if allKeyValue {
+ // all the composite literal fields are keyed
+ continue
+ }
+
+ pass.ReportRangef(cl, "%s composite literal uses unkeyed fields", typeName)
return
}
-
- pass.ReportRangef(cl, "%s composite literal uses unkeyed fields", typeName)
})
return nil, nil
}
+func deref(typ types.Type) types.Type {
+ for {
+ ptr, ok := typ.(*types.Pointer)
+ if !ok {
+ break
+ }
+ typ = ptr.Elem().Underlying()
+ }
+ return typ
+}
+
func isLocalType(pass *analysis.Pass, typ types.Type) bool {
switch x := typ.(type) {
case *types.Struct:
@@ -112,6 +124,8 @@
case *types.Named:
// names in package foo are local to foo_test too
return strings.TrimSuffix(x.Obj().Pkg().Path(), "_test") == strings.TrimSuffix(pass.Pkg.Path(), "_test")
+ case *typeparams.TypeParam:
+ return strings.TrimSuffix(x.Obj().Pkg().Path(), "_test") == strings.TrimSuffix(pass.Pkg.Path(), "_test")
}
return false
}
diff --git a/go/analysis/passes/composite/composite_test.go b/go/analysis/passes/composite/composite_test.go
index c55015c..952de8b 100644
--- a/go/analysis/passes/composite/composite_test.go
+++ b/go/analysis/passes/composite/composite_test.go
@@ -9,9 +9,14 @@
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/composite"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
testdata := analysistest.TestData()
- analysistest.Run(t, testdata, composite.Analyzer, "a")
+ pkgs := []string{"a"}
+ if typeparams.Enabled {
+ pkgs = append(pkgs, "typeparams")
+ }
+ analysistest.Run(t, testdata, composite.Analyzer, pkgs...)
}
diff --git a/go/analysis/passes/composite/testdata/src/typeparams/lib/lib.go b/go/analysis/passes/composite/testdata/src/typeparams/lib/lib.go
new file mode 100644
index 0000000..9d7710d
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/typeparams/lib/lib.go
@@ -0,0 +1,9 @@
+// Copyright 2021 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 lib
+
+type Struct struct{ F int }
+type Slice []int
+type Map map[int]int
diff --git a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
new file mode 100644
index 0000000..dd5d57e
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
@@ -0,0 +1,27 @@
+// Copyright 2021 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 typeparams
+
+import "typeparams/lib"
+
+type localStruct struct { F int }
+
+func F[
+ T1 ~struct{ f int },
+ T2a localStruct,
+ T2b lib.Struct,
+ T3 ~[]int,
+ T4 lib.Slice,
+ T5 ~map[int]int,
+ T6 lib.Map,
+]() {
+ _ = T1{2}
+ _ = T2a{2}
+ _ = T2b{2} // want "unkeyed fields"
+ _ = T3{1,2}
+ _ = T4{1,2}
+ _ = T5{1:2}
+ _ = T6{1:2}
+}
diff --git a/internal/typeparams/normalize.go b/internal/typeparams/normalize.go
index ef3dbab..2937350 100644
--- a/internal/typeparams/normalize.go
+++ b/internal/typeparams/normalize.go
@@ -5,6 +5,7 @@
package typeparams
import (
+ "errors"
"fmt"
"go/types"
"os"
@@ -65,6 +66,44 @@
return types.NewInterfaceType(methods, embeddeds), nil
}
+var ErrEmptyTypeSet = errors.New("empty type set")
+
+// StructuralTerms returns the normalized structural type restrictions of a
+// type, if any. For types that are not type parameters, it returns term slice
+// containing a single non-tilde term holding the given type. For type
+// parameters, it returns the normalized term list of the type parameter's
+// constraint. See NormalizeInterface for more information on the normal form
+// of a constraint interface.
+//
+// StructuralTerms returns an error if the structural term list cannot be
+// computed. If the type set of typ is empty, it returns ErrEmptyTypeSet.
+func StructuralTerms(typ types.Type) ([]*Term, error) {
+ switch typ := typ.(type) {
+ case *TypeParam:
+ iface, _ := typ.Constraint().(*types.Interface)
+ if iface == nil {
+ return nil, fmt.Errorf("constraint is %T, not *types.Interface", typ)
+ }
+ tset, err := computeTermSet(iface, make(map[types.Type]*termSet), 0)
+ if err != nil {
+ return nil, err
+ }
+ if tset.terms.isEmpty() {
+ return nil, ErrEmptyTypeSet
+ }
+ if tset.terms.isAll() {
+ return nil, nil
+ }
+ var terms []*Term
+ for _, term := range tset.terms {
+ terms = append(terms, NewTerm(term.tilde, term.typ))
+ }
+ return terms, nil
+ default:
+ return []*Term{NewTerm(false, typ)}, nil
+ }
+}
+
// A termSet holds the normalized set of terms for a given type.
//
// The name termSet is intentionally distinct from 'type set': a type set is
diff --git a/internal/typeparams/typeparams_go117.go b/internal/typeparams/typeparams_go117.go
index 61e07b0..6ad3a43 100644
--- a/internal/typeparams/typeparams_go117.go
+++ b/internal/typeparams/typeparams_go117.go
@@ -164,19 +164,25 @@
return named
}
-// Term is a placeholder type, as type parameters are not supported at this Go
-// version. Its methods panic on use.
-type Term struct{}
+// Term holds information about a structural type restriction.
+type Term struct {
+ tilde bool
+ typ types.Type
+}
-func (*Term) Tilde() bool { unsupported(); return false }
-func (*Term) Type() types.Type { unsupported(); return nil }
-func (*Term) String() string { unsupported(); return "" }
-func (*Term) Underlying() types.Type { unsupported(); return nil }
+func (m *Term) Tilde() bool { return m.tilde }
+func (m *Term) Type() types.Type { return m.typ }
+func (m *Term) String() string {
+ pre := ""
+ if m.tilde {
+ pre = "~"
+ }
+ return pre + m.typ.String()
+}
// NewTerm is unsupported at this Go version, and panics.
func NewTerm(tilde bool, typ types.Type) *Term {
- unsupported()
- return nil
+ return &Term{tilde, typ}
}
// Union is a placeholder type, as type parameters are not supported at this Go
