From 1050b5ce2e85a5aa5f779cedae7ccfd99de46963 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 19 Oct 2021 18:42:24 -0400 Subject: [PATCH] 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 Run-TryBot: Robert Findley gopls-CI: kokoro Reviewed-by: Tim King TryBot-Result: Go Bot --- go/analysis/passes/composite/composite.go | 68 +++++++++++-------- .../passes/composite/composite_test.go | 7 +- .../testdata/src/typeparams/lib/lib.go | 9 +++ .../testdata/src/typeparams/typeparams.go | 27 ++++++++ internal/typeparams/normalize.go | 39 +++++++++++ internal/typeparams/typeparams_go117.go | 24 ++++--- 6 files changed, 137 insertions(+), 37 deletions(-) create mode 100644 go/analysis/passes/composite/testdata/src/typeparams/lib/lib.go create mode 100644 go/analysis/passes/composite/testdata/src/typeparams/typeparams.go diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go index 4c3ac6647f..025952ed50 100644 --- a/go/analysis/passes/composite/composite.go +++ b/go/analysis/passes/composite/composite.go @@ -14,6 +14,7 @@ import ( "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 @@ func run(pass *analysis.Pass) (interface{}, error) { // 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 @@ func isLocalType(pass *analysis.Pass, typ types.Type) bool { 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 c55015c22b..952de8bfda 100644 --- a/go/analysis/passes/composite/composite_test.go +++ b/go/analysis/passes/composite/composite_test.go @@ -9,9 +9,14 @@ import ( "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 0000000000..9d7710dd26 --- /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 0000000000..dd5d57efed --- /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 ef3dbab52a..29373508e9 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 @@ func NormalizeInterface(iface *types.Interface) (*types.Interface, error) { 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 61e07b0799..6ad3a43a2c 100644 --- a/internal/typeparams/typeparams_go117.go +++ b/internal/typeparams/typeparams_go117.go @@ -164,19 +164,25 @@ func NamedTypeOrigin(named *types.Named) types.Type { 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