Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: initial release for generics support #42

Merged
merged 4 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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