Skip to content

Commit

Permalink
dev: Fix for missing TypeParams for methods (#82)
Browse files Browse the repository at this point in the history
* dev: Fix for missing TypeParams in methods

due to golang/go#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 #37
  • Loading branch information
butuzov committed Jan 23, 2024
1 parent 8e544a0 commit 99b5347
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 100 deletions.
1 change: 1 addition & 0 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
fail-fast: true
matrix:
go:
- "1.18"
- "1.19"
- "1.20"
- "1.21"
Expand Down
59 changes: 43 additions & 16 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"flag"
"go/ast"
gotypes "go/types"
"runtime"
"strings"
"sync"

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
74 changes: 41 additions & 33 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
},
})
Expand Down Expand Up @@ -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)",
},
})
Expand All @@ -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)",
Expand Down Expand Up @@ -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/*"},
Expand All @@ -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)",
Expand All @@ -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)",
},
})
Expand All @@ -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)",
Expand All @@ -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",
},
})

Expand All @@ -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)",
Expand Down
11 changes: 8 additions & 3 deletions analyzer/internal/types/iface.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type IFace struct {

Pos token.Pos // Token Position
FuncName string //
OfType string
}

func NewIssue(name string, interfaceType IType) IFace {
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions analyzer/testdata/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func newErrorInterface() error {

var _ error = (*errorInterface)(nil)

// never suppose to be linted
func newErrorInterfaceConcrete() *errorInterface {
return &errorInterface{}
}
48 changes: 38 additions & 10 deletions analyzer/testdata/generics.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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
}
Loading

0 comments on commit 99b5347

Please sign in to comment.