Skip to content

Commit

Permalink
feature: initial release for generics support (#42)
Browse files Browse the repository at this point in the history
* feature: initial release for generics support
* chores: bump go version for CI pipelines
* chores: rename test file for generics
* refactor: issues diagnostics improvments
  • Loading branch information
butuzov authored Apr 14, 2023
1 parent 62c7258 commit 947e10b
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 47 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ jobs:
fail-fast: true
matrix:
go:
- "1.17"
- "1.16"
- "1.15"
- "1.19"
- "1.20"

steps:

- uses: actions/checkout@v2
Expand All @@ -41,11 +41,11 @@ jobs:

- name: Install goveralls
env: { GO111MODULE: "off" }
if: matrix.go == '1.17'
if: matrix.go == '1.20'
run: go get github.com/mattn/goveralls

- name: Coverage - Sending Report to Coveral
if: matrix.go == '1.17'
if: matrix.go == '1.20'
env:
COVERALLS_TOKEN: ${{ secrets.github_token }}
run: goveralls -coverprofile=coverage.cov -service=github
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- v*

env:
GO_VERSION: 1.17
GO_VERSION: 1.20

jobs:

Expand Down
63 changes: 35 additions & 28 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package analyzer

import (
"flag"
"fmt"
"go/ast"
gotypes "go/types"
"strings"
Expand Down Expand Up @@ -41,7 +40,7 @@ func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {

ins, _ := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

// 00. does file have dot-imported stadard packages?
// 00. does file have dot-imported standard packages?
dotImportedStd := make(map[string]struct{})
ins.Preorder([]ast.Node{(*ast.ImportSpec)(nil)}, func(node ast.Node) {
i, _ := node.(*ast.ImportSpec)
Expand All @@ -66,17 +65,25 @@ func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {
return
}

seen := make(map[string]bool, 4)

// 004. Filtering Results.
for _, i := range filterInterfaces(pass, f.Type.Results, dotImportedStd) {
for _, issue := range filterInterfaces(pass, f.Type, dotImportedStd) {

if a.handler.IsValid(issue) {
continue
}

if a.handler.IsValid(i) {
issue.Enrich(f)

key := issue.HashString()

if ok := seen[key]; ok {
continue
}
seen[key] = true

a.found = append(a.found, analysis.Diagnostic{ //nolint: exhaustivestruct
Pos: f.Pos(),
Message: fmt.Sprintf("%s returns interface (%s)", f.Name.Name, i.Name),
})
a.found = append(a.found, issue.ExportDiagnostic())
}
})

Expand Down Expand Up @@ -122,20 +129,26 @@ func flags() flag.FlagSet {
return *set
}

func filterInterfaces(p *analysis.Pass, fl *ast.FieldList, di map[string]struct{}) []types.IFace {
func filterInterfaces(p *analysis.Pass, ft *ast.FuncType, di map[string]struct{}) []types.IFace {
var results []types.IFace

for pos, el := range fl.List {
if ft.Results == nil { // this can't happen, but double checking.
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, issue("interface{}", pos, types.EmptyInterface))
results = append(results, types.NewIssue("interface{}", types.EmptyInterface))
continue
}

results = append(results, issue("anonymous interface", pos, types.AnonInterface))
results = append(results, types.NewIssue("anonymous interface", types.AnonInterface))

// ------ Errors and interfaces from same package
case *ast.Ident:
Expand All @@ -148,24 +161,28 @@ func filterInterfaces(p *analysis.Pass, fl *ast.FieldList, di map[string]struct{
word := t1.String()
// only build in interface is error
if obj := gotypes.Universe.Lookup(word); obj != nil {
results = append(results, issue(obj.Name(), pos, types.ErrorInterface))
results = append(results, types.NewIssue(obj.Name(), types.ErrorInterface))
continue
}

// found in type params
if tp.In(word) {
results = append(results, types.NewIssue(word, types.Generic))
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, issue(word, pos, types.NamedStdInterface))
results = append(results, types.NewIssue(word, types.NamedStdInterface))

continue
}
}

results = append(results, issue(word, pos, types.NamedInterface))
results = append(results, types.NewIssue(word, types.NamedInterface))

// ------- standard library and 3rd party interfaces
case *ast.SelectorExpr:
Expand All @@ -177,12 +194,11 @@ func filterInterfaces(p *analysis.Pass, fl *ast.FieldList, di map[string]struct{

word := t1.String()
if isStdPkgInterface(word) {
results = append(results, issue(word, pos, types.NamedStdInterface))

results = append(results, types.NewIssue(word, types.NamedStdInterface))
continue
}

results = append(results, issue(word, pos, types.NamedInterface))
results = append(results, types.NewIssue(word, types.NamedInterface))
}
}

Expand Down Expand Up @@ -214,12 +230,3 @@ func stdPkg(pkg string) string {

return ""
}

// issue is shortcut that creates issue for next filtering.
func issue(name string, pos int, interfaceType types.IType) types.IFace {
return types.IFace{
Name: name,
Pos: pos,
Type: interfaceType,
}
}
71 changes: 66 additions & 5 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"testing"

"github.com/butuzov/ireturn/analyzer/internal/types"

"github.com/stretchr/testify/assert"
"golang.org/x/tools/go/analysis/analysistest"
)
Expand Down Expand Up @@ -43,6 +42,13 @@ func TestAll(t *testing.T) {
want: []string{},
})

// tests = append(tests, testCase{
// name: "Generic Interface",
// mask: []string{"generic.go", "go.*"},
// meta: map[string]string{},
// want: []string{},
// })

tests = append(tests, testCase{
name: "interface{}/allow",
mask: []string{"empty_interface.go", "go.*"},
Expand Down Expand Up @@ -204,6 +210,7 @@ func TestAll(t *testing.T) {
},
})

// was already commented
// tests = append(tests, testCase{
// name: "Named/allow/(stdmimic)",
// mask: []string{"internal/io/*"},
Expand Down Expand Up @@ -234,10 +241,16 @@ func TestAll(t *testing.T) {
})

tests = append(tests, testCase{
name: "default/all/non_std/reject_all_but_named",
name: "defaults",
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)",
"s returns interface (github.com/foo/bar.Buzzer)",
"New returns interface (github.com/foo/bar.Buzzer)",
"NewDeclared returns interface (internal/sample.Doer)",
Expand All @@ -252,6 +265,51 @@ func TestAll(t *testing.T) {
meta: map[string]string{
"allow": types.NameStdLib, // allow only interfaces from standard library
},
want: []string{
"NewanonymousInterface returns interface (anonymous interface)",
"dissAllowDirective2 returns interface (interface{})",
"dissAllowDirective6 returns interface (interface{})",
"fooInterface returns interface (interface{})",
"errorReturn returns interface (error)",
"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)",
"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)",
"NewNamedStruct returns interface (example.FooerBarer)",
},
})

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
},
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)",
},
})

tests = append(tests, testCase{
name: "generic/allow",
mask: []string{"*.go", "github.com/foo/bar/*", "internal/sample/*"},
meta: map[string]string{
"allow": types.NameGeneric, // allow only generic interfaces
},
want: []string{
"NewanonymousInterface returns interface (anonymous interface)",
"dissAllowDirective2 returns interface (interface{})",
Expand All @@ -266,6 +324,9 @@ func TestAll(t *testing.T) {
"NewDeclared returns interface (internal/sample.Doer)",
"newIDoer returns interface (example.iDoer)",
"NewNamedStruct returns interface (example.FooerBarer)",
"NamedContext returns interface (context.Context)",
"NamedBytes returns interface (io.Writer)",
"NamedStdFile returns interface (go/types.Importer)",
},
})

Expand Down Expand Up @@ -350,7 +411,7 @@ func directory(t *testing.T) (goroot, srcdir string, err error) {
goroot = t.TempDir()
srcdir = filepath.Join(goroot, "src")

if err := os.MkdirAll(srcdir, 0777); err != nil {
if err := os.MkdirAll(srcdir, 0o777); err != nil {
return "", "", err
}

Expand All @@ -375,7 +436,7 @@ func (tc testCase) xerox(root string) error {
}

// create if no exists
if err := os.MkdirAll(filepath.Join(root, directory), 0777); err != nil {
if err := os.MkdirAll(filepath.Join(root, directory), 0o777); err != nil {
return err
}

Expand All @@ -394,7 +455,7 @@ func cp(src, dst string) error {
return err
} else if data, err := ioutil.ReadFile(location); err != nil {
return err
} else if err := ioutil.WriteFile(filepath.Join(dst, filepath.Base(src)), data, 0600); err != nil {
} else if err := ioutil.WriteFile(filepath.Join(dst, filepath.Base(src)), data, 0o600); err != nil {
return err
}

Expand Down
2 changes: 2 additions & 0 deletions analyzer/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ func (config *defaultConfig) compileList() {
config.quick |= uint8(types.AnonInterface)
case types.NameStdLib:
config.quick |= uint8(types.NamedStdInterface)
case types.NameGeneric:
config.quick |= uint8(types.Generic)
}

// allow to parse regular expressions
Expand Down
4 changes: 2 additions & 2 deletions analyzer/internal/config/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

var ErrCollisionOfInterests = errors.New("can't have both `-accept` and `-reject` specified at same time")

//nolint: exhaustivestruct
// nolint: exhaustivestruct
func DefaultValidatorConfig() *allowConfig {
return allowAll([]string{
types.NameEmpty, // "empty": empty interfaces (interface{})
Expand Down Expand Up @@ -40,7 +40,7 @@ func New(fs *flag.FlagSet) (interface{}, error) {
return rejectAll(rejectList), nil
}

// can have none at same time.
// can have none (defaults are used) at same time.
return nil, nil
}

Expand Down
46 changes: 44 additions & 2 deletions analyzer/internal/types/iface.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,49 @@
package types

import (
"fmt"
"go/ast"
"go/token"

"golang.org/x/tools/go/analysis"
)

type IFace struct {
Name string // Preserved for named interfaces
Pos int // Position in return tuple
Name string // Interface name
Type IType // Type of the interface

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

func NewIssue(name string, interfaceType IType) IFace {
return IFace{
Name: name,
// Pos: pos,
Type: interfaceType,
}
}

func (i *IFace) Enrich(f *ast.FuncDecl) {
i.FuncName = f.Name.Name
i.Pos = f.Pos()
}

func (i IFace) String() string {
if i.Type == Generic {
return fmt.Sprintf("%s returns generic interface (%s)", i.FuncName, i.Name)
}

return fmt.Sprintf("%s returns interface (%s)", i.FuncName, i.Name)
}

func (i IFace) HashString() string {
return fmt.Sprintf("%v-%s", i.Pos, i.String())
}

func (i IFace) ExportDiagnostic() analysis.Diagnostic {
return analysis.Diagnostic{ //nolint: exhaustivestruct
Pos: i.Pos,
Message: i.String(),
}
}
Loading

0 comments on commit 947e10b

Please sign in to comment.