Implement autofix for C416 (#568)

This commit is contained in:
Charlie Marsh 2022-11-03 11:05:08 -04:00 committed by GitHub
parent 2f3bebe5a2
commit 578ec4d843
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 107 additions and 7 deletions

View file

@ -453,7 +453,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| C413 | UnnecessaryCallAroundSorted | Unnecessary `(list\|reversed)` call around `sorted()` | | | C413 | UnnecessaryCallAroundSorted | Unnecessary `(list\|reversed)` call around `sorted()` | |
| C414 | UnnecessaryDoubleCastOrProcess | Unnecessary `(list\|reversed\|set\|sorted\|tuple)` call within `(list\|set\|sorted\|tuple)()` | | | C414 | UnnecessaryDoubleCastOrProcess | Unnecessary `(list\|reversed\|set\|sorted\|tuple)` call within `(list\|set\|sorted\|tuple)()` | |
| C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within `(reversed\|set\|sorted)()` | | | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within `(reversed\|set\|sorted)()` | |
| C416 | UnnecessaryComprehension | Unnecessary `(list\|set)` comprehension (rewrite using `(list\|set)()`) | | | C416 | UnnecessaryComprehension | Unnecessary `(list\|set)` comprehension (rewrite using `(list\|set)()`) | 🛠 |
| C417 | UnnecessaryMap | Unnecessary `map` usage (rewrite using a `(list\|set\|dict)` comprehension) | | | C417 | UnnecessaryMap | Unnecessary `map` usage (rewrite using a `(list\|set\|dict)` comprehension) | |
### flake8-bugbear ### flake8-bugbear

View file

@ -1363,6 +1363,8 @@ where
expr, expr,
elt, elt,
generators, generators,
self.locator,
self.patch(),
self.locate_check(Range::from_located(expr)), self.locate_check(Range::from_located(expr)),
) { ) {
self.checks.push(check); self.checks.push(check);

View file

@ -1470,6 +1470,7 @@ impl CheckKind {
| CheckKind::TypeOfPrimitive(_) | CheckKind::TypeOfPrimitive(_)
| CheckKind::UnnecessaryAbspath | CheckKind::UnnecessaryAbspath
| CheckKind::UnnecessaryCollectionCall(_) | CheckKind::UnnecessaryCollectionCall(_)
| CheckKind::UnnecessaryComprehension(_)
| CheckKind::UnnecessaryGeneratorDict | CheckKind::UnnecessaryGeneratorDict
| CheckKind::UnnecessaryGeneratorList | CheckKind::UnnecessaryGeneratorList
| CheckKind::UnnecessaryGeneratorSet | CheckKind::UnnecessaryGeneratorSet

View file

@ -443,6 +443,8 @@ pub fn unnecessary_comprehension(
expr: &Expr, expr: &Expr,
elt: &Expr, elt: &Expr,
generators: &[Comprehension], generators: &[Comprehension],
locator: &SourceCodeLocator,
fix: bool,
location: Range, location: Range,
) -> Option<Check> { ) -> Option<Check> {
if generators.len() != 1 { if generators.len() != 1 {
@ -462,10 +464,17 @@ pub fn unnecessary_comprehension(
ExprKind::SetComp { .. } => "set", ExprKind::SetComp { .. } => "set",
_ => return None, _ => return None,
}; };
Some(Check::new( let mut check = Check::new(
CheckKind::UnnecessaryComprehension(expr_kind.to_string()), CheckKind::UnnecessaryComprehension(expr_kind.to_string()),
location, location,
)) );
if fix {
match fixes::fix_unnecessary_comprehension(locator, expr) {
Ok(fix) => check.amend(fix),
Err(e) => error!("Failed to generate fix: {}", e),
}
}
Some(check)
} }
/// C417 /// C417

View file

@ -1,7 +1,7 @@
use anyhow::Result; use anyhow::Result;
use libcst_native::{ use libcst_native::{
Arg, Call, Codegen, Dict, DictComp, DictElement, Element, Expr, Expression, LeftCurlyBrace, Arg, Call, Codegen, Dict, DictComp, DictElement, Element, Expr, Expression, LeftCurlyBrace,
LeftParen, LeftSquareBracket, List, ListComp, Module, ParenthesizableWhitespace, LeftParen, LeftSquareBracket, List, ListComp, Module, Name, ParenthesizableWhitespace,
RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace, RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace,
SmallStatement, Statement, Tuple, SmallStatement, Statement, Tuple,
}; };
@ -503,7 +503,7 @@ pub fn fix_unnecessary_literal_within_list_call(
)) ))
} }
/// (C411) Convert `list([i for i in x])` to `[i for i in x]`. /// (C411) Convert `list([i * i for i in x])` to `[i * i for i in x]`.
pub fn fix_unnecessary_list_call( pub fn fix_unnecessary_list_call(
locator: &SourceCodeLocator, locator: &SourceCodeLocator,
expr: &rustpython_ast::Expr, expr: &rustpython_ast::Expr,
@ -525,3 +525,73 @@ pub fn fix_unnecessary_list_call(
expr.end_location.unwrap(), expr.end_location.unwrap(),
)) ))
} }
/// (C416) Convert `[i for i in x]` to `list(x)`.
pub fn fix_unnecessary_comprehension(
locator: &SourceCodeLocator,
expr: &rustpython_ast::Expr,
) -> Result<Fix> {
let mut tree = match_tree(locator, expr)?;
let mut body = match_expr(&mut tree)?;
match &body.value {
Expression::ListComp(inner) => {
body.value = Expression::Call(Box::new(Call {
func: Box::new(Expression::Name(Box::new(Name {
value: "list",
lpar: Default::default(),
rpar: Default::default(),
}))),
args: vec![Arg {
value: inner.for_in.iter.clone(),
keyword: Default::default(),
equal: Default::default(),
comma: Default::default(),
star: Default::default(),
whitespace_after_star: Default::default(),
whitespace_after_arg: Default::default(),
}],
lpar: Default::default(),
rpar: Default::default(),
whitespace_after_func: Default::default(),
whitespace_before_args: Default::default(),
}))
}
Expression::SetComp(inner) => {
body.value = Expression::Call(Box::new(Call {
func: Box::new(Expression::Name(Box::new(Name {
value: "set",
lpar: Default::default(),
rpar: Default::default(),
}))),
args: vec![Arg {
value: inner.for_in.iter.clone(),
keyword: Default::default(),
equal: Default::default(),
comma: Default::default(),
star: Default::default(),
whitespace_after_star: Default::default(),
whitespace_after_arg: Default::default(),
}],
lpar: Default::default(),
rpar: Default::default(),
whitespace_after_func: Default::default(),
whitespace_before_args: Default::default(),
}))
}
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::ListComp | Expression:SetComp."
))
}
}
let mut state = Default::default();
tree.codegen(&mut state);
Ok(Fix::replacement(
state.to_string(),
expr.location,
expr.end_location.unwrap(),
))
}

View file

@ -10,7 +10,16 @@ expression: checks
end_location: end_location:
row: 2 row: 2
column: 14 column: 14
fix: ~ fix:
patch:
content: list(x)
location:
row: 2
column: 0
end_location:
row: 2
column: 14
applied: false
- kind: - kind:
UnnecessaryComprehension: set UnnecessaryComprehension: set
location: location:
@ -19,5 +28,14 @@ expression: checks
end_location: end_location:
row: 3 row: 3
column: 14 column: 14
fix: ~ fix:
patch:
content: set(x)
location:
row: 3
column: 0
end_location:
row: 3
column: 14
applied: false