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

chore: add more tests & API Change #22

Merged
merged 3 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
test: added golangci Issue tests + changed API
  • Loading branch information
butuzov committed May 15, 2023
commit 8b15c75ec5bb89564196dbfc0a554fea488183d9
12 changes: 12 additions & 0 deletions internal/checker/testdata/violations_new_lines.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
foobar
-- a.go --
package testdata


import (
"unicode/utf8"
)

// nothing as fix expected
var foo = utf8.ValidString(
string("foo"))
11 changes: 11 additions & 0 deletions internal/checker/testdata/violations_packages.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
foobar
-- a.go --
package testdata


import (
"unicode/utf8"
)

// nothing as fix expected
var foo = utf8.ValidString(string("foo"))
12 changes: 12 additions & 0 deletions internal/checker/testdata/violations_simple.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
foobar
-- a.go --
package testdata


import (
"unicode/utf8"
)

// we are only trying to match selector by description
// and we specially set
var foo = utf8.ValidString(string("foo"))
15 changes: 8 additions & 7 deletions internal/checker/violation.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,23 +149,24 @@ type GolangIssue struct {
Original string
}

// GolangCI-lint related diagnostic
func (v *Violation) Issue(pass *analysis.Pass) GolangIssue {
// Issue inteanded to be used only with golangci-lint, bu you can use use it
// alongside Diagnostic if you wish.
func (v *Violation) Issue(fSet *token.FileSet) GolangIssue {
issue := GolangIssue{
Start: pass.Fset.Position(v.callExpr.Pos()),
End: pass.Fset.Position(v.callExpr.End()),
Start: fSet.Position(v.callExpr.Pos()),
End: fSet.Position(v.callExpr.End()),
Message: v.Message(),
}

// original expression (useful for debug & requied for replace)
var buf bytes.Buffer
printer.Fprint(&buf, pass.Fset, v.callExpr)
printer.Fprint(&buf, fSet, v.callExpr)
issue.Original = buf.String()

noNl := strings.IndexByte(issue.Original, '\n') < 0

if v.Type == Method && noNl {
fix := v.suggest(pass.Fset)
fix := v.suggest(fSet)
issue.InlineFix = string(fix)
}

Expand All @@ -175,7 +176,7 @@ func (v *Violation) Issue(pass *analysis.Pass) GolangIssue {

// Hooray! we don't need to change package and redo imports.
if v.Type == Function && v.AltPackage == v.Package && noNl {
fix := v.suggest(pass.Fset)
fix := v.suggest(fSet)
issue.InlineFix = string(fix)
}

Expand Down
269 changes: 196 additions & 73 deletions internal/checker/violation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,94 @@ package checker

import (
"go/ast"
"go/importer"
"go/token"
"go/types"
"testing"

"github.com/stretchr/testify/assert"
"golang.org/x/tools/go/ast/inspector"
)

func TestViolation(t *testing.T) {
tests := []struct {
Name string
Violation Violation
Expression string
Base string
Args map[int]string
ExpectedSuggest string
Message string
}{
{
Name: "alt Caller",
Violation: Violation{
Targets: Strings,
Type: Function,
Package: "regexp",
Caller: "MatchString",
Args: []int{1},
AltPackage: "foobar",
AltCaller: "Match",
},
Expression: `regexp.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`},
Base: "regexp",
ExpectedSuggest: `regexp.Match("[0-9]+", "foo")`,
Message: `avoid allocations with foobar.Match`,
},
{
Name: "Has More Args Then WeNeed",
Violation: Violation{
Targets: Strings,
Type: Function,
Package: "regexp",
Caller: "MatchString",
Args: []int{1},
AltPackage: "foobar",
AltCaller: "Match",
},
Expression: `regexp.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`, 2: `"foo"`, 3: `"foo"`, 4: `"foo"`},
Base: "regexp",
ExpectedSuggest: `regexp.Match("[0-9]+", "foo")`,
Message: `avoid allocations with foobar.Match`,
},
{
Name: "Regular Suggestion Work",
Violation: Violation{
Targets: Strings,
Type: Function,
Package: "regexp",
Caller: "MatchString",
Args: []int{1},
AltCaller: "Match",
},
Expression: `regexp.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`},
Base: "regexp",
ExpectedSuggest: `regexp.Match("[0-9]+", "foo")`,
Message: `avoid allocations with regexp.Match`,
},
{
Name: "Methods",
Violation: Violation{
Targets: Strings,
Type: Method,
Package: "regexp",
Struct: "Regexp",
Caller: "MatchString",
Args: []int{1},
AltCaller: "Match",
},
Expression: `re.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`},
Base: "re",
ExpectedSuggest: `re.Match("[0-9]+", "foo")`,
Message: `avoid allocations with (*regexp.Regexp).Match`,
},
}

for i := range tests {
test := tests[i]
t.Run(test.Name, func(t *testing.T) {
Expand All @@ -28,80 +109,122 @@ func TestViolation(t *testing.T) {
}
}

var tests = []struct {
Name string
Violation Violation
Expression string
Base string
Args map[int]string
ExpectedSuggest string
Message string
}{
{
Name: "alt Caller",
Violation: Violation{
Targets: Strings,
Type: Function,
Package: "regexp",
Caller: "MatchString",
Args: []int{1},
AltPackage: "foobar",
AltCaller: "Match",
},
Expression: `regexp.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`},
Base: "regexp",
ExpectedSuggest: `regexp.Match("[0-9]+", "foo")`,
Message: `avoid allocations with foobar.Match`,
},
{
Name: "Has More Args Then WeNeed",
Violation: Violation{
Targets: Strings,
Type: Function,
Package: "regexp",
Caller: "MatchString",
Args: []int{1},
AltPackage: "foobar",
AltCaller: "Match",
func TestComplex(t *testing.T) {
tests := []struct {
Name string
TxtAr string
ImportPath string
Violation Violation
ExpectedMessage string
ExpectedFix string
}{
{
Name: "Simple Check",
TxtAr: "testdata/violations_simple.txtar",
ImportPath: "unicode/utf8",
Violation: Violation{
Type: Function,
Targets: Bytes,
Package: "unicode/utf8",
Caller: "ValidString",
AltCaller: "Valid",
Args: []int{0},
},
ExpectedMessage: `avoid allocations with utf8.Valid`,
ExpectedFix: `utf8.Valid("foo")`,
},
Expression: `regexp.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`, 2: `"foo"`, 3: `"foo"`, 4: `"foo"`},
Base: "regexp",
ExpectedSuggest: `regexp.Match("[0-9]+", "foo")`,
Message: `avoid allocations with foobar.Match`,
},
{
Name: "Regular Suggestion Work",
Violation: Violation{
Targets: Strings,
Type: Function,
Package: "regexp",
Caller: "MatchString",
Args: []int{1},
AltCaller: "Match",
{
Name: "Multiline Fix",
TxtAr: "testdata/violations_new_lines.txtar",
ImportPath: "unicode/utf8",
Violation: Violation{
Type: Function,
Targets: Bytes,
Package: "unicode/utf8",
Caller: "ValidString",
AltCaller: "Valid",
Args: []int{0},
},
ExpectedMessage: `avoid allocations with utf8.Valid`,
ExpectedFix: ``,
},
Expression: `regexp.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`},
Base: "regexp",
ExpectedSuggest: `regexp.Match("[0-9]+", "foo")`,
Message: `avoid allocations with regexp.Match`,
},
{
Name: "Methods",
Violation: Violation{
Targets: Strings,
Type: Method,
Package: "regexp",
Struct: "Regexp",
Caller: "MatchString",
Args: []int{1},
AltCaller: "Match",
{
Name: "Different Packages (No import)",
TxtAr: "testdata/violations_packages.txtar",
ImportPath: "unicode/utf8",
Violation: Violation{
Type: Function,
Targets: Bytes,
Package: "unicode/utf8",
AltPackage: "unicode/utf8v2",
Caller: "ValidString",
AltCaller: "Valid",
Args: []int{0},
},
ExpectedMessage: `avoid allocations with utf8v2.Valid`,
ExpectedFix: ``,
},
Expression: `re.MatchString("[0-9]+", []bytes("foo"))`,
Args: map[int]string{1: `"foo"`},
Base: "re",
ExpectedSuggest: `re.Match("[0-9]+", "foo")`,
Message: `avoid allocations with (*regexp.Regexp).Match`,
},
}

for i := range tests {
test := tests[i]
t.Run(test.Name, func(t *testing.T) {
fset := token.NewFileSet()
ar, err := Txtar(t, fset, test.TxtAr)
assert.Nil(t, err)

var (
ins = inspector.New(ar)
conf = types.Config{
Importer: importer.ForCompiler(fset, "source", nil),
}
info = types.Info{
Types: make(map[ast.Expr]types.TypeAndValue),
Defs: make(map[*ast.Ident]types.Object),
Uses: make(map[*ast.Ident]types.Object),
}
)

// ------ Setup ----------------------------------------------------------

_, err = conf.Check("source", fset, ar, &info)
assert.NoError(t, err)

check := New([]Violation{test.Violation})
check.Type = WrapType(&info)
check.Print = WrapPrint(fset)

var happend bool

ins.Preorder([]ast.Node{(*ast.CallExpr)(nil)}, func(n ast.Node) {
// allow to check only first call
if happend {
return
}
happend = true
// ---- test --------------------------------------------------
callExpr := n.(*ast.CallExpr)
expr := callExpr.Fun.(*ast.SelectorExpr)
x, ok := expr.X.(*ast.Ident)
assert.True(t, ok)

name := expr.Sel.Name
// skipping import checks with correct import path
v := check.Match(test.ImportPath, name)
assert.NotNil(t, v)
assert.Equal(t, *v, test.Violation)

args, found := check.Handle(v, callExpr)
assert.True(t, found, "no string to string conversions found")
v2 := v.With(check.Print(x), callExpr, args)

gciIssue := v2.Issue(fset)

assert.Equal(t, test.ExpectedFix, gciIssue.InlineFix, "fix not match")
assert.Equal(t, test.ExpectedMessage, gciIssue.Message, "message not match")
})

assert.True(t, happend, "Test Not Happend")
})
}
}