Skip to content

Commit 48d0401

Browse files
AlexanderAsmakovAlexanderAsmakov
authored
Merge pull request #42 from avito-tech/CTHL-5402-fix-make-ignore
Fixed corner cases when mutating arguments in the make function Fixed the regular expression for searching for a build tag in a test file
2 parents d5530ff + 9da8888 commit 48d0401

File tree

6 files changed

+143
-43
lines changed

6 files changed

+143
-43
lines changed

‎internal/filter/skip_mutation.go‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,12 @@ func (s *SkipMakeArgsFilter) Collect(file *ast.File, _ *token.FileSet, _ string)
2525
arg0 := callExpr.Args[0]
2626
_, isArray := arg0.(*ast.ArrayType)
2727
_, isMap := arg0.(*ast.MapType)
28-
if isArray || isMap {
29-
if lit, ok := callExpr.Args[1].(*ast.BasicLit); ok && lit.Kind == token.INT {
30-
s.IgnoredNodes[lit.Pos()] = callExpr
31-
}
28+
_, isIdent := arg0.(*ast.Ident)
29+
if isArray || isMap || isIdent {
30+
s.collectForIgnoredNodes(callExpr.Args[1], callExpr)
31+
3232
if len(callExpr.Args) > 2 {
33-
if lit, ok := callExpr.Args[2].(*ast.BasicLit); ok && lit.Kind == token.INT {
34-
s.IgnoredNodes[lit.Pos()] = callExpr
35-
}
33+
s.collectForIgnoredNodes(callExpr.Args[2], callExpr)
3634
}
3735
return false
3836
}
@@ -47,3 +45,37 @@ func (s *SkipMakeArgsFilter) ShouldSkip(node ast.Node, _ string) bool {
4745
_, exists := s.IgnoredNodes[node.Pos()]
4846
return exists
4947
}
48+
49+
// collectForIgnoredNodes recursively collects all numeric literals and unary/binary operators in an expression
50+
func (s *SkipMakeArgsFilter) collectForIgnoredNodes(expr ast.Expr, callExpr *ast.CallExpr) {
51+
switch e := expr.(type) {
52+
case *ast.BasicLit:
53+
// Direct numeric literal
54+
if e.Kind == token.INT {
55+
s.IgnoredNodes[e.Pos()] = callExpr
56+
}
57+
case *ast.BinaryExpr:
58+
// Binary operations (addition, subtraction, multiplication, division, etc.)
59+
s.IgnoredNodes[e.OpPos] = callExpr
60+
s.collectForIgnoredNodes(e.X, callExpr)
61+
s.collectForIgnoredNodes(e.Y, callExpr)
62+
case *ast.CallExpr:
63+
// Calling a function (e.g. len())
64+
for _, arg := range e.Args {
65+
s.collectForIgnoredNodes(arg, callExpr)
66+
}
67+
case *ast.ParenExpr:
68+
// Expression in brackets
69+
s.collectForIgnoredNodes(e.X, callExpr)
70+
case *ast.UnaryExpr:
71+
// Unary operators (+, -, etc.)
72+
if xLit, ok := e.X.(*ast.BasicLit); ok && xLit.Kind == token.INT {
73+
// If a unary operator is applied to a numeric literal, both the operator and the literal itself are added.
74+
s.IgnoredNodes[e.OpPos] = callExpr
75+
s.IgnoredNodes[xLit.Pos()] = callExpr
76+
} else {
77+
// Otherwise, we continue the tour inside.
78+
s.collectForIgnoredNodes(e.X, callExpr)
79+
}
80+
}
81+
}

‎internal/filter/skip_mutation_test.go‎

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,22 @@ import (
1111

1212
func TestSkipMutationForInitSlicesAndMaps(t *testing.T) {
1313
tests := []struct {
14-
name string
15-
code string
16-
expected bool
14+
name string
15+
code string
16+
expectedLiterals []string
17+
expectedOperators []string
1718
}{
1819
{
19-
name: "skip mutation for slice init with len",
20-
code: `package main; var a = make([]int, 10)`,
21-
expected: true,
20+
name: "skip mutation for slice init with len",
21+
code: `package main; var a = make([]int, 10)`,
22+
expectedLiterals: []string{"10"},
23+
expectedOperators: []string{},
2224
},
2325
{
24-
name: "skip mutation for slice init with len and cap",
25-
code: `package main; var a = make([]int, 10, 20)`,
26-
expected: true,
26+
name: "skip mutation for slice init with len and cap",
27+
code: `package main; var a = make([]int, 10, 20)`,
28+
expectedLiterals: []string{"10", "20"},
29+
expectedOperators: []string{},
2730
},
2831
{
2932
name: "skip mutation for slice init in assigning inside a struct",
@@ -39,22 +42,50 @@ func TestSkipMutationForInitSlicesAndMaps(t *testing.T) {
3942
func fff() {
4043
testCase := &TestCase{ Devices: make([]DeviceStatus, 0) }
4144
}`,
42-
expected: true,
45+
expectedLiterals: []string{"0"},
46+
expectedOperators: []string{},
4347
},
4448
{
45-
name: "skip mutation for map init with cap",
46-
code: `package main; var a = make(map[int]bool, 0)`,
47-
expected: true,
49+
name: "skip mutation for map init with cap",
50+
code: `package main; var a = make(map[int]bool, 0)`,
51+
expectedLiterals: []string{"0"},
52+
expectedOperators: []string{},
4853
},
4954
{
50-
name: "do not skip mutation for slice init with variable",
51-
code: `package main; var x = 10; var a = make([]int, x)`,
52-
expected: false,
55+
name: "do not skip mutation for slice init with variable",
56+
code: `package main; var x = 10; var a = make([]int, x)`,
57+
expectedLiterals: []string{},
58+
expectedOperators: []string{},
5359
},
5460
{
55-
name: "do not skip mutation for other literals",
56-
code: `package main; var a = 42`,
57-
expected: false,
61+
name: "do not skip mutation for other literals",
62+
code: `package main; var a = 42`,
63+
expectedLiterals: []string{},
64+
expectedOperators: []string{},
65+
},
66+
{
67+
name: "skip mutation for slice with type alias",
68+
code: `package main; type Contents []*string; var a = make(Contents, 5)`,
69+
expectedLiterals: []string{"5"},
70+
expectedOperators: []string{},
71+
},
72+
{
73+
name: "skip mutation for complex expression with binary op",
74+
code: `package main; var a = make([]int, 0, len(arr)+1)`,
75+
expectedLiterals: []string{"0", "1"},
76+
expectedOperators: []string{"+"},
77+
},
78+
{
79+
name: "skip mutation for unary operations",
80+
code: `package main; var a = make([]int, -5, +10)`,
81+
expectedLiterals: []string{"5", "10"},
82+
expectedOperators: []string{"-", "+"},
83+
},
84+
{
85+
name: "skip mutation for complex nested expressions",
86+
code: `package main; var a = make([]int, (3), len(arr)+2*4)`,
87+
expectedLiterals: []string{"3", "2", "4"},
88+
expectedOperators: []string{"+", "*"},
5889
},
5990
}
6091

@@ -67,20 +98,35 @@ func TestSkipMutationForInitSlicesAndMaps(t *testing.T) {
6798
}
6899

69100
s := NewSkipMakeArgsFilter()
70-
s.Collect(node, nil, "")
71-
s.ShouldSkip(node, "")
101+
s.Collect(node, fs, "")
102+
103+
var foundLiterals []string
104+
var foundOperators []string
72105

73-
var result bool
74106
ast.Inspect(node, func(n ast.Node) bool {
75107
if lit, ok := n.(*ast.BasicLit); ok && lit.Kind == token.INT {
76-
_, found := s.IgnoredNodes[lit.Pos()]
77-
result = found
78-
return false
108+
if _, exists := s.IgnoredNodes[lit.Pos()]; exists {
109+
foundLiterals = append(foundLiterals, lit.Value)
110+
}
111+
}
112+
113+
if binExpr, ok := n.(*ast.BinaryExpr); ok {
114+
if _, exists := s.IgnoredNodes[binExpr.OpPos]; exists {
115+
foundOperators = append(foundOperators, binExpr.Op.String())
116+
}
79117
}
118+
119+
if unaryExpr, ok := n.(*ast.UnaryExpr); ok {
120+
if _, exists := s.IgnoredNodes[unaryExpr.OpPos]; exists {
121+
foundOperators = append(foundOperators, unaryExpr.Op.String())
122+
}
123+
}
124+
80125
return true
81126
})
82127

83-
assert.Equal(t, tt.expected, result)
128+
assert.ElementsMatch(t, tt.expectedLiterals, foundLiterals, "Literals mismatch")
129+
assert.ElementsMatch(t, tt.expectedOperators, foundOperators, "Operators mismatch")
84130
})
85131
}
86132
}

‎internal/importing/filepath.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ import (
2323
"github.com/avito-tech/go-mutesting/internal/models"
2424
)
2525

26+
var (
27+
buildTagRegex = regexp.MustCompile(`(?m)^//(go:build|\s*\+build)`)
28+
testFileSuffix = "_test.go"
29+
)
30+
2631
func packagesWithFilesOfArgs(args []string, opts *models.Options) map[string]map[string]struct{} {
2732
var filenames []string
2833

@@ -48,10 +53,6 @@ func packagesWithFilesOfArgs(args []string, opts *models.Options) map[string]map
4853

4954
fileLookup := make(map[string]struct{})
5055
pkgs := make(map[string]map[string]struct{})
51-
var re *regexp.Regexp
52-
if opts.Config.SkipFileWithBuildTag {
53-
re = regexp.MustCompile("\\+build (.*)(\\s+)package") //nolint:gosimple
54-
}
5556

5657
for _, filename := range filenames {
5758
if _, ok := fileLookup[filename]; ok {
@@ -72,7 +73,7 @@ func packagesWithFilesOfArgs(args []string, opts *models.Options) map[string]map
7273
}
7374
}
7475

75-
if strings.HasSuffix(filename, "_test.go") { // ignore test files
76+
if strings.HasSuffix(filename, testFileSuffix) { // ignore test files
7677
continue
7778
}
7879

@@ -82,13 +83,13 @@ func packagesWithFilesOfArgs(args []string, opts *models.Options) map[string]map
8283
continue
8384
}
8485

85-
testName := filename[:nameSize-3] + "_test.go"
86+
testName := filename[:nameSize-3] + testFileSuffix
8687
if !exists(testName) {
8788
continue
8889
}
8990

9091
if opts.Config.SkipFileWithBuildTag { // ignore files with test with build tags
91-
isBuildTag := regexpSearchInFile(testName, re)
92+
isBuildTag := regexpSearchInFile(testName, buildTagRegex)
9293
if isBuildTag {
9394
continue
9495
}

‎internal/importing/filepath_test.go‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ package importing
22

33
import (
44
"fmt"
5-
"github.com/avito-tech/go-mutesting/internal/models"
65
"os"
76
"testing"
87

8+
"github.com/avito-tech/go-mutesting/internal/models"
9+
910
"github.com/stretchr/testify/assert"
1011
)
1112

@@ -29,11 +30,12 @@ func TestFilesOfArgs(t *testing.T) {
2930
// directories
3031
{
3132
[]string{"./filepathfixtures"},
32-
[]string{"filepathfixtures/first.go", "filepathfixtures/second.go", "filepathfixtures/third.go"},
33+
[]string{"filepathfixtures/fifth.go", "filepathfixtures/first.go", "filepathfixtures/second.go", "filepathfixtures/third.go"},
3334
},
3435
{
3536
[]string{"../importing/filepathfixtures"},
3637
[]string{
38+
"../importing/filepathfixtures/fifth.go",
3739
"../importing/filepathfixtures/first.go",
3840
"../importing/filepathfixtures/second.go",
3941
"../importing/filepathfixtures/third.go",
@@ -43,6 +45,7 @@ func TestFilesOfArgs(t *testing.T) {
4345
{
4446
[]string{"github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures"},
4547
[]string{
48+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
4649
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/first.go",
4750
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
4851
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",
@@ -51,6 +54,7 @@ func TestFilesOfArgs(t *testing.T) {
5154
{
5255
[]string{"github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/..."},
5356
[]string{
57+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
5458
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/first.go",
5559
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
5660
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",
@@ -86,6 +90,7 @@ func TestPackagesWithFilesOfArgs(t *testing.T) {
8690
{
8791
[]string{"./filepathfixtures"},
8892
[]Package{{Name: "filepathfixtures", Files: []string{
93+
"filepathfixtures/fifth.go",
8994
"filepathfixtures/first.go",
9095
"filepathfixtures/second.go",
9196
"filepathfixtures/third.go",
@@ -94,6 +99,7 @@ func TestPackagesWithFilesOfArgs(t *testing.T) {
9499
{
95100
[]string{"../importing/filepathfixtures"},
96101
[]Package{{Name: "../importing/filepathfixtures", Files: []string{
102+
"../importing/filepathfixtures/fifth.go",
97103
"../importing/filepathfixtures/first.go",
98104
"../importing/filepathfixtures/second.go",
99105
"../importing/filepathfixtures/third.go",
@@ -105,6 +111,7 @@ func TestPackagesWithFilesOfArgs(t *testing.T) {
105111
[]Package{{
106112
Name: p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures",
107113
Files: []string{
114+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
108115
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/first.go",
109116
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
110117
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",
@@ -117,6 +124,7 @@ func TestPackagesWithFilesOfArgs(t *testing.T) {
117124
{
118125
Name: p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures",
119126
Files: []string{
127+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
120128
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/first.go",
121129
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
122130
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",
@@ -157,12 +165,13 @@ func TestFilesWithSkipWithoutTests(t *testing.T) {
157165
// directories
158166
{
159167
[]string{"./filepathfixtures"},
160-
[]string{"filepathfixtures/second.go", "filepathfixtures/third.go"},
168+
[]string{"filepathfixtures/fifth.go", "filepathfixtures/second.go", "filepathfixtures/third.go"},
161169
},
162170
// packages
163171
{
164172
[]string{"github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/..."},
165173
[]string{
174+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
166175
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
167176
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",
168177
},
@@ -192,6 +201,10 @@ func TestFilesWithSkipWithBuildTagsTests(t *testing.T) {
192201
[]string{"./filepathfixtures/third.go"},
193202
[]string(nil),
194203
},
204+
{
205+
[]string{"./filepathfixtures/fifth.go"},
206+
[]string(nil),
207+
},
195208
{
196209
[]string{"./filepathfixtures/second.go"},
197210
[]string{"./filepathfixtures/second.go"},
@@ -250,6 +263,7 @@ func TestFilesWithExcludedDirs(t *testing.T) {
250263
{
251264
[]string{"./filepathfixtures/..."},
252265
[]string{
266+
"filepathfixtures/fifth.go",
253267
"filepathfixtures/first.go",
254268
"filepathfixtures/second.go",
255269
"filepathfixtures/third.go",
@@ -269,6 +283,7 @@ func TestFilesWithExcludedDirs(t *testing.T) {
269283
{
270284
[]string{"./filepathfixtures"},
271285
[]string{
286+
"filepathfixtures/fifth.go",
272287
"filepathfixtures/first.go",
273288
"filepathfixtures/second.go",
274289
"filepathfixtures/third.go",
@@ -280,6 +295,7 @@ func TestFilesWithExcludedDirs(t *testing.T) {
280295
{
281296
[]string{"github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/..."},
282297
[]string{
298+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
283299
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/first.go",
284300
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
285301
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",
@@ -290,6 +306,7 @@ func TestFilesWithExcludedDirs(t *testing.T) {
290306
{
291307
[]string{"github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/..."},
292308
[]string{
309+
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/fifth.go",
293310
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/first.go",
294311
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/second.go",
295312
p + "github.com/avito-tech/go-mutesting/internal/importing/filepathfixtures/third.go",

‎internal/importing/filepathfixtures/fifth.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package filepathfixtures

‎internal/importing/filepathfixtures/fifth_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
//go:build integration
2+
3+
package filepathfixtures

0 commit comments

Comments
 (0)