Skip to content

Commit

Permalink
[flake8-bugbear] Avoid false positive for usage after continue (`…
Browse files Browse the repository at this point in the history
…B031`) (#10539)

## Summary

Closes #10337.

I've fixed the code to count usage of variable.
Usage count inside the block is reset when there is a following
statement.
- continue
- break
- return 

## Test Plan

Add test case.
  • Loading branch information
yt2b committed Mar 25, 2024
1 parent 021f0bd commit 22f237f
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 0 deletions.
43 changes: 43 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B031.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,49 @@ def collect_shop_items(shopper, items):
collect_shop_items("Jane", group[1])
collect_shop_items("Joe", group[1])

# Shouldn't trigger the warning when there is a continue, break statement.
for _section, section_items in groupby(items, key=lambda p: p[1]):
if _section == "greens":
collect_shop_items(shopper, section_items)
continue
elif _section == "frozen items":
collect_shop_items(shopper, section_items)
break
collect_shop_items(shopper, section_items)

# Shouldn't trigger the warning when there is a return statement.
for _section, section_items in groupby(items, key=lambda p: p[1]):
if _section == "greens":
collect_shop_items(shopper, section_items)
return
elif _section == "frozen items":
return section_items
collect_shop_items(shopper, section_items)

# Should trigger the warning for duplicate access, even if is a return statement after.
for _section, section_items in groupby(items, key=lambda p: p[1]):
if _section == "greens":
collect_shop_items(shopper, section_items)
collect_shop_items(shopper, section_items)
return

# Should trigger the warning for duplicate access, even if is a return in another branch.
for _section, section_items in groupby(items, key=lambda p: p[1]):
if _section == "greens":
collect_shop_items(shopper, section_items)
return
elif _section == "frozen items":
collect_shop_items(shopper, section_items)
collect_shop_items(shopper, section_items)

# Should trigger, since only one branch has a return statement.
for _section, section_items in groupby(items, key=lambda p: p[1]):
if _section == "greens":
collect_shop_items(shopper, section_items)
return
elif _section == "frozen items":
collect_shop_items(shopper, section_items)
collect_shop_items(shopper, section_items) # B031

# Let's redefine the `groupby` function to make sure we pick up the correct one.
# NOTE: This should always be at the end of the file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ impl<'a> GroupNameFinder<'a> {
self.usage_count += value;
}
}

/// Reset the usage count for the group name by the given value.
/// This function is called when there is a `continue`, `break`, or `return` statement.
fn reset_usage_count(&mut self) {
if let Some(last) = self.counter_stack.last_mut() {
*last.last_mut().unwrap() = 0;
} else {
self.usage_count = 0;
}
}
}

impl<'a> Visitor<'a> for GroupNameFinder<'a> {
Expand Down Expand Up @@ -197,6 +207,15 @@ impl<'a> Visitor<'a> for GroupNameFinder<'a> {
self.visit_expr(expr);
}
}
Stmt::Continue(_) | Stmt::Break(_) => {
self.reset_usage_count();
}
Stmt::Return(ast::StmtReturn { value, range: _ }) => {
if let Some(expr) = value {
self.visit_expr(expr);
}
self.reset_usage_count();
}
_ => visitor::walk_stmt(self, stmt),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,31 @@ B031.py:144:33: B031 Using the generator returned from `itertools.groupby()` mor
146 | for group in groupby(items, key=lambda p: p[1]):
|

B031.py:200:37: B031 Using the generator returned from `itertools.groupby()` more than once will do nothing on the second usage
|
198 | if _section == "greens":
199 | collect_shop_items(shopper, section_items)
200 | collect_shop_items(shopper, section_items)
| ^^^^^^^^^^^^^ B031
201 | return
|

B031.py:210:37: B031 Using the generator returned from `itertools.groupby()` more than once will do nothing on the second usage
|
208 | elif _section == "frozen items":
209 | collect_shop_items(shopper, section_items)
210 | collect_shop_items(shopper, section_items)
| ^^^^^^^^^^^^^ B031
211 |
212 | # Should trigger, since only one branch has a return statement.
|

B031.py:219:33: B031 Using the generator returned from `itertools.groupby()` more than once will do nothing on the second usage
|
217 | elif _section == "frozen items":
218 | collect_shop_items(shopper, section_items)
219 | collect_shop_items(shopper, section_items) # B031
| ^^^^^^^^^^^^^ B031
220 |
221 | # Let's redefine the `groupby` function to make sure we pick up the correct one.
|

0 comments on commit 22f237f

Please sign in to comment.