Skip to content

Commit

Permalink
Increment priority should be (branch-local, global) (#4070)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila authored Apr 23, 2023
1 parent 0e79140 commit d641466
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 20 deletions.
10 changes: 10 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_bugbear/B031.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ def collect_shop_items(shopper, items):
collect_shop_items("Joe", group[1])


# https://github.com/charliermarsh/ruff/issues/4050
for _section, section_items in itertools.groupby(items, key=lambda p: p[1]):
if _section == "greens":
for item in section_items:
collect_shop_items(shopper, item)
elif _section == "frozen items":
_ = [item for item in section_items]
else:
collect_shop_items(shopper, section_items)

# Make sure we ignore - but don't fail on more complicated invocations
for _key, (_value1, _value2) in groupby(
[("a", (1, 2)), ("b", (3, 4)), ("a", (5, 6))], key=lambda p: p[1]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ impl<'a> GroupNameFinder<'a> {
false
}
}

/// Increment the usage count for the group name by the given value.
/// If we're in one of the branches of a mutually exclusive statement,
/// then increment the count for that branch. Otherwise, increment the
/// global count.
fn increment_usage_count(&mut self, value: u32) {
if let Some(last) = self.counter_stack.last_mut() {
*last.last_mut().unwrap() += value;
} else {
self.usage_count += value;
}
}
}

impl<'a, 'b> Visitor<'b> for GroupNameFinder<'a>
Expand All @@ -109,7 +121,7 @@ where
self.overridden = true;
} else {
if self.name_matches(iter) {
self.usage_count += 1;
self.increment_usage_count(1);
// This could happen when the group is being looped
// over multiple times:
// for item in group:
Expand Down Expand Up @@ -178,11 +190,7 @@ where
// This is the max number of group usage from all the
// branches of this `if` statement.
let max_count = last.into_iter().max().unwrap_or(0);
if let Some(current_last) = self.counter_stack.last_mut() {
*current_last.last_mut().unwrap() += max_count;
} else {
self.usage_count += max_count;
}
self.increment_usage_count(max_count);
}
}
}
Expand All @@ -197,11 +205,7 @@ where
// This is the max number of group usage from all the
// branches of this `match` statement.
let max_count = last.into_iter().max().unwrap_or(0);
if let Some(current_last) = self.counter_stack.last_mut() {
*current_last.last_mut().unwrap() += max_count;
} else {
self.usage_count += max_count;
}
self.increment_usage_count(max_count);
}
}
StmtKind::Assign { targets, value, .. } => {
Expand Down Expand Up @@ -230,7 +234,7 @@ where
return;
}
if self.name_matches(&comprehension.iter) {
self.usage_count += 1;
self.increment_usage_count(1);
if self.usage_count > 1 {
self.exprs.push(&comprehension.iter);
}
Expand Down Expand Up @@ -275,14 +279,7 @@ where
}
_ => {
if self.name_matches(expr) {
// If the stack isn't empty, then we're in one of the branches of
// a mutually exclusive statement. Otherwise, we'll add it to the
// global count.
if let Some(last) = self.counter_stack.last_mut() {
*last.last_mut().unwrap() += 1;
} else {
self.usage_count += 1;
}
self.increment_usage_count(1);

let current_usage_count = self.usage_count
+ self
Expand Down

0 comments on commit d641466

Please sign in to comment.