From 9948be0145c32937038780c1892fcac79aedb4ff Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 2 Nov 2022 20:39:35 -0400 Subject: [PATCH] Automatically fix a variety of comprehension rules (#553) --- README.md | 16 +- src/check_ast.rs | 45 +- src/checks.rs | 6 + src/flake8_comprehensions/checks.rs | 97 ++++- src/flake8_comprehensions/fixes.rs | 397 ++++++++++++++++++ src/flake8_comprehensions/mod.rs | 1 + .../ruff__linter__tests__C400_C400.py.snap | 11 +- .../ruff__linter__tests__C401_C401.py.snap | 11 +- .../ruff__linter__tests__C403_C403.py.snap | 11 +- .../ruff__linter__tests__C405_C405.py.snap | 44 +- .../ruff__linter__tests__C408_C408.py.snap | 33 +- .../ruff__linter__tests__C411_C411.py.snap | 11 +- 12 files changed, 639 insertions(+), 44 deletions(-) create mode 100644 src/flake8_comprehensions/fixes.rs diff --git a/README.md b/README.md index 210f042f43..bc85f316bc 100644 --- a/README.md +++ b/README.md @@ -392,17 +392,17 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | -| C400 | UnnecessaryGeneratorList | Unnecessary generator (rewrite as a `list` comprehension) | | -| C401 | UnnecessaryGeneratorSet | Unnecessary generator (rewrite as a `set` comprehension) | | +| 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) | | -| C403 | UnnecessaryListComprehensionSet | Unnecessary `list` comprehension (rewrite as a `set` 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) | | +| 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) | | +| 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) | | -| C411 | UnnecessaryListCall | Unnecessary `list` call (remove the outer call to `list()`) | | +| 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)()` | | | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within `(reversed\|set\|sorted)()` | | @@ -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/) (8/34) +- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (9/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/) (8/34). +and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (9/34). If you're looking to use Ruff, but rely on an unsupported Flake8 plugin, free to file an Issue. diff --git a/src/check_ast.rs b/src/check_ast.rs index e043d44727..1731f3894b 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -989,7 +989,12 @@ where // flake8-comprehensions if self.settings.enabled.contains(&CheckCode::C400) { if let Some(check) = flake8_comprehensions::checks::unnecessary_generator_list( - expr, func, args, keywords, + expr, + func, + args, + keywords, + self.locator, + self.patch(), ) { self.checks.push(check); }; @@ -997,7 +1002,12 @@ where if self.settings.enabled.contains(&CheckCode::C401) { if let Some(check) = flake8_comprehensions::checks::unnecessary_generator_set( - expr, func, args, keywords, + expr, + func, + args, + keywords, + self.locator, + self.patch(), ) { self.checks.push(check); }; @@ -1014,7 +1024,12 @@ where if self.settings.enabled.contains(&CheckCode::C403) { if let Some(check) = flake8_comprehensions::checks::unnecessary_list_comprehension_set( - expr, func, args, keywords, + expr, + func, + args, + keywords, + self.locator, + self.patch(), ) { self.checks.push(check); @@ -1033,7 +1048,12 @@ where if self.settings.enabled.contains(&CheckCode::C405) { if let Some(check) = flake8_comprehensions::checks::unnecessary_literal_set( - expr, func, args, keywords, + expr, + func, + args, + keywords, + self.locator, + self.patch(), ) { self.checks.push(check); }; @@ -1049,7 +1069,12 @@ where if self.settings.enabled.contains(&CheckCode::C408) { if let Some(check) = flake8_comprehensions::checks::unnecessary_collection_call( - expr, func, args, keywords, + expr, + func, + args, + keywords, + self.locator, + self.patch(), ) { self.checks.push(check); }; @@ -1076,9 +1101,13 @@ where } if self.settings.enabled.contains(&CheckCode::C411) { - if let Some(check) = - flake8_comprehensions::checks::unnecessary_list_call(expr, func, args) - { + if let Some(check) = flake8_comprehensions::checks::unnecessary_list_call( + expr, + func, + args, + self.locator, + self.patch(), + ) { self.checks.push(check); }; } diff --git a/src/checks.rs b/src/checks.rs index a789682165..d4d461b6e2 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1469,6 +1469,12 @@ impl CheckKind { | CheckKind::SuperCallWithParameters | CheckKind::TypeOfPrimitive(_) | CheckKind::UnnecessaryAbspath + | CheckKind::UnnecessaryCollectionCall(_) + | CheckKind::UnnecessaryGeneratorList + | CheckKind::UnnecessaryGeneratorSet + | CheckKind::UnnecessaryListCall + | CheckKind::UnnecessaryListComprehensionSet + | CheckKind::UnnecessaryLiteralSet(_) | CheckKind::UnusedImport(_, false) | CheckKind::UnusedLoopControlVariable(_) | CheckKind::UnusedNOQA(_) diff --git a/src/flake8_comprehensions/checks.rs b/src/flake8_comprehensions/checks.rs index 1c07d82744..56e77d25f8 100644 --- a/src/flake8_comprehensions/checks.rs +++ b/src/flake8_comprehensions/checks.rs @@ -1,3 +1,4 @@ +use log::error; use num_bigint::BigInt; use rustpython_ast::{ Comprehension, Constant, Expr, ExprKind, Keyword, KeywordData, Located, Unaryop, @@ -5,6 +6,8 @@ use rustpython_ast::{ use crate::ast::types::Range; use crate::checks::{Check, CheckKind}; +use crate::flake8_comprehensions::fixes; +use crate::source_code_locator::SourceCodeLocator; fn function_name(func: &Expr) -> Option<&str> { if let ExprKind::Name { id, .. } = &func.node { @@ -49,13 +52,22 @@ pub fn unnecessary_generator_list( func: &Expr, args: &[Expr], keywords: &[Keyword], + locator: &SourceCodeLocator, + fix: bool, ) -> Option { let argument = exactly_one_argument_with_matching_function("list", func, args, keywords)?; if let ExprKind::GeneratorExp { .. } = argument { - return Some(Check::new( + let mut check = Check::new( CheckKind::UnnecessaryGeneratorList, Range::from_located(expr), - )); + ); + if fix { + match fixes::fix_unnecessary_generator_list(locator, expr) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to generate fix: {}", e), + } + } + return Some(check); } None } @@ -66,13 +78,22 @@ pub fn unnecessary_generator_set( func: &Expr, args: &[Expr], keywords: &[Keyword], + locator: &SourceCodeLocator, + fix: bool, ) -> Option { let argument = exactly_one_argument_with_matching_function("set", func, args, keywords)?; if let ExprKind::GeneratorExp { .. } = argument { - return Some(Check::new( + let mut check = Check::new( CheckKind::UnnecessaryGeneratorSet, Range::from_located(expr), - )); + ); + if fix { + match fixes::fix_unnecessary_generator_set(locator, expr) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to generate fix: {}", e), + } + } + return Some(check); } None } @@ -105,13 +126,22 @@ pub fn unnecessary_list_comprehension_set( func: &Expr, args: &[Expr], keywords: &[Keyword], + locator: &SourceCodeLocator, + fix: bool, ) -> Option { let argument = exactly_one_argument_with_matching_function("set", func, args, keywords)?; if let ExprKind::ListComp { .. } = &argument { - return Some(Check::new( + let mut check = Check::new( CheckKind::UnnecessaryListComprehensionSet, Range::from_located(expr), - )); + ); + if fix { + match fixes::fix_unnecessary_list_comprehension_set(locator, expr) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to generate fix: {}", e), + } + } + return Some(check); } None } @@ -144,6 +174,8 @@ pub fn unnecessary_literal_set( func: &Expr, args: &[Expr], keywords: &[Keyword], + locator: &SourceCodeLocator, + fix: bool, ) -> Option { let argument = exactly_one_argument_with_matching_function("set", func, args, keywords)?; let kind = match argument { @@ -151,10 +183,17 @@ pub fn unnecessary_literal_set( ExprKind::Tuple { .. } => "tuple", _ => return None, }; - Some(Check::new( + let mut check = Check::new( CheckKind::UnnecessaryLiteralSet(kind.to_string()), Range::from_located(expr), - )) + ); + if fix { + match fixes::fix_unnecessary_literal_set(locator, expr) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to generate fix: {}", e), + } + } + Some(check) } /// C406 (`dict([(1, 2)])`) @@ -190,22 +229,36 @@ pub fn unnecessary_collection_call( func: &Expr, args: &[Expr], keywords: &[Located], + locator: &SourceCodeLocator, + fix: bool, ) -> Option { if !args.is_empty() { return None; } let id = function_name(func)?; match id { - "dict" if keywords.is_empty() || keywords.iter().all(|kw| kw.node.arg.is_some()) => (), + "dict" if keywords.is_empty() || keywords.iter().all(|kw| kw.node.arg.is_some()) => { + // `dict()` or `dict(a=1)` (as opposed to `dict(**a)`) + } "list" | "tuple" => { - // list() or tuple() + // `list()` or `tuple()` } _ => return None, }; - Some(Check::new( + let mut check = Check::new( CheckKind::UnnecessaryCollectionCall(id.to_string()), Range::from_located(expr), - )) + ); + if fix { + // TODO(charlie): Support fixing `dict(a=1)`. + if keywords.is_empty() { + match fixes::fix_unnecessary_collection_call(locator, expr) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to generate fix: {}", e), + } + } + } + Some(check) } /// C409 @@ -245,13 +298,23 @@ pub fn unnecessary_literal_within_list_call( } /// C411 -pub fn unnecessary_list_call(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { +pub fn unnecessary_list_call( + expr: &Expr, + func: &Expr, + args: &[Expr], + locator: &SourceCodeLocator, + fix: bool, +) -> Option { let argument = first_argument_with_matching_function("list", func, args)?; if let ExprKind::ListComp { .. } = argument { - return Some(Check::new( - CheckKind::UnnecessaryListCall, - Range::from_located(expr), - )); + let mut check = Check::new(CheckKind::UnnecessaryListCall, Range::from_located(expr)); + if fix { + match fixes::fix_unnecessary_list_call(locator, expr) { + Ok(fix) => check.amend(fix), + Err(e) => error!("Failed to generate fix: {}", e), + } + } + return Some(check); } None } diff --git a/src/flake8_comprehensions/fixes.rs b/src/flake8_comprehensions/fixes.rs new file mode 100644 index 0000000000..9b4e21232c --- /dev/null +++ b/src/flake8_comprehensions/fixes.rs @@ -0,0 +1,397 @@ +use anyhow::Result; +use libcst_native::{ + Arg, Codegen, Dict, Expression, LeftCurlyBrace, LeftSquareBracket, List, ListComp, + RightCurlyBrace, RightSquareBracket, Set, SetComp, SmallStatement, Statement, Tuple, +}; +use rustpython_ast::Expr; + +use crate::ast::types::Range; +use crate::autofix::Fix; +use crate::source_code_locator::SourceCodeLocator; + +/// (C400) Convert `list(x for x in y)` to `[x for x in y]`. +pub fn fix_unnecessary_generator_list(locator: &SourceCodeLocator, expr: &Expr) -> Result { + // Module(SimpleStatementLine(Expr(Call(GeneratorExp)))) -> + // Module(SimpleStatementLine(Expr(ListComp))) + 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) = &mut 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() { + value + } 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." + )); + }; + + body.value = Expression::ListComp(Box::new(ListComp { + elt: generator_exp.elt.clone(), + for_in: generator_exp.for_in.clone(), + lbracket: LeftSquareBracket { + whitespace_after: call.whitespace_before_args.clone(), + }, + rbracket: RightSquareBracket { + whitespace_before: call.whitespace_after_func.clone(), + }, + lpar: generator_exp.lpar.clone(), + rpar: generator_exp.rpar.clone(), + })); + + let mut state = Default::default(); + tree.codegen(&mut state); + + Ok(Fix::replacement( + state.to_string(), + expr.location, + expr.end_location.unwrap(), + )) +} + +/// (C401) Convert `set(x for x in y)` to `{x for x in y}`. +pub fn fix_unnecessary_generator_set(locator: &SourceCodeLocator, expr: &Expr) -> Result { + // Module(SimpleStatementLine(Expr(Call(GeneratorExp)))) -> + // Module(SimpleStatementLine(Expr(SetComp))) + 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) = &mut 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() { + value + } 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." + )); + }; + + body.value = Expression::SetComp(Box::new(SetComp { + elt: generator_exp.elt.clone(), + for_in: generator_exp.for_in.clone(), + lbrace: LeftCurlyBrace { + whitespace_after: call.whitespace_before_args.clone(), + }, + rbrace: RightCurlyBrace { + whitespace_before: call.whitespace_after_func.clone(), + }, + lpar: generator_exp.lpar.clone(), + rpar: generator_exp.rpar.clone(), + })); + + 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, + expr: &Expr, +) -> Result { + // Module(SimpleStatementLine(Expr(Call(ListComp)))) -> + // Module(SimpleStatementLine(Expr(SetComp))) + 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) = &mut 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() { + value + } else { + return Err(anyhow::anyhow!("Expected node to be: Arg.")); + }; + let list_comp = if let Expression::ListComp(list_comp) = arg { + list_comp + } else { + return Err(anyhow::anyhow!( + "Expected node to be: Expression::ListComp." + )); + }; + + body.value = Expression::SetComp(Box::new(SetComp { + elt: list_comp.elt.clone(), + for_in: list_comp.for_in.clone(), + lbrace: LeftCurlyBrace { + whitespace_after: call.whitespace_before_args.clone(), + }, + rbrace: RightCurlyBrace { + whitespace_before: call.whitespace_after_func.clone(), + }, + lpar: list_comp.lpar.clone(), + rpar: list_comp.rpar.clone(), + })); + + let mut state = Default::default(); + tree.codegen(&mut state); + + Ok(Fix::replacement( + state.to_string(), + expr.location, + expr.end_location.unwrap(), + )) +} + +/// (C405) Convert `set((1, 2))` to `{1, 2}`. +pub fn fix_unnecessary_literal_set(locator: &SourceCodeLocator, expr: &Expr) -> Result { + // Module(SimpleStatementLine(Expr(Call(List|Tuple)))) -> + // Module(SimpleStatementLine(Expr(Set))) + 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) = &mut 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() { + value + } else { + return Err(anyhow::anyhow!("Expected node to be: Arg.")); + }; + let elements = match arg { + Expression::Tuple(inner) => inner.elements.clone(), + Expression::List(inner) => inner.elements.clone(), + _ => { + return Err(anyhow::anyhow!( + "Expected node to be: Expression::Tuple | Expression::List." + )) + } + }; + + if elements.is_empty() { + call.args = vec![]; + } else { + body.value = Expression::Set(Box::new(Set { + elements, + lbrace: LeftCurlyBrace { + whitespace_after: call.whitespace_before_args.clone(), + }, + rbrace: RightCurlyBrace { + whitespace_before: call.whitespace_after_func.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, expr: &Expr) -> Result { + // Module(SimpleStatementLine(Expr(Call("list" | "tuple" | "dict")))) -> + // Module(SimpleStatementLine(Expr(List|Tuple|Dict))) + 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 name = if let Expression::Name(name) = &call.func.as_ref() { + name + } else { + return Err(anyhow::anyhow!("Expected node to be: Expression::Name.")); + }; + + match name.value { + "tuple" => { + body.value = Expression::Tuple(Box::new(Tuple { + elements: vec![], + lpar: vec![Default::default()], + rpar: vec![Default::default()], + })); + } + "list" => { + body.value = Expression::List(Box::new(List { + elements: vec![], + lbracket: Default::default(), + rbracket: Default::default(), + lpar: vec![], + rpar: vec![], + })); + } + "dict" => { + body.value = Expression::Dict(Box::new(Dict { + elements: vec![], + lbrace: Default::default(), + rbrace: Default::default(), + lpar: vec![], + rpar: vec![], + })); + } + _ => { + return Err(anyhow::anyhow!("Expected function name to be one of: \ + 'tuple', 'list', 'dict'." + .to_string())); + } + }; + + 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 { + // Module(SimpleStatementLine(Expr(Call(List|Tuple)))) -> + // Module(SimpleStatementLine(Expr(List|Tuple))) + 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) = &mut 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() { + value + } else { + return Err(anyhow::anyhow!("Expected node to be: Arg.")); + }; + + body.value = arg.clone(); + + let mut state = Default::default(); + tree.codegen(&mut state); + + Ok(Fix::replacement( + state.to_string(), + expr.location, + expr.end_location.unwrap(), + )) +} diff --git a/src/flake8_comprehensions/mod.rs b/src/flake8_comprehensions/mod.rs index f6b5329a02..0f6dbc1b70 100644 --- a/src/flake8_comprehensions/mod.rs +++ b/src/flake8_comprehensions/mod.rs @@ -1 +1,2 @@ pub mod checks; +mod fixes; diff --git a/src/snapshots/ruff__linter__tests__C400_C400.py.snap b/src/snapshots/ruff__linter__tests__C400_C400.py.snap index af55756877..c7cec50ee1 100644 --- a/src/snapshots/ruff__linter__tests__C400_C400.py.snap +++ b/src/snapshots/ruff__linter__tests__C400_C400.py.snap @@ -9,5 +9,14 @@ expression: checks end_location: row: 1 column: 29 - fix: ~ + fix: + patch: + content: "[x for x in range(3)]" + location: + row: 1 + column: 4 + end_location: + row: 1 + column: 29 + applied: false diff --git a/src/snapshots/ruff__linter__tests__C401_C401.py.snap b/src/snapshots/ruff__linter__tests__C401_C401.py.snap index 671b63e9d0..58d0cb0c51 100644 --- a/src/snapshots/ruff__linter__tests__C401_C401.py.snap +++ b/src/snapshots/ruff__linter__tests__C401_C401.py.snap @@ -9,5 +9,14 @@ expression: checks end_location: row: 1 column: 28 - fix: ~ + fix: + patch: + content: "{x for x in range(3)}" + location: + row: 1 + column: 4 + end_location: + row: 1 + column: 28 + applied: false diff --git a/src/snapshots/ruff__linter__tests__C403_C403.py.snap b/src/snapshots/ruff__linter__tests__C403_C403.py.snap index 2e55701a92..a71b386c44 100644 --- a/src/snapshots/ruff__linter__tests__C403_C403.py.snap +++ b/src/snapshots/ruff__linter__tests__C403_C403.py.snap @@ -9,5 +9,14 @@ expression: checks end_location: row: 1 column: 30 - fix: ~ + fix: + patch: + content: "{x for x in range(3)}" + location: + row: 1 + column: 4 + end_location: + row: 1 + column: 30 + applied: false diff --git a/src/snapshots/ruff__linter__tests__C405_C405.py.snap b/src/snapshots/ruff__linter__tests__C405_C405.py.snap index 01431d414d..28e1f6624b 100644 --- a/src/snapshots/ruff__linter__tests__C405_C405.py.snap +++ b/src/snapshots/ruff__linter__tests__C405_C405.py.snap @@ -10,7 +10,16 @@ expression: checks end_location: row: 1 column: 16 - fix: ~ + fix: + patch: + content: "{1, 2}" + location: + row: 1 + column: 5 + end_location: + row: 1 + column: 16 + applied: false - kind: UnnecessaryLiteralSet: tuple location: @@ -19,7 +28,16 @@ expression: checks end_location: row: 2 column: 16 - fix: ~ + fix: + patch: + content: "{1, 2}" + location: + row: 2 + column: 5 + end_location: + row: 2 + column: 16 + applied: false - kind: UnnecessaryLiteralSet: list location: @@ -28,7 +46,16 @@ expression: checks end_location: row: 3 column: 12 - fix: ~ + fix: + patch: + content: set() + location: + row: 3 + column: 5 + end_location: + row: 3 + column: 12 + applied: false - kind: UnnecessaryLiteralSet: tuple location: @@ -37,5 +64,14 @@ expression: checks end_location: row: 4 column: 12 - fix: ~ + fix: + patch: + content: set() + location: + row: 4 + column: 5 + end_location: + row: 4 + column: 12 + applied: false diff --git a/src/snapshots/ruff__linter__tests__C408_C408.py.snap b/src/snapshots/ruff__linter__tests__C408_C408.py.snap index 9f08e3fed9..47aa5f872e 100644 --- a/src/snapshots/ruff__linter__tests__C408_C408.py.snap +++ b/src/snapshots/ruff__linter__tests__C408_C408.py.snap @@ -10,7 +10,16 @@ expression: checks end_location: row: 1 column: 11 - fix: ~ + fix: + patch: + content: () + location: + row: 1 + column: 4 + end_location: + row: 1 + column: 11 + applied: false - kind: UnnecessaryCollectionCall: list location: @@ -19,7 +28,16 @@ expression: checks end_location: row: 2 column: 10 - fix: ~ + fix: + patch: + content: "[]" + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 10 + applied: false - kind: UnnecessaryCollectionCall: dict location: @@ -28,7 +46,16 @@ expression: checks end_location: row: 3 column: 11 - fix: ~ + fix: + patch: + content: "{}" + location: + row: 3 + column: 5 + end_location: + row: 3 + column: 11 + applied: false - kind: UnnecessaryCollectionCall: dict location: diff --git a/src/snapshots/ruff__linter__tests__C411_C411.py.snap b/src/snapshots/ruff__linter__tests__C411_C411.py.snap index 2962c7fc30..1f4e999005 100644 --- a/src/snapshots/ruff__linter__tests__C411_C411.py.snap +++ b/src/snapshots/ruff__linter__tests__C411_C411.py.snap @@ -9,5 +9,14 @@ expression: checks end_location: row: 2 column: 20 - fix: ~ + fix: + patch: + content: "[i for i in x]" + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 20 + applied: false