Skip to content

Commit

Permalink
Update all checks to support explicit alias nodes in the type graph
Browse files Browse the repository at this point in the history
Go 1.22 introduced go/types.Alias, a node that explicitly encodes type
aliases. GODEBUG=gotypesalias=1 enables this behavior. In Go 1.23, that
setting will become the new default.

Update all code that compares types (via equality, string equality, or
type assertions) to account for aliases. Most checks don't require
changes, as they operate on underlying types. For the remainder, a mix
of types.Unalias and explicit handling of aliases is necessary.

Closes: gh-1523
  • Loading branch information
dominikh committed Apr 24, 2024
1 parent 4c40d00 commit 9df349e
Show file tree
Hide file tree
Showing 52 changed files with 409 additions and 106 deletions.
40 changes: 36 additions & 4 deletions analysis/code/code.go
Expand Up @@ -29,8 +29,30 @@ type Positioner interface {
Pos() token.Pos
}

func IsOfType(pass *analysis.Pass, expr ast.Expr, name string) bool {
return typeutil.IsType(pass.TypesInfo.TypeOf(expr), name)
func IsOfStringConvertibleByteSlice(pass *analysis.Pass, expr ast.Expr) bool {
typ, ok := pass.TypesInfo.TypeOf(expr).Underlying().(*types.Slice)
if !ok {
return false
}
elem := types.Unalias(typ.Elem())
if LanguageVersion(pass, expr) >= 18 {
// Before Go 1.18, one could not directly convert from []T (where 'type T byte')
// to string. See also https://github.com/golang/go/issues/23536.
elem = elem.Underlying()
}
return types.Identical(elem, types.Typ[types.Byte])
}

func IsOfPointerToTypeWithName(pass *analysis.Pass, expr ast.Expr, name string) bool {
ptr, ok := types.Unalias(pass.TypesInfo.TypeOf(expr)).(*types.Pointer)
if !ok {
return false
}
return typeutil.IsTypeWithName(ptr.Elem(), name)
}

func IsOfTypeWithName(pass *analysis.Pass, expr ast.Expr, name string) bool {
return typeutil.IsTypeWithName(pass.TypesInfo.TypeOf(expr), name)
}

func IsInTest(pass *analysis.Pass, node Positioner) bool {
Expand Down Expand Up @@ -99,8 +121,9 @@ func BoolConst(pass *analysis.Pass, expr ast.Expr) bool {

func IsBoolConst(pass *analysis.Pass, expr ast.Expr) bool {
// We explicitly don't support typed bools because more often than
// not, custom bool types are used as binary enums and the
// explicit comparison is desired.
// not, custom bool types are used as binary enums and the explicit
// comparison is desired. We err on the side of false negatives and
// treat aliases like other custom types.

ident, ok := expr.(*ast.Ident)
if !ok {
Expand Down Expand Up @@ -144,6 +167,9 @@ func ExprToString(pass *analysis.Pass, expr ast.Expr) (string, bool) {
}

func CallName(pass *analysis.Pass, call *ast.CallExpr) string {
// See the comment in typeutil.FuncName for why this doesn't require special handling
// of aliases.

fun := astutil.Unparen(call.Fun)

// Instantiating a function cannot return another generic function, so doing this once is enough
Expand Down Expand Up @@ -179,6 +205,9 @@ func CallName(pass *analysis.Pass, call *ast.CallExpr) string {
}

func IsCallTo(pass *analysis.Pass, node ast.Node, name string) bool {
// See the comment in typeutil.FuncName for why this doesn't require special handling
// of aliases.

call, ok := node.(*ast.CallExpr)
if !ok {
return false
Expand All @@ -187,6 +216,9 @@ func IsCallTo(pass *analysis.Pass, node ast.Node, name string) bool {
}

func IsCallToAny(pass *analysis.Pass, node ast.Node, names ...string) bool {
// See the comment in typeutil.FuncName for why this doesn't require special handling
// of aliases.

call, ok := node.(*ast.CallExpr)
if !ok {
return false
Expand Down
2 changes: 1 addition & 1 deletion cmd/structlayout/main.go
Expand Up @@ -65,7 +65,7 @@ func main() {
log.Fatal("identifier is not a struct type")
}

fields := sizes(st, typ.(*types.Named).Obj().Name(), 0, nil)
fields := sizes(st, types.Unalias(typ).(*types.Named).Obj().Name(), 0, nil)
if fJSON {
emitJSON(fields)
} else {
Expand Down
3 changes: 3 additions & 0 deletions go/ir/methods.go
Expand Up @@ -235,6 +235,9 @@ func (prog *Program) needMethods(T types.Type, skip bool) {
prog.needMethods(t.At(i).Type(), false)
}

case *types.Alias:
prog.needMethods(types.Unalias(t), false)

default:
lint.ExhaustiveTypeSwitch(T)
}
Expand Down
2 changes: 1 addition & 1 deletion go/ir/sanity.go
Expand Up @@ -371,7 +371,7 @@ func (s *sanity) checkBlock(b *BasicBlock, index int) {

// Check that "untyped" types only appear on constant operands.
if _, ok := (*op).(*Const); !ok {
if basic, ok := (*op).Type().(*types.Basic); ok {
if basic, ok := types.Unalias((*op).Type()).(*types.Basic); ok {
if basic.Info()&types.IsUntyped != 0 {
s.errorf("operand #%d of %s is untyped: %s", i, instr, basic)
}
Expand Down
3 changes: 3 additions & 0 deletions go/ir/util.go
Expand Up @@ -45,9 +45,12 @@ func isPointer(typ types.Type) bool {
// deref returns a pointer's element type; otherwise it returns typ.
func deref(typ types.Type) types.Type {
orig := typ
typ = types.Unalias(typ)

if t, ok := typ.(*types.TypeParam); ok {
if ctyp := typeutil.CoreType(t); ctyp != nil {
// This can happen, for example, with len(T) where T is a
// type parameter whose core type is a pointer to array.
typ = ctyp
}
}
Expand Down
2 changes: 1 addition & 1 deletion go/ir/wrappers.go
Expand Up @@ -291,7 +291,7 @@ func makeInstance(prog *Program, fn *Function, sig *types.Signature, targs *type
if sig.Recv() != nil {
assert(targs == nil)
// Methods don't have their own type parameters, but the receiver does
targs = deref(sig.Recv().Type()).(*types.Named).TypeArgs()
targs = types.Unalias(deref(sig.Recv().Type())).(*types.Named).TypeArgs()
} else {
assert(targs != nil)
}
Expand Down
63 changes: 61 additions & 2 deletions go/types/typeutil/util.go
Expand Up @@ -3,6 +3,7 @@ package typeutil
import (
"bytes"
"go/types"
"strings"
"sync"

"golang.org/x/exp/typeparams"
Expand All @@ -17,6 +18,11 @@ var bufferPool = &sync.Pool{
}

func FuncName(f *types.Func) string {
// We don't care about aliases in this function because we use FuncName to check calls
// to known methods, and method receivers are determined by the method declaration,
// not the call. Thus, even if a user does 'type Alias = *sync.Mutex' and calls
// Alias.Lock, we'll still see it as (*sync.Mutex).Lock.

buf := bufferPool.Get().(*bytes.Buffer)
buf.Reset()
if f.Type() != nil {
Expand Down Expand Up @@ -82,8 +88,61 @@ func IsObject(obj types.Object, name string) bool {
return path+obj.Name() == name
}

// OPT(dh): IsType is kind of expensive; should we really use it?
func IsType(T types.Type, name string) bool { return types.TypeString(T, nil) == name }
// IsTypeName reports whether obj represents the qualified name. If obj is a type alias,
// IsTypeName checks both the alias and the aliased type, if the aliased type has a type
// name.
func IsTypeName(obj *types.TypeName, name string) bool {
var qf string
if idx := strings.LastIndex(name, "."); idx != -1 {
qf = name[:idx]
name = name[idx+1:]
}
if obj.Name() == name &&
((qf == "" && obj.Pkg() == nil) || (obj.Pkg() != nil && obj.Pkg().Path() == qf)) {
return true
}

if !obj.IsAlias() {
return false
}

// FIXME(dh): we should peel away one layer of alias at a time; this is blocked on
// github.com/golang/go/issues/66559
if typ, ok := types.Unalias(obj.Type()).(interface{ Obj() *types.TypeName }); ok {
return IsTypeName(typ.Obj(), name)
}

return false
}

func IsPointerToTypeWithName(typ types.Type, name string) bool {
ptr, ok := types.Unalias(typ).(*types.Pointer)
if !ok {
return false
}
return IsTypeWithName(ptr.Elem(), name)
}

// IsTypeWithName reports whether typ represents a type with the qualified name, If typ is
// a type alias, IsTypeWithName checks both the alias and the aliased type. The following
// types can have names: Basic, Named, Alias.
func IsTypeWithName(typ types.Type, name string) bool {
switch typ := typ.(type) {
case *types.Basic:
return typ.Name() == name
case *types.Named:
return IsTypeName(typ.Obj(), name)
case *types.Alias:
// FIXME(dh): we should peel away one layer of alias at a time; this is blocked on
// github.com/golang/go/issues/66559

// IsTypeName already handles aliases to other aliases or named types; our
// fallback is required for aliases to basic types.
return IsTypeName(typ.Obj(), name) || IsTypeWithName(types.Unalias(typ), name)
default:
return false
}
}

// IsPointerLike returns true if type T is like a pointer. This returns true for all nillable types,
// unsafe.Pointer, and type sets where at least one term is pointer-like.
Expand Down
4 changes: 2 additions & 2 deletions internal/sharedcheck/lint.go
Expand Up @@ -44,7 +44,7 @@ func CheckRangeStringRunes(pass *analysis.Pass) (interface{}, error) {
if !ok {
return true
}
TdstElem, ok := Tdst.Elem().(*types.Basic)
TdstElem, ok := types.Unalias(Tdst.Elem()).(*types.Basic)
if !ok || TdstElem.Kind() != types.Int32 {
return true
}
Expand Down Expand Up @@ -154,7 +154,7 @@ func RedundantTypeInDeclarationChecker(verb string, flagHelpfulTypes bool) *anal
if err != nil {
panic(err)
}
if b, ok := tv.Type.(*types.Basic); ok && (b.Info()&types.IsUntyped) != 0 {
if b, ok := types.Unalias(tv.Type).(*types.Basic); ok && (b.Info()&types.IsUntyped) != 0 {
if Tlhs != types.Default(b) {
// The rhs is untyped and its default type differs from the explicit type on the lhs
continue specLoop
Expand Down
2 changes: 1 addition & 1 deletion quickfix/qf1009/qf1009.go
Expand Up @@ -39,7 +39,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
if expr.Op != token.EQL {
return
}
if !code.IsOfType(pass, expr.X, "time.Time") || !code.IsOfType(pass, expr.Y, "time.Time") {
if !code.IsOfTypeWithName(pass, expr.X, "time.Time") || !code.IsOfTypeWithName(pass, expr.Y, "time.Time") {
return
}
report.Report(pass, node, "probably want to use time.Time.Equal instead",
Expand Down
18 changes: 8 additions & 10 deletions quickfix/qf1010/qf1010.go
Expand Up @@ -8,7 +8,6 @@ import (
"honnef.co/go/tools/analysis/edit"
"honnef.co/go/tools/analysis/lint"
"honnef.co/go/tools/analysis/report"
"honnef.co/go/tools/go/types/typeutil"
"honnef.co/go/tools/knowledge"
"honnef.co/go/tools/pattern"

Expand Down Expand Up @@ -66,16 +65,15 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
args := m.State["args"].([]ast.Expr)
for _, arg := range args {
T := pass.TypesInfo.TypeOf(arg)
if typeutil.IsType(T.Underlying(), "[]byte") {
// don't convert arguments that implement fmt.Stringer
if types.Implements(T, knowledge.Interfaces["fmt.Stringer"]) {
continue
}

fix := edit.Fix("Convert argument to string", edit.ReplaceWithPattern(pass.Fset, arg, byteSlicePrintingR, pattern.State{"arg": arg}))
report.Report(pass, arg, "could convert argument to string", report.Fixes(fix))
if !code.IsOfStringConvertibleByteSlice(pass, arg) {
continue
}
if types.Implements(pass.TypesInfo.TypeOf(arg), knowledge.Interfaces["fmt.Stringer"]) {
continue
}

fix := edit.Fix("Convert argument to string", edit.ReplaceWithPattern(pass.Fset, arg, byteSlicePrintingR, pattern.State{"arg": arg}))
report.Report(pass, arg, "could convert argument to string", report.Fixes(fix))
}
}
code.Preorder(pass, fn, (*ast.CallExpr)(nil))
Expand Down
Expand Up @@ -8,13 +8,20 @@ func (*Stringable) String() string { return "" }

func fn() {
type ByteSlice []byte
type Alias1 = []byte
type Alias2 = ByteSlice
type Alias3 = byte
type Alias4 = []Alias3
var b1 []byte
var b2 ByteSlice
var b3 *Stringable
var b4 Stringable

var s string
fmt.Print(1, b1, 2, []byte(""), b2, s) //@ diag(`could convert argument to string`), diag(`could convert argument to string`), diag(`could convert argument to string`)
fmt.Print(Alias1{}) //@ diag(`could convert argument to string`)
fmt.Print(Alias2{}) //@ diag(`could convert argument to string`)
fmt.Print(Alias4{}) //@ diag(`could convert argument to string`)
fmt.Fprint(nil, 1, b1, 2, []byte(""), b2, s) //@ diag(`could convert argument to string`), diag(`could convert argument to string`), diag(`could convert argument to string`)
fmt.Print()
fmt.Fprint(nil)
Expand Down
Expand Up @@ -8,13 +8,20 @@ func (*Stringable) String() string { return "" }

func fn() {
type ByteSlice []byte
type Alias1 = []byte
type Alias2 = ByteSlice
type Alias3 = byte
type Alias4 = []Alias3
var b1 []byte
var b2 ByteSlice
var b3 *Stringable
var b4 Stringable

var s string
fmt.Print(1, string(b1), 2, string([]byte("")), string(b2), s) //@ diag(`could convert argument to string`), diag(`could convert argument to string`), diag(`could convert argument to string`)
fmt.Print(string(Alias1{})) //@ diag(`could convert argument to string`)
fmt.Print(string(Alias2{})) //@ diag(`could convert argument to string`)
fmt.Print(string(Alias4{})) //@ diag(`could convert argument to string`)
fmt.Fprint(nil, 1, string(b1), 2, string([]byte("")), string(b2), s) //@ diag(`could convert argument to string`), diag(`could convert argument to string`), diag(`could convert argument to string`)
fmt.Print()
fmt.Fprint(nil)
Expand Down
30 changes: 26 additions & 4 deletions simple/s1016/s1016.go
Expand Up @@ -54,7 +54,19 @@ func run(pass *analysis.Pass) (interface{}, error) {
}

lit := node.(*ast.CompositeLit)
typ1, _ := pass.TypesInfo.TypeOf(lit.Type).(*types.Named)
var typ1 types.Type
var named1 *types.Named
switch typ := pass.TypesInfo.TypeOf(lit.Type).(type) {
case *types.Named:
typ1 = typ
named1 = typ
case *types.Alias:
ua := types.Unalias(typ)
if n, ok := ua.(*types.Named); ok {
typ1 = typ
named1 = n
}
}
if typ1 == nil {
return
}
Expand All @@ -63,7 +75,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
return
}

var typ2 *types.Named
var typ2 types.Type
var named2 *types.Named
var ident *ast.Ident
getSelType := func(expr ast.Expr) (types.Type, *ast.Ident, bool) {
sel, ok := expr.(*ast.SelectorExpr)
Expand Down Expand Up @@ -115,7 +128,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
if ident != nil && pass.TypesInfo.ObjectOf(ident) != pass.TypesInfo.ObjectOf(id) {
return
}
typ2, _ = t.(*types.Named)
switch t := t.(type) {
case *types.Named:
typ2 = t
named2 = t
case *types.Alias:
if n, ok := types.Unalias(t).(*types.Named); ok {
typ2 = t
named2 = n
}
}
if typ2 == nil {
return
}
Expand All @@ -126,7 +148,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
return
}

if typ1.Obj().Pkg() != typ2.Obj().Pkg() {
if named1.Obj().Pkg() != named2.Obj().Pkg() {
// Do not suggest type conversions between different
// packages. Types in different packages might only match
// by coincidence. Furthermore, if the dependency ever
Expand Down
@@ -0,0 +1,21 @@
package pkg

type S1 struct {
A int
}

type S2 struct {
A int
}

type Alias = S2

// XXX the diagnostics depend on GODEBUG

func foo() {
v1 := S1{A: 1}
v2 := Alias{A: 1}

_ = Alias{A: v1.A} //@ diag(`should convert v1`)
_ = S1{A: v2.A} //@ diag(`should convert v2`)
}

0 comments on commit 9df349e

Please sign in to comment.