Skip to content

Commit

Permalink
SA9001: fix false positive in presense of control flow statements
Browse files Browse the repository at this point in the history
As noted in issue gh-488, let's check if there is either a 'return' or a
'break' statement when looping over channels.

Add a test for the same as noted in the issue!

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Closes: gh-1298 [via git-merge-pr]
Closes: gh-488
(cherry picked from commit bfa1dc5)
  • Loading branch information
KarthikNayak authored and dominikh committed Aug 17, 2023
1 parent 9fae04b commit 972a70e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
16 changes: 15 additions & 1 deletion staticcheck/lint.go
Expand Up @@ -1144,17 +1144,31 @@ func CheckDubiousDeferInChannelRangeLoop(pass *analysis.Pass) (interface{}, erro
if !ok {
return
}

stmts := []*ast.DeferStmt{}
exits := false
fn2 := func(node ast.Node) bool {
switch stmt := node.(type) {
case *ast.DeferStmt:
report.Report(pass, stmt, "defers in this range loop won't run unless the channel gets closed")
stmts = append(stmts, stmt)
case *ast.FuncLit:
// Don't look into function bodies
return false
case *ast.ReturnStmt:
exits = true
case *ast.BranchStmt:
exits = node.(*ast.BranchStmt).Tok == token.BREAK
}
return true
}
ast.Inspect(loop.Body, fn2)

if exits {
return
}
for _, stmt := range stmts {
report.Report(pass, stmt, "defers in this range loop won't run unless the channel gets closed")
}
}
code.Preorder(pass, fn, (*ast.RangeStmt)(nil))
return nil, nil
Expand Down
Expand Up @@ -6,3 +6,16 @@ func fn() {
defer println() //@ diag(`defers in this range loop`)
}
}

func fn2() {
var ch chan int
for range ch {
defer println()
break
}

for range ch {
defer println()
return
}
}

0 comments on commit 972a70e

Please sign in to comment.