diff --git a/README.md b/README.md index a013ee35cd..0c17194cde 100644 --- a/README.md +++ b/README.md @@ -445,7 +445,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | 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) | | +| 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) | 🛠 | diff --git a/src/check_ast.rs b/src/check_ast.rs index aa48ad756c..95a3427644 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -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); diff --git a/src/checks.rs b/src/checks.rs index 5afd0bd9bb..0fa3024e89 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1476,6 +1476,7 @@ impl CheckKind { | CheckKind::UnnecessaryGeneratorSet | CheckKind::UnnecessaryListCall | CheckKind::UnnecessaryListComprehensionSet + | CheckKind::UnnecessaryLiteralDict(_) | CheckKind::UnnecessaryLiteralSet(_) | CheckKind::UnnecessaryLiteralWithinListCall(_) | CheckKind::UnnecessaryLiteralWithinTupleCall(_) diff --git a/src/flake8_comprehensions/checks.rs b/src/flake8_comprehensions/checks.rs index 54b3891f57..1647541c0c 100644 --- a/src/flake8_comprehensions/checks.rs +++ b/src/flake8_comprehensions/checks.rs @@ -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 { 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 diff --git a/src/flake8_comprehensions/fixes.rs b/src/flake8_comprehensions/fixes.rs index 34041ec56c..e15bd5e654 100644 --- a/src/flake8_comprehensions/fixes.rs +++ b/src/flake8_comprehensions/fixes.rs @@ -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 { + // 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 = 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::>>()?; + + 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" )) } } diff --git a/src/pyflakes/fixes.rs b/src/pyflakes/fixes.rs index c30444d10c..d5a4fb79a3 100644 --- a/src/pyflakes/fixes.rs +++ b/src/pyflakes/fixes.rs @@ -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. diff --git a/src/snapshots/ruff__linter__tests__C406_C406.py.snap b/src/snapshots/ruff__linter__tests__C406_C406.py.snap index b2804ba5c5..405d385735 100644 --- a/src/snapshots/ruff__linter__tests__C406_C406.py.snap +++ b/src/snapshots/ruff__linter__tests__C406_C406.py.snap @@ -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