Skip to content

Commit

Permalink
[refurb] Implement reimplemented-starmap rule (FURB140) (#7253)
Browse files Browse the repository at this point in the history
## Summary

This PR is part of a bigger effort of re-implementing `refurb` rules
#1348. It adds support for
[FURB140](https://github.com/dosisod/refurb/blob/master/refurb/checks/itertools/use_starmap.py)

## Test Plan

I included a new test + checked that all other tests pass.
  • Loading branch information
SavchenkoValeriy committed Sep 19, 2023
1 parent c6ba7df commit 4123d07
Show file tree
Hide file tree
Showing 9 changed files with 562 additions and 16 deletions.
43 changes: 43 additions & 0 deletions crates/ruff/resources/test/fixtures/refurb/FURB140.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
def zipped():
return zip([1, 2, 3], "ABC")

# Errors.

# FURB140
[print(x, y) for x, y in zipped()]

# FURB140
(print(x, y) for x, y in zipped())

# FURB140
{print(x, y) for x, y in zipped()}


from itertools import starmap as sm

# FURB140
[print(x, y) for x, y in zipped()]

# FURB140
(print(x, y) for x, y in zipped())

# FURB140
{print(x, y) for x, y in zipped()}

# Non-errors.

[print(x, int) for x, _ in zipped()]

[print(x, *y) for x, y in zipped()]

[print(x, y, 1) for x, y in zipped()]

[print(y, x) for x, y in zipped()]

[print(x + 1, y) for x, y in zipped()]

[print(x) for x in range(100)]

[print() for x, y in zipped()]

[print(x, end=y) for x, y in zipped()]
59 changes: 44 additions & 15 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1279,16 +1279,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
flake8_simplify::rules::twisted_arms_in_ifexpr(checker, expr, test, body, orelse);
}
}
Expr::ListComp(ast::ExprListComp {
elt,
generators,
range: _,
})
| Expr::SetComp(ast::ExprSetComp {
elt,
generators,
range: _,
}) => {
Expr::ListComp(
comp @ ast::ExprListComp {
elt,
generators,
range: _,
},
) => {
if checker.enabled(Rule::UnnecessaryComprehension) {
flake8_comprehensions::rules::unnecessary_list_set_comprehension(
checker, expr, elt, generators,
Expand All @@ -1302,6 +1299,33 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pylint::rules::iteration_over_set(checker, &generator.iter);
}
}
if checker.enabled(Rule::ReimplementedStarmap) {
refurb::rules::reimplemented_starmap(checker, &comp.into());
}
}
Expr::SetComp(
comp @ ast::ExprSetComp {
elt,
generators,
range: _,
},
) => {
if checker.enabled(Rule::UnnecessaryComprehension) {
flake8_comprehensions::rules::unnecessary_list_set_comprehension(
checker, expr, elt, generators,
);
}
if checker.enabled(Rule::FunctionUsesLoopVariable) {
flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Expr(expr));
}
if checker.enabled(Rule::IterationOverSet) {
for generator in generators {
pylint::rules::iteration_over_set(checker, &generator.iter);
}
}
if checker.enabled(Rule::ReimplementedStarmap) {
refurb::rules::reimplemented_starmap(checker, &comp.into());
}
}
Expr::DictComp(ast::ExprDictComp {
key,
Expand All @@ -1326,11 +1350,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
ruff::rules::static_key_dict_comprehension(checker, key);
}
}
Expr::GeneratorExp(ast::ExprGeneratorExp {
generators,
elt: _,
range: _,
}) => {
Expr::GeneratorExp(
generator @ ast::ExprGeneratorExp {
generators,
elt: _,
range: _,
},
) => {
if checker.enabled(Rule::FunctionUsesLoopVariable) {
flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Expr(expr));
}
Expand All @@ -1339,6 +1365,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pylint::rules::iteration_over_set(checker, &generator.iter);
}
}
if checker.enabled(Rule::ReimplementedStarmap) {
refurb::rules::reimplemented_starmap(checker, &generator.into());
}
}
Expr::BoolOp(
bool_op @ ast::ExprBoolOp {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "131") => (RuleGroup::Nursery, rules::refurb::rules::DeleteFullSlice),
#[allow(deprecated)]
(Refurb, "132") => (RuleGroup::Nursery, rules::refurb::rules::CheckAndRemoveFromSet),
(Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
(Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),

// flake8-logging
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod tests {
#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
#[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
#[test_case(Rule::CheckAndRemoveFromSet, Path::new("FURB132.py"))]
#[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))]
#[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
pub(crate) use check_and_remove_from_set::*;
pub(crate) use delete_full_slice::*;
pub(crate) use reimplemented_starmap::*;
pub(crate) use repeated_append::*;
pub(crate) use slice_copy::*;

mod check_and_remove_from_set;
mod delete_full_slice;
mod reimplemented_starmap;
mod repeated_append;
mod slice_copy;

0 comments on commit 4123d07

Please sign in to comment.