Skip to content

Commit

Permalink
S1011: reject dynamic lhs
Browse files Browse the repository at this point in the history
Closes: gh-1399
  • Loading branch information
dominikh committed Apr 28, 2023
1 parent 90a075d commit bb0858e
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
27 changes: 23 additions & 4 deletions simple/lint.go
Expand Up @@ -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
Expand All @@ -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
}

Expand All @@ -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) {
Expand Down
22 changes: 22 additions & 0 deletions simple/testdata/src/example.com/CheckLoopAppend/loop-append.go
Expand Up @@ -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])
}
}
Expand Up @@ -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])
}
}

0 comments on commit bb0858e

Please sign in to comment.