Implement autofix for dict and tuple comprehensions (#555)

This commit is contained in:
Charlie Marsh 2022-11-02 21:36:20 -04:00 committed by GitHub
parent 416c338237
commit 6a180b95d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 476 additions and 45 deletions

View File

@ -394,14 +394,14 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| ---- | ---- | ------- | --- |
| C400 | UnnecessaryGeneratorList | Unnecessary generator (rewrite as a `list` comprehension) | 🛠 |
| C401 | UnnecessaryGeneratorSet | Unnecessary generator (rewrite as a `set` comprehension) | 🛠 |
| C402 | UnnecessaryGeneratorDict | Unnecessary generator (rewrite as a `dict` comprehension) | |
| C402 | UnnecessaryGeneratorDict | Unnecessary generator (rewrite as a `dict` comprehension) | 🛠 |
| C403 | UnnecessaryListComprehensionSet | Unnecessary `list` comprehension (rewrite as a `set` comprehension) | 🛠 |
| C404 | UnnecessaryListComprehensionDict | Unnecessary `list` comprehension (rewrite as a `dict` comprehension) | |
| C405 | UnnecessaryLiteralSet | Unnecessary `(list\|tuple)` literal (rewrite as a `set` literal) | 🛠 |
| C406 | UnnecessaryLiteralDict | Unnecessary `(list\|tuple)` literal (rewrite as a `dict` literal) | |
| C408 | UnnecessaryCollectionCall | Unnecessary `(dict\|list\|tuple)` call (rewrite as a literal) | 🛠 |
| C409 | UnnecessaryLiteralWithinTupleCall | Unnecessary `(list\|tuple)` literal passed to `tuple()` (remove the outer call to `tuple()`) | |
| C410 | UnnecessaryLiteralWithinListCall | Unnecessary `(list\|tuple)` literal passed to `list()` (rewrite as a `list` literal) | |
| C409 | UnnecessaryLiteralWithinTupleCall | Unnecessary `(list\|tuple)` literal passed to `tuple()` (remove the outer call to `tuple()`) | 🛠 |
| C410 | UnnecessaryLiteralWithinListCall | Unnecessary `(list\|tuple)` literal passed to `list()` (rewrite as a `list` literal) | 🛠 |
| C411 | UnnecessaryListCall | Unnecessary `list` call (remove the outer call to `list()`) | 🛠 |
| C413 | UnnecessaryCallAroundSorted | Unnecessary `(list\|reversed)` call around `sorted()` | |
| C414 | UnnecessaryDoubleCastOrProcess | Unnecessary `(list\|reversed\|set\|sorted\|tuple)` call within `(list\|set\|sorted\|tuple)()` | |
@ -521,7 +521,7 @@ including:
- [`flake8-quotes`](https://pypi.org/project/flake8-quotes/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (10/32)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (9/34)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (10/34)
- [`autoflake`](https://pypi.org/project/autoflake/) (1/7)
Beyond rule-set parity, Ruff suffers from the following limitations vis-à-vis Flake8:
@ -545,7 +545,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (10/32)
Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa),
and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (9/34).
and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (10/34).
If you're looking to use Ruff, but rely on an unsupported Flake8 plugin, free to file an Issue.

View File

@ -1,2 +1,5 @@
dict((x, x) for x in range(3))
dict(
(x, x) for x in range(3)
)
dict(((x, x) for x in range(3)), z=3)

View File

@ -1,3 +1,10 @@
t1 = tuple([1, 2])
t2 = tuple((1, 2))
t3 = tuple([])
t1 = tuple([])
t2 = tuple([1, 2])
t3 = tuple((1, 2))
t4 = tuple([
1,
2
])
t5 = tuple(
(1, 2)
)

View File

@ -1015,7 +1015,12 @@ where
if self.settings.enabled.contains(&CheckCode::C402) {
if let Some(check) = flake8_comprehensions::checks::unnecessary_generator_dict(
expr, func, args, keywords,
expr,
func,
args,
keywords,
self.locator,
self.patch(),
) {
self.checks.push(check);
};
@ -1083,7 +1088,11 @@ where
if self.settings.enabled.contains(&CheckCode::C409) {
if let Some(check) =
flake8_comprehensions::checks::unnecessary_literal_within_tuple_call(
expr, func, args,
expr,
func,
args,
self.locator,
self.patch(),
)
{
self.checks.push(check);
@ -1093,7 +1102,11 @@ where
if self.settings.enabled.contains(&CheckCode::C410) {
if let Some(check) =
flake8_comprehensions::checks::unnecessary_literal_within_list_call(
expr, func, args,
expr,
func,
args,
self.locator,
self.patch(),
)
{
self.checks.push(check);

View File

@ -1470,11 +1470,14 @@ impl CheckKind {
| CheckKind::TypeOfPrimitive(_)
| CheckKind::UnnecessaryAbspath
| CheckKind::UnnecessaryCollectionCall(_)
| CheckKind::UnnecessaryGeneratorDict
| CheckKind::UnnecessaryGeneratorList
| CheckKind::UnnecessaryGeneratorSet
| CheckKind::UnnecessaryListCall
| CheckKind::UnnecessaryListComprehensionSet
| CheckKind::UnnecessaryLiteralSet(_)
| CheckKind::UnnecessaryLiteralWithinListCall(_)
| CheckKind::UnnecessaryLiteralWithinTupleCall(_)
| CheckKind::UnusedImport(_, false)
| CheckKind::UnusedLoopControlVariable(_)
| CheckKind::UnusedNOQA(_)

View File

@ -104,15 +104,24 @@ pub fn unnecessary_generator_dict(
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
locator: &SourceCodeLocator,
fix: bool,
) -> Option<Check> {
let argument = exactly_one_argument_with_matching_function("dict", func, args, keywords)?;
if let ExprKind::GeneratorExp { elt, .. } = argument {
match &elt.node {
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
let mut check = Check::new(
CheckKind::UnnecessaryGeneratorDict,
Range::from_located(expr),
));
);
if fix {
match fixes::fix_unnecessary_generator_dict(locator, expr) {
Ok(fix) => check.amend(fix),
Err(e) => error!("Failed to generate fix: {}", e),
}
}
return Some(check);
}
_ => {}
}
@ -266,6 +275,8 @@ pub fn unnecessary_literal_within_tuple_call(
expr: &Expr,
func: &Expr,
args: &[Expr],
locator: &SourceCodeLocator,
fix: bool,
) -> Option<Check> {
let argument = first_argument_with_matching_function("tuple", func, args)?;
let argument_kind = match argument {
@ -273,10 +284,17 @@ pub fn unnecessary_literal_within_tuple_call(
ExprKind::List { .. } => "list",
_ => return None,
};
Some(Check::new(
let mut check = Check::new(
CheckKind::UnnecessaryLiteralWithinTupleCall(argument_kind.to_string()),
Range::from_located(expr),
))
);
if fix {
match fixes::fix_unnecessary_literal_within_tuple_call(locator, expr) {
Ok(fix) => check.amend(fix),
Err(e) => error!("Failed to generate fix: {}", e),
}
}
Some(check)
}
/// C410
@ -284,6 +302,8 @@ pub fn unnecessary_literal_within_list_call(
expr: &Expr,
func: &Expr,
args: &[Expr],
locator: &SourceCodeLocator,
fix: bool,
) -> Option<Check> {
let argument = first_argument_with_matching_function("list", func, args)?;
let argument_kind = match argument {
@ -291,10 +311,17 @@ pub fn unnecessary_literal_within_list_call(
ExprKind::List { .. } => "list",
_ => return None,
};
Some(Check::new(
let mut check = Check::new(
CheckKind::UnnecessaryLiteralWithinListCall(argument_kind.to_string()),
Range::from_located(expr),
))
);
if fix {
match fixes::fix_unnecessary_literal_within_list_call(locator, expr) {
Ok(fix) => check.amend(fix),
Err(e) => error!("Failed to generate fix: {}", e),
}
}
Some(check)
}
/// C411

View File

@ -1,7 +1,8 @@
use anyhow::Result;
use libcst_native::{
Arg, Codegen, Dict, Expression, LeftCurlyBrace, LeftSquareBracket, List, ListComp,
RightCurlyBrace, RightSquareBracket, Set, SetComp, SmallStatement, Statement, Tuple,
Arg, Codegen, Dict, DictComp, Element, Expression, LeftCurlyBrace, LeftParen,
LeftSquareBracket, List, ListComp, ParenthesizableWhitespace, RightCurlyBrace, RightParen,
RightSquareBracket, Set, SetComp, SimpleWhitespace, SmallStatement, Statement, Tuple,
};
use rustpython_ast::Expr;
@ -32,7 +33,7 @@ pub fn fix_unnecessary_generator_list(locator: &SourceCodeLocator, expr: &Expr)
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &mut body.value {
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
@ -41,7 +42,7 @@ pub fn fix_unnecessary_generator_list(locator: &SourceCodeLocator, expr: &Expr)
value,
whitespace_after_arg,
..
}) = call.args.first_mut()
}) = call.args.first()
{
(value, whitespace_after_arg)
} else {
@ -101,7 +102,7 @@ pub fn fix_unnecessary_generator_set(locator: &SourceCodeLocator, expr: &Expr) -
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &mut body.value {
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
@ -110,7 +111,7 @@ pub fn fix_unnecessary_generator_set(locator: &SourceCodeLocator, expr: &Expr) -
value,
whitespace_after_arg,
..
}) = call.args.first_mut()
}) = call.args.first()
{
(value, whitespace_after_arg)
} else {
@ -147,6 +148,96 @@ pub fn fix_unnecessary_generator_set(locator: &SourceCodeLocator, expr: &Expr) -
))
}
/// (C402) Convert `dict((x, x) for x in range(3))` to `{x: x for x in
/// range(3)}`.
pub fn fix_unnecessary_generator_dict(locator: &SourceCodeLocator, expr: &Expr) -> Result<Fix> {
let mut tree = match libcst_native::parse_module(
locator.slice_source_code_range(&Range::from_located(expr)),
None,
) {
Ok(m) => m,
Err(_) => return Err(anyhow::anyhow!("Failed to extract CST from source.")),
};
let body = if let Some(Statement::Simple(body)) = tree.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple."));
};
let body = if let Some(SmallStatement::Expr(body)) = body.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!(
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
};
let (arg, whitespace_after_arg) = if let Some(Arg {
value,
whitespace_after_arg,
..
}) = call.args.first()
{
(value, whitespace_after_arg)
} else {
return Err(anyhow::anyhow!("Expected node to be: Arg."));
};
let generator_exp = if let Expression::GeneratorExp(generator_exp) = &arg {
generator_exp
} else {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::GeneratorExp."
));
};
let tuple = if let Expression::Tuple(tuple) = &generator_exp.elt.as_ref() {
tuple
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Tuple."));
};
let key = if let Some(Element::Simple { value, .. }) = &tuple.elements.get(0) {
value
} else {
return Err(anyhow::anyhow!(
"Expected tuple to contain a key as the first element."
));
};
let value = if let Some(Element::Simple { value, .. }) = &tuple.elements.get(1) {
value
} else {
return Err(anyhow::anyhow!(
"Expected tuple to contain a key as the second element."
));
};
body.value = Expression::DictComp(Box::new(DictComp {
key: Box::new(key.clone()),
value: Box::new(value.clone()),
for_in: generator_exp.for_in.clone(),
lbrace: LeftCurlyBrace {
whitespace_after: call.whitespace_before_args.clone(),
},
rbrace: RightCurlyBrace {
whitespace_before: whitespace_after_arg.clone(),
},
lpar: Default::default(),
rpar: Default::default(),
whitespace_before_colon: Default::default(),
whitespace_after_colon: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
}));
let mut state = Default::default();
tree.codegen(&mut state);
Ok(Fix::replacement(
state.to_string(),
expr.location,
expr.end_location.unwrap(),
))
}
/// (C403) Convert `set([x for x in y])` to `{x for x in y}`.
pub fn fix_unnecessary_list_comprehension_set(
locator: &SourceCodeLocator,
@ -173,7 +264,7 @@ pub fn fix_unnecessary_list_comprehension_set(
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &mut body.value {
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
@ -182,7 +273,7 @@ pub fn fix_unnecessary_list_comprehension_set(
value,
whitespace_after_arg,
..
}) = call.args.first_mut()
}) = call.args.first()
{
(value, whitespace_after_arg)
} else {
@ -370,6 +461,168 @@ pub fn fix_unnecessary_collection_call(locator: &SourceCodeLocator, expr: &Expr)
))
}
/// (C409) Convert `tuple([1, 2])` to `tuple(1, 2)`
pub fn fix_unnecessary_literal_within_tuple_call(
locator: &SourceCodeLocator,
expr: &Expr,
) -> Result<Fix> {
let mut tree = match libcst_native::parse_module(
locator.slice_source_code_range(&Range::from_located(expr)),
None,
) {
Ok(m) => m,
Err(_) => return Err(anyhow::anyhow!("Failed to extract CST from source.")),
};
let body = if let Some(Statement::Simple(body)) = tree.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple."));
};
let body = if let Some(SmallStatement::Expr(body)) = body.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!(
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
};
let arg = if let Some(Arg { value, .. }) = call.args.first() {
value
} else {
return Err(anyhow::anyhow!("Expected node to be: Arg."));
};
let (elements, whitespace_after, whitespace_before) = match arg {
Expression::Tuple(inner) => (
&inner.elements,
&inner
.lpar
.first()
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses."))?
.whitespace_after,
&inner
.rpar
.first()
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses."))?
.whitespace_before,
),
Expression::List(inner) => (
&inner.elements,
&inner.lbracket.whitespace_after,
&inner.rbracket.whitespace_before,
),
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::Tuple | Expression::List."
))
}
};
body.value = Expression::Tuple(Box::new(Tuple {
elements: elements.clone(),
lpar: vec![LeftParen {
whitespace_after: whitespace_after.clone(),
}],
rpar: vec![RightParen {
whitespace_before: whitespace_before.clone(),
}],
}));
let mut state = Default::default();
tree.codegen(&mut state);
Ok(Fix::replacement(
state.to_string(),
expr.location,
expr.end_location.unwrap(),
))
}
/// (C410) Convert `list([1, 2])` to `[1, 2]`
pub fn fix_unnecessary_literal_within_list_call(
locator: &SourceCodeLocator,
expr: &Expr,
) -> Result<Fix> {
let mut tree = match libcst_native::parse_module(
locator.slice_source_code_range(&Range::from_located(expr)),
None,
) {
Ok(m) => m,
Err(_) => return Err(anyhow::anyhow!("Failed to extract CST from source.")),
};
let body = if let Some(Statement::Simple(body)) = tree.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple."));
};
let body = if let Some(SmallStatement::Expr(body)) = body.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!(
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
};
let arg = if let Some(Arg { value, .. }) = call.args.first() {
value
} else {
return Err(anyhow::anyhow!("Expected node to be: Arg."));
};
let (elements, whitespace_after, whitespace_before) = match arg {
Expression::Tuple(inner) => (
&inner.elements,
&inner
.lpar
.first()
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses."))?
.whitespace_after,
&inner
.rpar
.first()
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses."))?
.whitespace_before,
),
Expression::List(inner) => (
&inner.elements,
&inner.lbracket.whitespace_after,
&inner.rbracket.whitespace_before,
),
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::Tuple | Expression::List."
))
}
};
body.value = Expression::List(Box::new(List {
elements: elements.clone(),
lbracket: LeftSquareBracket {
whitespace_after: whitespace_after.clone(),
},
rbracket: RightSquareBracket {
whitespace_before: whitespace_before.clone(),
},
lpar: Default::default(),
rpar: Default::default(),
}));
let mut state = Default::default();
tree.codegen(&mut state);
Ok(Fix::replacement(
state.to_string(),
expr.location,
expr.end_location.unwrap(),
))
}
/// (C411) Convert `list([i for i in x])` to `[i for i in x]`.
pub fn fix_unnecessary_list_call(locator: &SourceCodeLocator, expr: &Expr) -> Result<Fix> {
// Module(SimpleStatementLine(Expr(Call(List|Tuple)))) ->
@ -393,12 +646,12 @@ pub fn fix_unnecessary_list_call(locator: &SourceCodeLocator, expr: &Expr) -> Re
"Expected node to be: SmallStatement::Expr."
));
};
let call = if let Expression::Call(call) = &mut body.value {
let call = if let Expression::Call(call) = &body.value {
call
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Call."));
};
let arg = if let Some(Arg { value, .. }) = call.args.first_mut() {
let arg = if let Some(Arg { value, .. }) = call.args.first() {
value
} else {
return Err(anyhow::anyhow!("Expected node to be: Arg."));

View File

@ -9,5 +9,31 @@ expression: checks
end_location:
row: 1
column: 30
fix: ~
fix:
patch:
content: "{x: x for x in range(3)}"
location:
row: 1
column: 0
end_location:
row: 1
column: 30
applied: false
- kind: UnnecessaryGeneratorDict
location:
row: 2
column: 0
end_location:
row: 4
column: 1
fix:
patch:
content: "{\n x: x for x in range(3)\n}"
location:
row: 2
column: 0
end_location:
row: 4
column: 1
applied: false

View File

@ -9,24 +9,87 @@ expression: checks
column: 5
end_location:
row: 1
column: 18
fix: ~
- kind:
UnnecessaryLiteralWithinTupleCall: tuple
column: 14
fix:
patch:
content: ()
location:
row: 2
row: 1
column: 5
end_location:
row: 2
column: 18
fix: ~
row: 1
column: 14
applied: false
- kind:
UnnecessaryLiteralWithinTupleCall: list
location:
row: 2
column: 5
end_location:
row: 2
column: 18
fix:
patch:
content: "(1, 2)"
location:
row: 2
column: 5
end_location:
row: 2
column: 18
applied: false
- kind:
UnnecessaryLiteralWithinTupleCall: tuple
location:
row: 3
column: 5
end_location:
row: 3
column: 14
fix: ~
column: 18
fix:
patch:
content: "(1, 2)"
location:
row: 3
column: 5
end_location:
row: 3
column: 18
applied: false
- kind:
UnnecessaryLiteralWithinTupleCall: list
location:
row: 4
column: 5
end_location:
row: 7
column: 2
fix:
patch:
content: "(\n 1,\n 2\n)"
location:
row: 4
column: 5
end_location:
row: 7
column: 2
applied: false
- kind:
UnnecessaryLiteralWithinTupleCall: tuple
location:
row: 8
column: 5
end_location:
row: 10
column: 1
fix:
patch:
content: "(1, 2)"
location:
row: 8
column: 5
end_location:
row: 10
column: 1
applied: false

View File

@ -10,7 +10,16 @@ expression: checks
end_location:
row: 1
column: 17
fix: ~
fix:
patch:
content: "[1, 2]"
location:
row: 1
column: 5
end_location:
row: 1
column: 17
applied: false
- kind:
UnnecessaryLiteralWithinListCall: tuple
location:
@ -19,7 +28,16 @@ expression: checks
end_location:
row: 2
column: 17
fix: ~
fix:
patch:
content: "[1, 2]"
location:
row: 2
column: 5
end_location:
row: 2
column: 17
applied: false
- kind:
UnnecessaryLiteralWithinListCall: list
location:
@ -28,7 +46,16 @@ expression: checks
end_location:
row: 3
column: 13
fix: ~
fix:
patch:
content: "[]"
location:
row: 3
column: 5
end_location:
row: 3
column: 13
applied: false
- kind:
UnnecessaryLiteralWithinListCall: tuple
location:
@ -37,5 +64,14 @@ expression: checks
end_location:
row: 4
column: 13
fix: ~
fix:
patch:
content: "[]"
location:
row: 4
column: 5
end_location:
row: 4
column: 13
applied: false