Skip to content

Commit

Permalink
Merge pull request #14 from karamaru-alpha/update/karamaru/ignore-ren…
Browse files Browse the repository at this point in the history
…ame-by-default

update: ignore assigning the loop variable to another variable by default / add check-alias option
  • Loading branch information
karamaru-alpha committed Apr 9, 2024
2 parents 1954e6f + fbc4bad commit 777a221
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 69 deletions.
16 changes: 6 additions & 10 deletions copyloopvar.go
Expand Up @@ -10,7 +10,7 @@ import (
"golang.org/x/tools/go/ast/inspector"
)

var ignoreAlias bool
var checkAlias bool

func NewAnalyzer() *analysis.Analyzer {
analyzer := &analysis.Analyzer{
Expand All @@ -21,19 +21,15 @@ func NewAnalyzer() *analysis.Analyzer {
inspect.Analyzer,
},
}
analyzer.Flags.BoolVar(&ignoreAlias, "ignore-alias", false, "ignore aliasing of loop variables")
analyzer.Flags.BoolVar(&checkAlias, "check-alias", false, "check all assigning the loop variable to another variable")
return analyzer
}

func run(pass *analysis.Pass) (any, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

nodeFilter := []ast.Node{
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{
(*ast.RangeStmt)(nil),
(*ast.ForStmt)(nil),
}

inspect.Preorder(nodeFilter, func(n ast.Node) {
}, func(n ast.Node) {
switch node := n.(type) {
case *ast.RangeStmt:
checkRangeStmt(pass, node)
Expand Down Expand Up @@ -72,7 +68,7 @@ func checkRangeStmt(pass *analysis.Pass, rangeStmt *ast.RangeStmt) {
if right.Name != key.Name && (value == nil || right.Name != value.Name) {
continue
}
if ignoreAlias {
if !checkAlias {
left, ok := assignStmt.Lhs[i].(*ast.Ident)
if !ok {
continue
Expand Down Expand Up @@ -119,7 +115,7 @@ func checkForStmt(pass *analysis.Pass, forStmt *ast.ForStmt) {
if _, ok := initVarNameMap[right.Name]; !ok {
continue
}
if ignoreAlias {
if !checkAlias {
left, ok := assignStmt.Lhs[i].(*ast.Ident)
if !ok {
continue
Expand Down
6 changes: 3 additions & 3 deletions copyloopvar_test.go
Expand Up @@ -13,12 +13,12 @@ func TestAnalyzer(t *testing.T) {
analysistest.Run(t, testdata, NewAnalyzer(), "basic")
})

t.Run("ignore-alias", func(t *testing.T) {
t.Run("check-alias", func(t *testing.T) {
analyzer := NewAnalyzer()
if err := analyzer.Flags.Set("ignore-alias", "true"); err != nil {
if err := analyzer.Flags.Set("check-alias", "true"); err != nil {
t.Error(err)
}
testdata := testutil.WithModules(t, analysistest.TestData(), nil)
analysistest.Run(t, testdata, analyzer, "ignorealias")
analysistest.Run(t, testdata, analyzer, "checkalias")
})
}
37 changes: 21 additions & 16 deletions testdata/src/basic/main.go
Expand Up @@ -2,31 +2,36 @@ package main

func main() {
for i, v := range []int{1, 2, 3} {
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
_v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
a, b := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
c, d := 1, v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i
v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
_v := v
a, i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
b, _i := 1, i
c, v := 1, v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
d, _v := 1, v
e := false
_, _, _, _, _, _, _, _, _ = i, _i, v, _v, a, b, c, d, e
}

for i, j := 1, 1; i+j <= 3; i++ {
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
j := j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
_j := j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
a, b := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
c, d := 1, j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i
j := j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
_j := j
a, i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
b, _i := 1, i
c, j := 1, j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
d, _j := 1, j
e := false
_, _, _, _, _, _, _, _, _ = i, _i, j, _j, a, b, c, d, e
}

for i := range []int{1, 2, 3} {
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
a, b := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i
a, i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
b, _i := 1, i
c := false
_, _, _, _, _ = i, _i, a, b, c
}
Expand All @@ -35,6 +40,6 @@ func main() {
Bool bool
}
for _, t.Bool = range []bool{true, false} {
_ = t
t.Bool = t.Bool // NOTE: ignore
}
}
File renamed without changes.
45 changes: 45 additions & 0 deletions testdata/src/checkalias/main.go
@@ -0,0 +1,45 @@
package main

func main() {
for i, v := range []int{1, 2, 3} {
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
_v := v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
a, i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
b, _i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
c, v := 1, v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
d, _v := 1, v // want `The copy of the 'for' variable "v" can be deleted \(Go 1\.22\+\)`
e := false
_, _, _, _, _, _, _, _, _ = i, _i, v, _v, a, b, c, d, e
}

for i, j := 1, 1; i+j <= 3; i++ {
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
j := j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
_j := j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
a, i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
b, _i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
c, j := 1, j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
d, _j := 1, j // want `The copy of the 'for' variable "j" can be deleted \(Go 1\.22\+\)`
e := false
_, _, _, _, _, _, _, _, _ = i, _i, j, _j, a, b, c, d, e
}

for i := range []int{1, 2, 3} {
i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
_i := i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
a, i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
b, _i := 1, i // want `The copy of the 'for' variable "i" can be deleted \(Go 1\.22\+\)`
c := false
_, _, _, _, _ = i, _i, a, b, c
}

var t struct {
Bool bool
}
for _, t.Bool = range []bool{true, false} {
t.Bool = t.Bool // NOTE: ignore
}
}
40 changes: 0 additions & 40 deletions testdata/src/ignorealias/main.go

This file was deleted.

0 comments on commit 777a221

Please sign in to comment.