fix logic in IncrementVisitor

There used to be a logical bug where IncrementVisitor would
completely stop checking an expression/block after seeing a continue
statement. This led to issue #10058 where a variable incremented
(or otherwise modified) after any continue statement would still be
considered incremented only once.

The solution is to continue scanning the expression after seeing a
`continue` statement, but increment self.depth so that the Visitor
thinks that the rest of the loop is within a conditional.
This commit is contained in:
Eric Wu 2022-12-16 10:22:29 -05:00
parent 02f3959f2b
commit 97c12e0460
2 changed files with 33 additions and 7 deletions

View file

@ -25,7 +25,6 @@ pub(super) struct IncrementVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>, // context reference cx: &'a LateContext<'tcx>, // context reference
states: HirIdMap<IncrementVisitorVarState>, // incremented variables states: HirIdMap<IncrementVisitorVarState>, // incremented variables
depth: u32, // depth of conditional expressions depth: u32, // depth of conditional expressions
done: bool,
} }
impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> { impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
@ -34,7 +33,6 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
cx, cx,
states: HirIdMap::default(), states: HirIdMap::default(),
depth: 0, depth: 0,
done: false,
} }
} }
@ -51,10 +49,6 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.done {
return;
}
// If node is a variable // If node is a variable
if let Some(def_id) = path_to_local(expr) { if let Some(def_id) = path_to_local(expr) {
if let Some(parent) = get_parent_expr(self.cx, expr) { if let Some(parent) = get_parent_expr(self.cx, expr) {
@ -95,7 +89,9 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
walk_expr(self, expr); walk_expr(self, expr);
self.depth -= 1; self.depth -= 1;
} else if let ExprKind::Continue(_) = expr.kind { } else if let ExprKind::Continue(_) = expr.kind {
self.done = true; // If we see a `continue` block, then we increment depth so that the IncrementVisitor
// state will be set to DontWarn if we see the variable being modified anywhere afterwards.
self.depth += 1;
} else { } else {
walk_expr(self, expr); walk_expr(self, expr);
} }

View file

@ -189,3 +189,33 @@ mod issue_7920 {
} }
} }
} }
mod issue_10058 {
pub fn test() {
// should not lint since we are increasing counter potentially more than once in the loop
let values = [0, 1, 0, 1, 1, 1, 0, 1, 0, 1];
let mut counter = 0;
for value in values {
counter += 1;
if value == 0 {
continue;
}
counter += 1;
}
}
pub fn test2() {
// should not lint since we are increasing counter potentially more than once in the loop
let values = [0, 1, 0, 1, 1, 1, 0, 1, 0, 1];
let mut counter = 0;
for value in values {
counter += 1;
if value != 0 {
counter += 1;
}
}
}
}