From 99b5347149a34764a7986ef066c0ef4199016e22 Mon Sep 17 00:00:00 2001 From: Oleg Butuzov Date: Tue, 23 Jan 2024 09:20:09 +0200 Subject: [PATCH] dev: Fix for missing TypeParams for methods (#82) * dev: Fix for missing TypeParams in methods due to https://github.com/golang/go/issues/50956 I must to drop check for typeParams (which is `nil` for methods). So from now on, i am checking if param is interface implementation, then exclude one by one interface types. - Added new tests. - Added compatibility code (to align with go1.18 and go1.19 type params representation) closes https://github.com/butuzov/ireturn/issues/37 --- .github/workflows/main.yaml | 1 + analyzer/analyzer.go | 59 +++++++++++----- analyzer/analyzer_test.go | 74 +++++++++++--------- analyzer/internal/types/iface.go | 11 ++- analyzer/testdata/errors.go | 1 + analyzer/testdata/generics.go | 48 ++++++++++--- analyzer/testdata/named_interfaces_simple.go | 5 ++ analyzer/typeparams.go | 38 ---------- 8 files changed, 137 insertions(+), 100 deletions(-) delete mode 100644 analyzer/typeparams.go diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 26fcb3f..49ca685 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -16,6 +16,7 @@ jobs: fail-fast: true matrix: go: + - "1.18" - "1.19" - "1.20" - "1.21" diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index 21e5897..56ba250 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -4,6 +4,7 @@ import ( "flag" "go/ast" gotypes "go/types" + "runtime" "strings" "sync" @@ -70,7 +71,6 @@ func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) { // 004. Filtering Results. for _, issue := range filterInterfaces(pass, f.Type, dotImportedStd) { - if a.handler.IsValid(issue) { continue } @@ -146,13 +146,10 @@ func filterInterfaces(p *analysis.Pass, ft *ast.FuncType, di map[string]struct{} return results } - tp := newTypeParams(ft.TypeParams) - for _, el := range ft.Results.List { switch v := el.Type.(type) { // ----- empty or anonymous interfaces case *ast.InterfaceType: - if len(v.Methods.List) == 0 { results = append(results, types.NewIssue("interface{}", types.EmptyInterface)) continue @@ -164,35 +161,65 @@ func filterInterfaces(p *analysis.Pass, ft *ast.FuncType, di map[string]struct{} case *ast.Ident: t1 := p.TypesInfo.TypeOf(el.Type) - if !gotypes.IsInterface(t1.Underlying()) { + val, ok := t1.Underlying().(*gotypes.Interface) + if !ok { continue } - word := t1.String() - // only build in interface is error - if obj := gotypes.Universe.Lookup(word); obj != nil { - results = append(results, types.NewIssue(obj.Name(), types.ErrorInterface)) + var ( + name = t1.String() + isNamed = strings.Contains(name, ".") + isEmpty = val.Empty() + ) + + // catching any + if isEmpty && name == "any" { + results = append(results, types.NewIssue(name, types.EmptyInterface)) + continue + } + + // NOTE: FIXED! + if name == "error" { + results = append(results, types.NewIssue(name, types.ErrorInterface)) continue } - // found in type params - if tp.In(word) { - results = append(results, types.NewIssue(word, types.Generic)) + if !isNamed { + + typeParams := val.String() + prefix, suffix := "interface{", "}" + if strings.HasPrefix(typeParams, prefix) { // nolint: gosimple + typeParams = typeParams[len(prefix):] + } + if strings.HasSuffix(typeParams, suffix) { + typeParams = typeParams[:len(typeParams)-1] + } + + goVersion := runtime.Version() + if strings.HasPrefix(goVersion, "go1.18") || strings.HasPrefix(goVersion, "go1.19") { + typeParams = strings.ReplaceAll(typeParams, "|", " | ") + } + + results = append(results, types.IFace{ + Name: name, + Type: types.Generic, + OfType: typeParams, + }) continue } // is it dot-imported package? // handling cases when stdlib package imported via "." dot-import if len(di) > 0 { - name := stdPkgInterface(word) - if _, ok := di[name]; ok { - results = append(results, types.NewIssue(word, types.NamedStdInterface)) + pkgName := stdPkgInterface(name) + if _, ok := di[pkgName]; ok { + results = append(results, types.NewIssue(name, types.NamedStdInterface)) continue } } - results = append(results, types.NewIssue(word, types.NamedInterface)) + results = append(results, types.NewIssue(name, types.NamedInterface)) // ------- standard library and 3rd party interfaces case *ast.SelectorExpr: diff --git a/analyzer/analyzer_test.go b/analyzer/analyzer_test.go index 5cf658d..9841e8d 100644 --- a/analyzer/analyzer_test.go +++ b/analyzer/analyzer_test.go @@ -139,6 +139,7 @@ func TestAll(t *testing.T) { "New returns interface (github.com/foo/bar.Buzzer)", "NewDeclared returns interface (internal/sample.Doer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", }, }) @@ -167,6 +168,7 @@ func TestAll(t *testing.T) { "New returns interface (github.com/foo/bar.Buzzer)", "NewDeclared returns interface (internal/sample.Doer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", }, }) @@ -180,6 +182,7 @@ func TestAll(t *testing.T) { want: []string{ "NewDeclared returns interface (internal/sample.Doer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", "NamedContext returns interface (context.Context)", "NamedBytes returns interface (io.Writer)", @@ -210,19 +213,6 @@ func TestAll(t *testing.T) { }, }) - // was already commented - // tests = append(tests, testCase{ - // name: "Named/allow/(stdmimic)", - // mask: []string{"internal/io/*"}, - // meta: map[string]string{ - // "allow": "", // - // }, - // pkgm: "io", - // want: []string{ - // "Get returns interface (io.Writer)", - // }, - // }) - tests = append(tests, testCase{ name: "Named Interfaces/regexp/allow", mask: []string{"named_*.go", "github.com/foo/bar/*", "internal/sample/*"}, @@ -233,6 +223,7 @@ func TestAll(t *testing.T) { "s returns interface (github.com/foo/bar.Buzzer)", "New returns interface (github.com/foo/bar.Buzzer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", "NamedContext returns interface (context.Context)", "NamedBytes returns interface (io.Writer)", @@ -245,16 +236,20 @@ func TestAll(t *testing.T) { mask: []string{"*.go", "github.com/foo/bar/*", "internal/sample/*"}, meta: map[string]string{}, // skipping any configuration to run default one. want: []string{ - "Min returns generic interface (T)", - "MixedReturnParameters returns generic interface (T)", - "MixedReturnParameters returns generic interface (K)", - "Max returns generic interface (foobar)", - "Foo returns generic interface (GENERIC)", - "SumIntsOrFloats returns generic interface (V)", + "Min returns generic interface (T) of type param ~int | ~float64 | ~float32", + "MixedReturnParameters returns generic interface (T) of type param ~int | ~float64 | ~float32", + "MixedReturnParameters returns generic interface (K) of type param ~int | ~float64 | ~float32", + "Max returns generic interface (foobar) of type param ~int | ~float64 | ~float32", + "SumIntsOrFloats returns generic interface (V) of type param int64 | float64", + "FuncWithGenericAny_NamedReturn returns generic interface (T_ANY) of type param any", + "FuncWithGenericAny returns generic interface (T_ANY) of type param any", + "Get returns generic interface (V_COMPARABLE) of type param comparable", + "Get returns generic interface (V_ANY) of type param any", "s returns interface (github.com/foo/bar.Buzzer)", "New returns interface (github.com/foo/bar.Buzzer)", "NewDeclared returns interface (internal/sample.Doer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", }, }) @@ -263,7 +258,7 @@ func TestAll(t *testing.T) { name: "all/stdlib/allow", mask: []string{"*.go", "github.com/foo/bar/*", "internal/sample/*"}, meta: map[string]string{ - "allow": types.NameStdLib, // allow only interfaces from standard library + "allow": types.NameStdLib, // allow only interfaces from standard library (e.g. io.Writer, fmt.Stringer) }, want: []string{ "NewanonymousInterface returns interface (anonymous interface)", @@ -274,33 +269,43 @@ func TestAll(t *testing.T) { "errorAliasReturn returns interface (error)", "errorTypeReturn returns interface (error)", "newErrorInterface returns interface (error)", - "Min returns generic interface (T)", - "MixedReturnParameters returns generic interface (T)", - "MixedReturnParameters returns generic interface (K)", - "Max returns generic interface (foobar)", - "Foo returns generic interface (GENERIC)", - "SumIntsOrFloats returns generic interface (V)", + "Min returns generic interface (T) of type param ~int | ~float64 | ~float32", + "MixedReturnParameters returns generic interface (T) of type param ~int | ~float64 | ~float32", + "MixedReturnParameters returns generic interface (K) of type param ~int | ~float64 | ~float32", + "Max returns generic interface (foobar) of type param ~int | ~float64 | ~float32", + "SumIntsOrFloats returns generic interface (V) of type param int64 | float64", + "FuncWithGenericAny_NamedReturn returns generic interface (T_ANY) of type param any", + "FuncWithGenericAny returns generic interface (T_ANY) of type param any", + "Get returns generic interface (V_COMPARABLE) of type param comparable", + "Get returns generic interface (V_ANY) of type param any", + "FunctionAny returns interface (any)", + "FunctionInterface returns interface (interface{})", "s returns interface (github.com/foo/bar.Buzzer)", "New returns interface (github.com/foo/bar.Buzzer)", "NewDeclared returns interface (internal/sample.Doer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", }, }) + // Rejecting Only Generic Returns tests = append(tests, testCase{ name: "generic/reject", mask: []string{"*.go", "github.com/foo/bar/*", "internal/sample/*"}, meta: map[string]string{ - "reject": types.NameGeneric, // allow only generic interfaces + "reject": types.NameGeneric, // reject only generic interfaces }, want: []string{ - "Min returns generic interface (T)", - "MixedReturnParameters returns generic interface (T)", - "MixedReturnParameters returns generic interface (K)", - "Max returns generic interface (foobar)", - "Foo returns generic interface (GENERIC)", - "SumIntsOrFloats returns generic interface (V)", + "Min returns generic interface (T) of type param ~int | ~float64 | ~float32", + "MixedReturnParameters returns generic interface (T) of type param ~int | ~float64 | ~float32", + "MixedReturnParameters returns generic interface (K) of type param ~int | ~float64 | ~float32", + "Max returns generic interface (foobar) of type param ~int | ~float64 | ~float32", + "SumIntsOrFloats returns generic interface (V) of type param int64 | float64", + "FuncWithGenericAny_NamedReturn returns generic interface (T_ANY) of type param any", + "FuncWithGenericAny returns generic interface (T_ANY) of type param any", + "Get returns generic interface (V_COMPARABLE) of type param comparable", + "Get returns generic interface (V_ANY) of type param any", }, }) @@ -319,10 +324,13 @@ func TestAll(t *testing.T) { "errorAliasReturn returns interface (error)", "errorTypeReturn returns interface (error)", "newErrorInterface returns interface (error)", + "FunctionAny returns interface (any)", + "FunctionInterface returns interface (interface{})", "s returns interface (github.com/foo/bar.Buzzer)", "New returns interface (github.com/foo/bar.Buzzer)", "NewDeclared returns interface (internal/sample.Doer)", "newIDoer returns interface (example.iDoer)", + "newIDoerAny returns interface (example.iDoerAny)", "NewNamedStruct returns interface (example.FooerBarer)", "NamedContext returns interface (context.Context)", "NamedBytes returns interface (io.Writer)", diff --git a/analyzer/internal/types/iface.go b/analyzer/internal/types/iface.go index 13f19a3..5e57637 100644 --- a/analyzer/internal/types/iface.go +++ b/analyzer/internal/types/iface.go @@ -14,6 +14,7 @@ type IFace struct { Pos token.Pos // Token Position FuncName string // + OfType string } func NewIssue(name string, interfaceType IType) IFace { @@ -30,11 +31,15 @@ func (i *IFace) Enrich(f *ast.FuncDecl) { } func (i IFace) String() string { - if i.Type == Generic { - return fmt.Sprintf("%s returns generic interface (%s)", i.FuncName, i.Name) + if i.Type != Generic { + return fmt.Sprintf("%s returns interface (%s)", i.FuncName, i.Name) } - return fmt.Sprintf("%s returns interface (%s)", i.FuncName, i.Name) + if i.OfType != "" { + return fmt.Sprintf("%s returns generic interface (%s) of type param %s", i.FuncName, i.Name, i.OfType) + } + + return fmt.Sprintf("%s returns generic interface (%s)", i.FuncName, i.Name) } func (i IFace) HashString() string { diff --git a/analyzer/testdata/errors.go b/analyzer/testdata/errors.go index 8a78ccc..f9aabd2 100644 --- a/analyzer/testdata/errors.go +++ b/analyzer/testdata/errors.go @@ -28,6 +28,7 @@ func newErrorInterface() error { var _ error = (*errorInterface)(nil) +// never suppose to be linted func newErrorInterfaceConcrete() *errorInterface { return &errorInterface{} } diff --git a/analyzer/testdata/generics.go b/analyzer/testdata/generics.go index 002a747..2afaf43 100644 --- a/analyzer/testdata/generics.go +++ b/analyzer/testdata/generics.go @@ -1,32 +1,27 @@ package example -type Numeric interface { - ~int +type typeConstraints interface { + ~int | ~float64 | ~float32 } -func Min[T Numeric](x T, y T) (T, T) { +func Min[T typeConstraints](x T, y T) (T, T) { if x > y { return y, y } return x, x } -func MixedReturnParameters[T, K Numeric](x T, y K) (T, K, T, K, K, T) { +func MixedReturnParameters[T, K typeConstraints](x T, y K) (T, K, T, K, K, T) { return x, y, x, y, y, x } -func Max[foobar Numeric](x foobar, y foobar) foobar { +func Max[foobar typeConstraints](x foobar, y foobar) foobar { if x < y { return y } return x } -func Foo[GENERIC any](foobar GENERIC) (variable GENERIC) { - variable = foobar - return -} - // SumIntsOrFloats sums the values of map m. It supports both int64 and float64 // as types for map values. func SumIntsOrFloats[K comparable, V int64 | float64](m map[K]V) V { @@ -36,3 +31,36 @@ func SumIntsOrFloats[K comparable, V int64 | float64](m map[K]V) V { } return s } + +func FuncWithGenericAny_NamedReturn[T_ANY any](foobar T_ANY) (variable T_ANY) { + variable = foobar + return +} + +func FuncWithGenericAny[T_ANY any](foo T_ANY) T_ANY { + return foo +} + +func is_test() { + if FuncWithGenericAny[int64](65) == 65 { + print("yeap") + } +} + +// ISSUE: https://github.com/butuzov/ireturn/issues/37 +type Map1[K comparable, V_COMPARABLE comparable] map[K]V_COMPARABLE + +func (m Map1[K, V_COMPARABLE]) Get(key K) V_COMPARABLE { return m[key] } + +type Map2[K comparable, V_ANY any] map[K]V_ANY + +func (m Map2[K, V_ANY]) Get(key K) V_ANY { return m[key] } + +// Empty Interface return +func FunctionAny() any { + return nil +} + +func FunctionInterface() interface{} { + return nil +} diff --git a/analyzer/testdata/named_interfaces_simple.go b/analyzer/testdata/named_interfaces_simple.go index 0d7b0be..8f80dcb 100644 --- a/analyzer/testdata/named_interfaces_simple.go +++ b/analyzer/testdata/named_interfaces_simple.go @@ -2,6 +2,7 @@ package example type ( iDoer interface{} + iDoerAny any iDoerConcreat int ) @@ -9,6 +10,10 @@ func newIDoer() iDoer { return iDoerConcreat(0) } +func newIDoerAny() iDoerAny { + return iDoerConcreat(0) +} + type Fooer interface { Foo() } diff --git a/analyzer/typeparams.go b/analyzer/typeparams.go deleted file mode 100644 index 14193c3..0000000 --- a/analyzer/typeparams.go +++ /dev/null @@ -1,38 +0,0 @@ -package analyzer - -import ( - "go/ast" -) - -type typeParams struct { - found []string -} - -func newTypeParams(fl *ast.FieldList) typeParams { - tp := typeParams{} - - if fl == nil { - return tp - } - - for _, el := range fl.List { - if el == nil { - continue - } - - for _, name := range el.Names { - tp.found = append(tp.found, name.Name) - } - } - - return tp -} - -func (tp typeParams) In(t string) bool { - for _, i := range tp.found { - if i == t { - return true - } - } - return false -}