diff --git a/simple/lint.go b/simple/lint.go index 08426436..8458f7be 100644 --- a/simple/lint.go +++ b/simple/lint.go @@ -778,13 +778,13 @@ var checkLoopAppendQ = pattern.MustParse(` x [(AssignStmt [lhs] "=" [(CallExpr (Builtin "append") [lhs val])])]) (RangeStmt - idx@(Ident _) + idx@(Object _) nil _ x [(AssignStmt [lhs] "=" [(CallExpr (Builtin "append") [lhs (IndexExpr x idx)])])]) (RangeStmt - idx@(Ident _) + idx@(Object _) nil _ x @@ -800,8 +800,7 @@ func CheckLoopAppend(pass *analysis.Pass) (interface{}, error) { return } - val, ok := m.State["val"].(types.Object) - if ok && refersTo(pass, m.State["lhs"].(ast.Expr), val) { + if val, ok := m.State["val"].(types.Object); ok && refersTo(pass, m.State["lhs"].(ast.Expr), val) { return } @@ -811,6 +810,26 @@ func CheckLoopAppend(pass *analysis.Pass) (interface{}, error) { return } + if idx, ok := m.State["idx"].(types.Object); ok && refersTo(pass, m.State["lhs"].(ast.Expr), idx) { + // The lhs mustn't refer to the index loop variable. + return + } + + if code.MayHaveSideEffects(pass, m.State["lhs"].(ast.Expr), pure) { + // The lhs may be dynamic and return different values on each iteration. For example: + // + // func bar() map[int][]int { /* return one of several maps */ } + // + // func foo(x []int, y [][]int) { + // for i := range x { + // bar()[0] = append(bar()[0], x[i]) + // } + // } + // + // The dynamic nature of the lhs might also affect the value of the index. + return + } + src := pass.TypesInfo.TypeOf(m.State["x"].(ast.Expr)) dst := pass.TypesInfo.TypeOf(m.State["lhs"].(ast.Expr)) if !types.Identical(src, dst) { diff --git a/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go b/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go index 19dec137..fd093c6b 100644 --- a/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go +++ b/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go @@ -109,3 +109,25 @@ func fn7() { x = append(x, fn6()[i]) } } + +func fn8() { + // The lhs isn't allowed to refer to i + var i int + var x []int + var y [][]int + for i = range x { + y[i] = append(y[i], x[i]) + } + for i := range x { + y[i] = append(y[i], x[i]) + } +} + +func fn9() { + // The lhs isn't allowed to have side effects + bar := func() map[int][]int { return nil } + var x []int + for i := range x { + bar()[0] = append(bar()[0], x[i]) + } +} diff --git a/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go.golden b/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go.golden index cf2fcae7..b12a2026 100644 --- a/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go.golden +++ b/simple/testdata/src/example.com/CheckLoopAppend/loop-append.go.golden @@ -99,3 +99,25 @@ func fn7() { x = append(x, fn6()[i]) } } + +func fn8() { + // The lhs isn't allowed to refer to i + var i int + var x []int + var y [][]int + for i = range x { + y[i] = append(y[i], x[i]) + } + for i := range x { + y[i] = append(y[i], x[i]) + } +} + +func fn9() { + // The lhs isn't allowed to have side effects + bar := func() map[int][]int { return nil } + var x []int + for i := range x { + bar()[0] = append(bar()[0], x[i]) + } +}