Enable autofix for C406 (#570)

This commit is contained in:
Charlie Marsh
2022-11-03 11:27:46 -04:00
committed by GitHub
parent 578ec4d843
commit f26f38d023
7 changed files with 161 additions and 45 deletions

View File

@@ -1076,9 +1076,12 @@ where
if self.settings.enabled.contains(&CheckCode::C406) {
if let Some(check) = flake8_comprehensions::checks::unnecessary_literal_dict(
expr,
func,
args,
keywords,
self.locator,
self.patch(),
self.locate_check(Range::from_located(expr)),
) {
self.checks.push(check);

View File

@@ -1476,6 +1476,7 @@ impl CheckKind {
| CheckKind::UnnecessaryGeneratorSet
| CheckKind::UnnecessaryListCall
| CheckKind::UnnecessaryListComprehensionSet
| CheckKind::UnnecessaryLiteralDict(_)
| CheckKind::UnnecessaryLiteralSet(_)
| CheckKind::UnnecessaryLiteralWithinListCall(_)
| CheckKind::UnnecessaryLiteralWithinTupleCall(_)

View File

@@ -197,9 +197,12 @@ pub fn unnecessary_literal_set(
/// C406 (`dict([(1, 2)])`)
pub fn unnecessary_literal_dict(
expr: &Expr,
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
locator: &SourceCodeLocator,
fix: bool,
location: Range,
) -> Option<Check> {
let argument = exactly_one_argument_with_matching_function("dict", func, args, keywords)?;
@@ -208,18 +211,24 @@ pub fn unnecessary_literal_dict(
ExprKind::List { elts, .. } => ("list", elts),
_ => return None,
};
if let Some(elt) = elts.first() {
// dict((1, 2), ...)) or dict([(1, 2), ...])
if !matches!(&elt.node, ExprKind::Tuple { elts, .. } if elts.len() == 2) {
return None;
}
// Accept `dict((1, 2), ...))` `dict([(1, 2), ...])`.
if !elts
.iter()
.all(|elt| matches!(&elt.node, ExprKind::Tuple { elts, .. } if elts.len() == 2))
{
return None;
}
Some(Check::new(
let mut check = Check::new(
CheckKind::UnnecessaryLiteralDict(kind.to_string()),
location,
))
);
if fix {
match fixes::fix_unnecessary_literal_dict(locator, expr) {
Ok(fix) => check.amend(fix),
Err(e) => error!("Failed to generate fix: {}", e),
}
}
Some(check)
}
/// C408

View File

@@ -15,12 +15,10 @@ fn match_expr<'a, 'b>(module: &'a mut Module<'b>) -> Result<&'a mut Expr<'b>> {
if let Some(SmallStatement::Expr(expr)) = expr.body.first_mut() {
Ok(expr)
} else {
Err(anyhow::anyhow!(
"Expected node to be: SmallStatement::Expr."
))
Err(anyhow::anyhow!("Expected node to be: SmallStatement::Expr"))
}
} else {
Err(anyhow::anyhow!("Expected node to be: Statement::Simple."))
Err(anyhow::anyhow!("Expected node to be: Statement::Simple"))
}
}
@@ -28,7 +26,7 @@ fn match_call<'a, 'b>(expr: &'a mut Expr<'b>) -> Result<&'a mut Call<'b>> {
if let Expression::Call(call) = &mut expr.value {
Ok(call)
} else {
Err(anyhow::anyhow!("Expected node to be: Expression::Call."))
Err(anyhow::anyhow!("Expected node to be: Expression::Call"))
}
}
@@ -36,7 +34,7 @@ fn match_arg<'a, 'b>(call: &'a Call<'b>) -> Result<&'a Arg<'b>> {
if let Some(arg) = call.args.first() {
Ok(arg)
} else {
Err(anyhow::anyhow!("Expected node to be: Arg."))
Err(anyhow::anyhow!("Expected node to be: Arg"))
}
}
@@ -55,7 +53,7 @@ pub fn fix_unnecessary_generator_list(
generator_exp
} else {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::GeneratorExp."
"Expected node to be: Expression::GeneratorExp"
));
};
@@ -97,7 +95,7 @@ pub fn fix_unnecessary_generator_set(
generator_exp
} else {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::GeneratorExp."
"Expected node to be: Expression::GeneratorExp"
));
};
@@ -140,26 +138,26 @@ pub fn fix_unnecessary_generator_dict(
generator_exp
} else {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::GeneratorExp."
"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."));
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."
"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."
"Expected tuple to contain a key as the second element"
));
};
@@ -204,9 +202,7 @@ pub fn fix_unnecessary_list_comprehension_set(
let list_comp = if let Expression::ListComp(list_comp) = &arg.value {
list_comp
} else {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::ListComp."
));
return Err(anyhow::anyhow!("Expected node to be: Expression::ListComp"));
};
body.value = Expression::SetComp(Box::new(SetComp {
@@ -248,7 +244,7 @@ pub fn fix_unnecessary_literal_set(
Expression::List(inner) => &inner.elements,
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::Tuple | Expression::List."
"Expected node to be: Expression::Tuple | Expression::List"
))
}
};
@@ -279,6 +275,77 @@ pub fn fix_unnecessary_literal_set(
))
}
/// (C406) Convert `dict([(1, 2)])` to `{1: 2}`.
pub fn fix_unnecessary_literal_dict(
locator: &SourceCodeLocator,
expr: &rustpython_ast::Expr,
) -> Result<Fix> {
// Expr(Call(List|Tuple)))) -> Expr(Dict)))
let mut tree = match_tree(locator, expr)?;
let mut body = match_expr(&mut tree)?;
let call = match_call(body)?;
let arg = match_arg(call)?;
let elements = match &arg.value {
Expression::Tuple(inner) => &inner.elements,
Expression::List(inner) => &inner.elements,
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::Tuple | Expression::List"
))
}
};
let elements: Vec<DictElement> = elements
.iter()
.map(|element| {
if let Element::Simple {
value: Expression::Tuple(tuple),
comma,
} = element
{
if let Some(Element::Simple { value: key, .. }) = tuple.elements.get(0) {
if let Some(Element::Simple { value, .. }) = tuple.elements.get(1) {
return Ok(DictElement::Simple {
key: key.clone(),
value: value.clone(),
comma: comma.clone(),
whitespace_before_colon: Default::default(),
whitespace_after_colon: ParenthesizableWhitespace::SimpleWhitespace(
SimpleWhitespace(" "),
),
});
}
}
}
Err(anyhow::anyhow!(
"Expected each argument to be a tuple of length two"
))
})
.collect::<Result<Vec<DictElement>>>()?;
body.value = Expression::Dict(Box::new(Dict {
elements,
lbrace: LeftCurlyBrace {
whitespace_after: call.whitespace_before_args.clone(),
},
rbrace: RightCurlyBrace {
whitespace_before: arg.whitespace_after_arg.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(),
))
}
/// (C408)
pub fn fix_unnecessary_collection_call(
locator: &SourceCodeLocator,
@@ -291,7 +358,7 @@ pub fn fix_unnecessary_collection_call(
let name = if let Expression::Name(name) = &call.func.as_ref() {
name
} else {
return Err(anyhow::anyhow!("Expected node to be: Expression::Name."));
return Err(anyhow::anyhow!("Expected node to be: Expression::Name"));
};
// Arena allocator used to create formatted strings of sufficient lifetime,
@@ -331,7 +398,7 @@ pub fn fix_unnecessary_collection_call(
"\"{}\"",
arg.keyword
.as_ref()
.expect("Expected dictionary argument to be kwarg.")
.expect("Expected dictionary argument to be kwarg")
.value
);
arena.push(quoted);
@@ -376,7 +443,7 @@ pub fn fix_unnecessary_collection_call(
}
_ => {
return Err(anyhow::anyhow!("Expected function name to be one of: \
'tuple', 'list', 'dict'."
'tuple', 'list', 'dict'"
.to_string()));
}
};
@@ -406,12 +473,12 @@ pub fn fix_unnecessary_literal_within_tuple_call(
&inner
.lpar
.first()
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses."))?
.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."))?
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
.whitespace_before,
),
Expression::List(inner) => (
@@ -421,7 +488,7 @@ pub fn fix_unnecessary_literal_within_tuple_call(
),
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::Tuple | Expression::List."
"Expected node to be: Expression::Tuple | Expression::List"
))
}
};
@@ -461,12 +528,12 @@ pub fn fix_unnecessary_literal_within_list_call(
&inner
.lpar
.first()
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses."))?
.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."))?
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
.whitespace_before,
),
Expression::List(inner) => (
@@ -476,7 +543,7 @@ pub fn fix_unnecessary_literal_within_list_call(
),
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::Tuple | Expression::List."
"Expected node to be: Expression::Tuple | Expression::List"
))
}
};
@@ -581,7 +648,7 @@ pub fn fix_unnecessary_comprehension(
}
_ => {
return Err(anyhow::anyhow!(
"Expected node to be: Expression::ListComp | Expression:SetComp."
"Expected node to be: Expression::ListComp | Expression:SetComp"
))
}
}

View File

@@ -20,13 +20,13 @@ pub fn remove_unused_imports(
let body = if let Some(Statement::Simple(body)) = tree.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple."));
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple"));
};
let body = if let Some(SmallStatement::Import(body)) = body.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!(
"Expected node to be: SmallStatement::ImportFrom."
"Expected node to be: SmallStatement::ImportFrom"
));
};
let aliases = &mut body.names;
@@ -77,20 +77,20 @@ pub fn remove_unused_import_froms(
let body = if let Some(Statement::Simple(body)) = tree.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple."));
return Err(anyhow::anyhow!("Expected node to be: Statement::Simple"));
};
let body = if let Some(SmallStatement::ImportFrom(body)) = body.body.first_mut() {
body
} else {
return Err(anyhow::anyhow!(
"Expected node to be: SmallStatement::ImportFrom."
"Expected node to be: SmallStatement::ImportFrom"
));
};
let aliases = if let ImportNames::Aliases(aliases) = &mut body.names {
aliases
} else {
return Err(anyhow::anyhow!("Expected node to be: Aliases."));
return Err(anyhow::anyhow!("Expected node to be: Aliases"));
};
// Preserve the trailing comma (or not) from the last entry.

View File

@@ -10,7 +10,16 @@ expression: checks
end_location:
row: 1
column: 19
fix: ~
fix:
patch:
content: "{1: 2}"
location:
row: 1
column: 5
end_location:
row: 1
column: 19
applied: false
- kind:
UnnecessaryLiteralDict: tuple
location:
@@ -19,7 +28,16 @@ expression: checks
end_location:
row: 2
column: 20
fix: ~
fix:
patch:
content: "{1: 2,}"
location:
row: 2
column: 5
end_location:
row: 2
column: 20
applied: false
- kind:
UnnecessaryLiteralDict: 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:
UnnecessaryLiteralDict: 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