Skip to content

Commit

Permalink
go/analysis/passes/composite: update for generics
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
findleyr committed Oct 25, 2021
1 parent 5a40697 commit 1050b5c
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 37 deletions.
66 changes: 40 additions & 26 deletions go/analysis/passes/composite/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
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
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
}
if isLocalType(pass, term.Type()) {
// allow unkeyed locally defined composite literal
continue
}

// check if the CompositeLit contains an unkeyed field
allKeyValue := true
for _, e := range cl.Elts {
if _, ok := e.(*ast.KeyValueExpr); !ok {
allKeyValue = false
break
// 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
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:
Expand All @@ -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
}
7 changes: 6 additions & 1 deletion go/analysis/passes/composite/composite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
Original file line number Diff line number Diff line change
@@ -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
27 changes: 27 additions & 0 deletions go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
Original file line number Diff line number Diff line change
@@ -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}
}
39 changes: 39 additions & 0 deletions internal/typeparams/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package typeparams

import (
"errors"
"fmt"
"go/types"
"os"
Expand Down Expand Up @@ -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
Expand Down
26 changes: 16 additions & 10 deletions internal/typeparams/typeparams_go117.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

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 }
// Term holds information about a structural type restriction.
type Term struct {
tilde bool
typ types.Type
}

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
Expand Down

0 comments on commit 1050b5c

Please sign in to comment.