From 0123425be15a6b09f22bd23489c648e534fd9197 Mon Sep 17 00:00:00 2001 From: Simon Brugman Date: Sun, 12 Feb 2023 06:20:42 +0100 Subject: [PATCH] [`flake8-comprehensions`] autofix C414 and C417 + bugfix (#2693) Closes https://github.com/charliermarsh/ruff/issues/2262 and closes https://github.com/charliermarsh/ruff/issues/2423 Fixes bug where some cases generated duplicated violations (see https://github.com/charliermarsh/ruff/pull/2732#issuecomment-1426397842) --- README.md | 4 +- .../fixtures/flake8_comprehensions/C414.py | 6 + .../fixtures/flake8_comprehensions/C417.py | 24 +++ crates/ruff/src/checkers/ast.rs | 8 +- .../src/rules/flake8_comprehensions/fixes.rs | 203 +++++++++++++++++- .../unnecessary_double_cast_or_process.rs | 88 +++++--- .../rules/unnecessary_map.rs | 126 +++++++++-- ...8_comprehensions__tests__C414_C414.py.snap | 155 +++++++++++-- ...8_comprehensions__tests__C417_C417.py.snap | 195 +++++++++++++---- .../unnecessary-double-cast-or-process.md | 40 ++++ docs/rules/unnecessary-map.md | 32 +++ 11 files changed, 780 insertions(+), 101 deletions(-) create mode 100644 docs/rules/unnecessary-double-cast-or-process.md create mode 100644 docs/rules/unnecessary-map.md diff --git a/README.md b/README.md index 9f598d63b9..faef76d6a4 100644 --- a/README.md +++ b/README.md @@ -1048,10 +1048,10 @@ For more, see [flake8-comprehensions](https://pypi.org/project/flake8-comprehens | C410 | unnecessary-literal-within-list-call | Unnecessary `{literal}` literal passed to `list()` (remove the outer call to `list()`) | 🛠 | | C411 | unnecessary-list-call | Unnecessary `list` call (remove the outer call to `list()`) | 🛠 | | C413 | unnecessary-call-around-sorted | Unnecessary `{func}` call around `sorted()` | 🛠 | -| C414 | unnecessary-double-cast-or-process | Unnecessary `{inner}` call within `{outer}()` | | +| C414 | [unnecessary-double-cast-or-process](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-double-cast-or-process.md) | Unnecessary `{inner}` call within `{outer}()` | 🛠 | | C415 | unnecessary-subscript-reversal | Unnecessary subscript reversal of iterable within `{func}()` | | | C416 | unnecessary-comprehension | Unnecessary `{obj_type}` comprehension (rewrite using `{obj_type}()`) | 🛠 | -| C417 | unnecessary-map | Unnecessary `map` usage (rewrite using a generator expression) | | +| C417 | [unnecessary-map](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-map.md) | Unnecessary `map` usage (rewrite using a generator expression) | 🛠 | ### flake8-datetimez (DTZ) diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py index bd6491cd46..fc0b502d5f 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py @@ -12,3 +12,9 @@ sorted(list(x)) sorted(tuple(x)) sorted(sorted(x)) sorted(reversed(x)) +tuple( + list( + [x, 3, "hell"\ + "o"] + ) +) diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py index 50816e4cb9..8abbdab6c8 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py @@ -1,6 +1,30 @@ +# Errors. nums = [1, 2, 3] map(lambda x: x + 1, nums) map(lambda x: str(x), nums) list(map(lambda x: x * 2, nums)) set(map(lambda x: x % 2 == 0, nums)) dict(map(lambda v: (v, v**2), nums)) +map(lambda: "const", nums) +map(lambda _: 3.0, nums) +_ = "".join(map(lambda x: x in nums and "1" or "0", range(123))) +all(map(lambda v: isinstance(v, dict), nums)) +filter(func, map(lambda v: v, nums)) + +# Error, but unfixable. +# For simple expressions, this could be: `(x if x else 1 for x in nums)`. +# For more complex expressions, this would differ: `(x + 2 if x else 3 for x in nums)`. +map(lambda x=1: x, nums) + +# False negatives. +map(lambda x=2, y=1: x + y, nums, nums) +set(map(lambda x, y: x, nums, nums)) + + +def myfunc(arg1: int, arg2: int = 4): + return 2 * arg1 + arg2 + + +list(map(myfunc, nums)) + +[x for x in nums] diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 31283f5d3c..5bead3b1cc 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -2528,7 +2528,13 @@ where ); } if self.settings.rules.enabled(&Rule::UnnecessaryMap) { - flake8_comprehensions::rules::unnecessary_map(self, expr, func, args); + flake8_comprehensions::rules::unnecessary_map( + self, + expr, + self.current_expr_parent().map(Into::into), + func, + args, + ); } // flake8-boolean-trap diff --git a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs index 64767eddb9..90b93e73aa 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs @@ -1,9 +1,10 @@ use anyhow::{bail, Result}; +use itertools::Itertools; use libcst_native::{ - Arg, AssignEqual, Call, Codegen, CodegenState, Dict, DictComp, DictElement, Element, Expr, - Expression, LeftCurlyBrace, LeftParen, LeftSquareBracket, List, ListComp, Name, - ParenthesizableWhitespace, RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, - SimpleString, SimpleWhitespace, Tuple, + Arg, AssignEqual, AssignTargetExpression, Call, Codegen, CodegenState, CompFor, Dict, DictComp, + DictElement, Element, Expr, Expression, GeneratorExp, LeftCurlyBrace, LeftParen, + LeftSquareBracket, List, ListComp, Name, ParenthesizableWhitespace, RightCurlyBrace, + RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace, Tuple, }; use crate::ast::types::Range; @@ -802,6 +803,45 @@ pub fn fix_unnecessary_call_around_sorted( )) } +/// (C414) Convert `sorted(list(foo))` to `sorted(foo)` +pub fn fix_unnecessary_double_cast_or_process( + locator: &Locator, + stylist: &Stylist, + expr: &rustpython_parser::ast::Expr, +) -> Result { + let module_text = locator.slice_source_code_range(&Range::from_located(expr)); + let mut tree = match_module(module_text)?; + let body = match_expr(&mut tree)?; + let mut outer_call = match_call(body)?; + let inner_call = match &outer_call.args[..] { + [arg] => { + if let Expression::Call(call) = &arg.value { + &call.args + } else { + bail!("Expected Expression::Call "); + } + } + _ => { + bail!("Expected one argument in outer function call"); + } + }; + + outer_call.args = inner_call.clone(); + + let mut state = CodegenState { + default_newline: stylist.line_ending(), + default_indent: stylist.indentation(), + ..CodegenState::default() + }; + tree.codegen(&mut state); + + Ok(Fix::replacement( + state.to_string(), + expr.location, + expr.end_location.unwrap(), + )) +} + /// (C416) Convert `[i for i in x]` to `list(x)`. pub fn fix_unnecessary_comprehension( locator: &Locator, @@ -875,3 +915,158 @@ pub fn fix_unnecessary_comprehension( expr.end_location.unwrap(), )) } + +/// (C417) Convert `map(lambda x: x * 2, bar)` to `(x * 2 for x in bar)`. +pub fn fix_unnecessary_map( + locator: &Locator, + stylist: &Stylist, + expr: &rustpython_parser::ast::Expr, + kind: &str, +) -> Result { + let module_text = locator.slice_source_code_range(&Range::from_located(expr)); + let mut tree = match_module(module_text)?; + let mut body = match_expr(&mut tree)?; + let call = match_call(body)?; + let arg = match_arg(call)?; + + let (args, lambda_func) = match &arg.value { + Expression::Call(outer_call) => { + let inner_lambda = outer_call.args.first().unwrap().value.clone(); + match &inner_lambda { + Expression::Lambda(..) => (outer_call.args.clone(), inner_lambda), + _ => { + bail!("Expected a lambda function") + } + } + } + Expression::Lambda(..) => (call.args.clone(), arg.value.clone()), + _ => { + bail!("Expected a lambda or call") + } + }; + + let Expression::Lambda(func_body) = &lambda_func else { + bail!("Expected a lambda") + }; + + if args.len() == 2 { + if func_body.params.params.iter().any(|f| f.default.is_some()) { + bail!("Currently not supporting default values"); + } + + let mut args_str = func_body + .params + .params + .iter() + .map(|f| f.name.value) + .join(", "); + if args_str.is_empty() { + args_str = "_".to_string(); + } + + let compfor = Box::new(CompFor { + target: AssignTargetExpression::Name(Box::new(Name { + value: args_str.as_str(), + lpar: vec![], + rpar: vec![], + })), + iter: args.last().unwrap().value.clone(), + ifs: vec![], + inner_for_in: None, + asynchronous: None, + whitespace_before: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")), + whitespace_after_for: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace( + " ", + )), + whitespace_before_in: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace( + " ", + )), + whitespace_after_in: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")), + }); + + match kind { + "generator" => { + body.value = Expression::GeneratorExp(Box::new(GeneratorExp { + elt: func_body.body.clone(), + for_in: compfor, + lpar: vec![LeftParen::default()], + rpar: vec![RightParen::default()], + })); + } + "list" => { + body.value = Expression::ListComp(Box::new(ListComp { + elt: func_body.body.clone(), + for_in: compfor, + lbracket: LeftSquareBracket::default(), + rbracket: RightSquareBracket::default(), + lpar: vec![], + rpar: vec![], + })); + } + "set" => { + body.value = Expression::SetComp(Box::new(SetComp { + elt: func_body.body.clone(), + for_in: compfor, + lpar: vec![], + rpar: vec![], + lbrace: LeftCurlyBrace::default(), + rbrace: RightCurlyBrace::default(), + })); + } + "dict" => { + let (key, value) = if let Expression::Tuple(tuple) = func_body.body.as_ref() { + if tuple.elements.len() != 2 { + bail!("Expected two elements") + } + + let Some(Element::Simple { value: key, .. }) = &tuple.elements.get(0) else { + bail!( + "Expected tuple to contain a key as the first element" + ); + }; + let Some(Element::Simple { value, .. }) = &tuple.elements.get(1) else { + bail!( + "Expected tuple to contain a key as the second element" + ); + }; + + (key, value) + } else { + bail!("Expected tuple for dict comprehension") + }; + + body.value = Expression::DictComp(Box::new(DictComp { + for_in: compfor, + lpar: vec![], + rpar: vec![], + key: Box::new(key.clone()), + value: Box::new(value.clone()), + lbrace: LeftCurlyBrace::default(), + rbrace: RightCurlyBrace::default(), + whitespace_before_colon: ParenthesizableWhitespace::default(), + whitespace_after_colon: ParenthesizableWhitespace::SimpleWhitespace( + SimpleWhitespace(" "), + ), + })); + } + _ => { + bail!("Expected generator, list, set or dict"); + } + } + + let mut state = CodegenState { + default_newline: stylist.line_ending(), + default_indent: stylist.indentation(), + ..CodegenState::default() + }; + tree.codegen(&mut state); + + Ok(Fix::replacement( + state.to_string(), + expr.location, + expr.end_location.unwrap(), + )) + } else { + bail!("Should have two arguments"); + } +} diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs index 4471ff9a51..c24a731b05 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs @@ -1,24 +1,66 @@ -use ruff_macros::{define_violation, derive_message_formats}; use rustpython_parser::ast::{Expr, ExprKind}; -use super::helpers; +use ruff_macros::{define_violation, derive_message_formats}; + use crate::ast::types::Range; use crate::checkers::ast::Checker; use crate::registry::Diagnostic; -use crate::violation::Violation; +use crate::rules::flake8_comprehensions::fixes; +use crate::violation::AlwaysAutofixableViolation; + +use super::helpers; define_violation!( + /// ## What it does + /// Checks for unnecessary `list`, `reversed`, `set`, `sorted`, and `tuple` + /// call within `list`, `set`, `sorted`, and `tuple` calls. + /// + /// ## Why is this bad? + /// It's unnecessary to double-cast or double-process iterables by wrapping + /// the listed functions within an additional `list`, `set`, `sorted`, or + /// `tuple` call. Doing so is redundant and can be confusing for readers. + /// + /// ## Examples + /// ```python + /// list(tuple(iterable)) + /// ``` + /// + /// Use instead: + /// ```python + /// list(iterable) + /// ``` + /// + /// This rule applies to a variety of functions, including `list`, `reversed`, + /// `set`, `sorted`, and `tuple`. For example: + /// * Instead of `list(list(iterable))`, use `list(iterable)`. + /// * Instead of `list(tuple(iterable))`, use `list(iterable)`. + /// * Instead of `tuple(list(iterable))`, use `tuple(iterable)`. + /// * Instead of `tuple(tuple(iterable))`, use `tuple(iterable)`. + /// * Instead of `set(set(iterable))`, use `set(iterable)`. + /// * Instead of `set(list(iterable))`, use `set(iterable)`. + /// * Instead of `set(tuple(iterable))`, use `set(iterable)`. + /// * Instead of `set(sorted(iterable))`, use `set(iterable)`. + /// * Instead of `set(reversed(iterable))`, use `set(iterable)`. + /// * Instead of `sorted(list(iterable))`, use `sorted(iterable)`. + /// * Instead of `sorted(tuple(iterable))`, use `sorted(iterable)`. + /// * Instead of `sorted(sorted(iterable))`, use `sorted(iterable)`. + /// * Instead of `sorted(reversed(iterable))`, use `sorted(iterable)`. pub struct UnnecessaryDoubleCastOrProcess { pub inner: String, pub outer: String, } ); -impl Violation for UnnecessaryDoubleCastOrProcess { +impl AlwaysAutofixableViolation for UnnecessaryDoubleCastOrProcess { #[derive_message_formats] fn message(&self) -> String { let UnnecessaryDoubleCastOrProcess { inner, outer } = self; format!("Unnecessary `{inner}` call within `{outer}()`") } + + fn autofix_title(&self) -> String { + let UnnecessaryDoubleCastOrProcess { inner, .. } = self; + format!("Remove the inner `{inner}` call") + } } /// C414 @@ -28,7 +70,7 @@ pub fn unnecessary_double_cast_or_process( func: &Expr, args: &[Expr], ) { - fn diagnostic(inner: &str, outer: &str, location: Range) -> Diagnostic { + fn create_diagnostic(inner: &str, outer: &str, location: Range) -> Diagnostic { Diagnostic::new( UnnecessaryDoubleCastOrProcess { inner: inner.to_string(), @@ -63,27 +105,23 @@ pub fn unnecessary_double_cast_or_process( } // Ex) set(tuple(...)) - if (outer == "set" || outer == "sorted") - && (inner == "list" || inner == "tuple" || inner == "reversed" || inner == "sorted") - { - checker - .diagnostics - .push(diagnostic(inner, outer, Range::from_located(expr))); - return; - } - // Ex) list(tuple(...)) - if (outer == "list" || outer == "tuple") && (inner == "list" || inner == "tuple") { - checker - .diagnostics - .push(diagnostic(inner, outer, Range::from_located(expr))); - return; - } - // Ex) set(set(...)) - if outer == "set" && inner == "set" { - checker - .diagnostics - .push(diagnostic(inner, outer, Range::from_located(expr))); + if ((outer == "set" || outer == "sorted") + && (inner == "list" || inner == "tuple" || inner == "reversed" || inner == "sorted")) + || (outer == "set" && inner == "set") + || ((outer == "list" || outer == "tuple") && (inner == "list" || inner == "tuple")) + { + let mut diagnostic = create_diagnostic(inner, outer, Range::from_located(expr)); + if checker.patch(diagnostic.kind.rule()) { + if let Ok(fix) = fixes::fix_unnecessary_double_cast_or_process( + checker.locator, + checker.stylist, + expr, + ) { + diagnostic.amend(fix); + } + } + checker.diagnostics.push(diagnostic); } } diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs index dc1d91b9b3..456cb77b44 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs @@ -1,18 +1,50 @@ -use ruff_macros::{define_violation, derive_message_formats}; +use log::error; use rustpython_parser::ast::{Expr, ExprKind}; -use super::helpers; +use ruff_macros::{define_violation, derive_message_formats}; + use crate::ast::types::Range; use crate::checkers::ast::Checker; use crate::registry::Diagnostic; -use crate::violation::Violation; +use crate::rules::flake8_comprehensions::fixes; +use crate::violation::{AutofixKind, Availability, Violation}; + +use super::helpers; define_violation!( + /// ## What it does + /// Checks for unnecessary `map` calls with `lambda` functions. + /// + /// ## Why is this bad? + /// Using `map(func, iterable)` when `func` is a `lambda` is slower than + /// using a generator expression or a comprehension, as the latter approach + /// avoids the function call overhead, in addition to being more readable. + /// + /// ## Examples + /// ```python + /// map(lambda x: x + 1, iterable) + /// ``` + /// + /// Use instead: + /// ```python + /// (x + 1 for x in iterable) + /// ``` + /// + /// This rule also applies to `map` calls within `list`, `set`, and `dict` + /// calls. For example: + /// * Instead of `list(map(lambda num: num * 2, nums))`, use + /// `[num * 2 for num in nums]`. + /// * Instead of `set(map(lambda num: num % 2 == 0, nums))`, use + /// `{num % 2 == 0 for num in nums}`. + /// * Instead of `dict(map(lambda v: (v, v ** 2), values))`, use + /// `{v: v ** 2 for v in values}`. pub struct UnnecessaryMap { pub obj_type: String, } ); impl Violation for UnnecessaryMap { + const AUTOFIX: Option = Some(AutofixKind::new(Availability::Sometimes)); + #[derive_message_formats] fn message(&self) -> String { let UnnecessaryMap { obj_type } = self; @@ -22,11 +54,27 @@ impl Violation for UnnecessaryMap { format!("Unnecessary `map` usage (rewrite using a `{obj_type}` comprehension)") } } + + fn autofix_title_formatter(&self) -> Option String> { + Some(|UnnecessaryMap { obj_type }| { + if obj_type == "generator" { + format!("Replace `map` using a generator expression") + } else { + format!("Replace `map` using a `{obj_type}` comprehension") + } + }) + } } /// C417 -pub fn unnecessary_map(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { - fn diagnostic(kind: &str, location: Range) -> Diagnostic { +pub fn unnecessary_map( + checker: &mut Checker, + expr: &Expr, + parent: Option<&Expr>, + func: &Expr, + args: &[Expr], +) { + fn create_diagnostic(kind: &str, location: Range) -> Diagnostic { Diagnostic::new( UnnecessaryMap { obj_type: kind.to_string(), @@ -44,10 +92,33 @@ pub fn unnecessary_map(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[ return; } + // Exclude the parent if already matched by other arms + if let Some(parent) = parent { + if let ExprKind::Call { func: f, .. } = &parent.node { + if let Some(id_parent) = helpers::function_name(f) { + if id_parent == "dict" || id_parent == "set" || id_parent == "list" { + return; + } + } + }; + }; + if args.len() == 2 && matches!(&args[0].node, ExprKind::Lambda { .. }) { - checker - .diagnostics - .push(diagnostic("generator", Range::from_located(expr))); + let mut diagnostic = create_diagnostic("generator", Range::from_located(expr)); + if checker.patch(diagnostic.kind.rule()) { + match fixes::fix_unnecessary_map( + checker.locator, + checker.stylist, + expr, + "generator", + ) { + Ok(fix) => { + diagnostic.amend(fix); + } + Err(e) => error!("Failed to generate fix: {e}"), + } + } + checker.diagnostics.push(diagnostic); } } "list" | "set" => { @@ -57,13 +128,28 @@ pub fn unnecessary_map(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[ if let Some(arg) = args.first() { if let ExprKind::Call { func, args, .. } = &arg.node { + if args.len() != 2 { + return; + } let Some(argument) = helpers::first_argument_with_matching_function("map", func, args) else { return; }; if let ExprKind::Lambda { .. } = argument { - checker - .diagnostics - .push(diagnostic(id, Range::from_located(expr))); + let mut diagnostic = create_diagnostic(id, Range::from_located(expr)); + if checker.patch(diagnostic.kind.rule()) { + match fixes::fix_unnecessary_map( + checker.locator, + checker.stylist, + expr, + id, + ) { + Ok(fix) => { + diagnostic.amend(fix); + } + Err(e) => error!("Failed to generate fix: {e}"), + } + } + checker.diagnostics.push(diagnostic); } } } @@ -81,9 +167,21 @@ pub fn unnecessary_map(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[ if let ExprKind::Lambda { body, .. } = &argument { if matches!(&body.node, ExprKind::Tuple { elts, .. } | ExprKind::List { elts, .. } if elts.len() == 2) { - checker - .diagnostics - .push(diagnostic(id, Range::from_located(expr))); + let mut diagnostic = create_diagnostic(id, Range::from_located(expr)); + if checker.patch(diagnostic.kind.rule()) { + match fixes::fix_unnecessary_map( + checker.locator, + checker.stylist, + expr, + id, + ) { + Ok(fix) => { + diagnostic.amend(fix); + } + Err(e) => error!("Failed to generate fix: {e}"), + } + } + checker.diagnostics.push(diagnostic); } } } diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap index 695aae3fc7..d34e57f601 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap @@ -1,5 +1,5 @@ --- -source: src/rules/flake8_comprehensions/mod.rs +source: crates/ruff/src/rules/flake8_comprehensions/mod.rs expression: diagnostics --- - kind: @@ -12,7 +12,15 @@ expression: diagnostics end_location: row: 2 column: 13 - fix: ~ + fix: + content: + - list(x) + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 13 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -24,7 +32,15 @@ expression: diagnostics end_location: row: 3 column: 14 - fix: ~ + fix: + content: + - list(x) + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 14 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -36,7 +52,15 @@ expression: diagnostics end_location: row: 4 column: 14 - fix: ~ + fix: + content: + - tuple(x) + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 14 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -48,7 +72,15 @@ expression: diagnostics end_location: row: 5 column: 15 - fix: ~ + fix: + content: + - tuple(x) + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 15 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -60,7 +92,15 @@ expression: diagnostics end_location: row: 6 column: 11 - fix: ~ + fix: + content: + - set(x) + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 11 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -72,7 +112,15 @@ expression: diagnostics end_location: row: 7 column: 12 - fix: ~ + fix: + content: + - set(x) + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 12 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -84,7 +132,15 @@ expression: diagnostics end_location: row: 8 column: 13 - fix: ~ + fix: + content: + - set(x) + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 13 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -96,7 +152,15 @@ expression: diagnostics end_location: row: 9 column: 14 - fix: ~ + fix: + content: + - set(x) + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 14 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -108,7 +172,15 @@ expression: diagnostics end_location: row: 10 column: 16 - fix: ~ + fix: + content: + - set(x) + location: + row: 10 + column: 0 + end_location: + row: 10 + column: 16 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -120,7 +192,15 @@ expression: diagnostics end_location: row: 11 column: 15 - fix: ~ + fix: + content: + - sorted(x) + location: + row: 11 + column: 0 + end_location: + row: 11 + column: 15 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -132,7 +212,15 @@ expression: diagnostics end_location: row: 12 column: 16 - fix: ~ + fix: + content: + - sorted(x) + location: + row: 12 + column: 0 + end_location: + row: 12 + column: 16 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -144,7 +232,15 @@ expression: diagnostics end_location: row: 13 column: 17 - fix: ~ + fix: + content: + - sorted(x) + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 17 parent: ~ - kind: UnnecessaryDoubleCastOrProcess: @@ -156,6 +252,37 @@ expression: diagnostics end_location: row: 14 column: 19 - fix: ~ + fix: + content: + - sorted(x) + location: + row: 14 + column: 0 + end_location: + row: 14 + column: 19 + parent: ~ +- kind: + UnnecessaryDoubleCastOrProcess: + inner: list + outer: tuple + location: + row: 15 + column: 0 + end_location: + row: 20 + column: 1 + fix: + content: + - tuple( + - " [x, 3, \"hell\"\\" + - " \"o\"]" + - " )" + location: + row: 15 + column: 0 + end_location: + row: 20 + column: 1 parent: ~ diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap index 0fadc735b2..5ce90483c7 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap @@ -1,93 +1,206 @@ --- -source: src/rules/flake8_comprehensions/mod.rs +source: crates/ruff/src/rules/flake8_comprehensions/mod.rs expression: diagnostics --- - kind: UnnecessaryMap: obj_type: generator location: - row: 2 + row: 3 column: 0 end_location: - row: 2 + row: 3 column: 26 - fix: ~ + fix: + content: + - (x + 1 for x in nums) + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 26 parent: ~ - kind: UnnecessaryMap: obj_type: generator location: - row: 3 + row: 4 column: 0 end_location: - row: 3 + row: 4 column: 27 - fix: ~ + fix: + content: + - (str(x) for x in nums) + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 27 parent: ~ - kind: UnnecessaryMap: obj_type: list location: - row: 4 + row: 5 column: 0 end_location: - row: 4 + row: 5 column: 32 - fix: ~ - parent: ~ -- kind: - UnnecessaryMap: - obj_type: generator - location: - row: 4 - column: 5 - end_location: - row: 4 - column: 31 - fix: ~ + fix: + content: + - "[x * 2 for x in nums]" + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 32 parent: ~ - kind: UnnecessaryMap: obj_type: set location: - row: 5 + row: 6 column: 0 end_location: - row: 5 + row: 6 column: 36 - fix: ~ - parent: ~ -- kind: - UnnecessaryMap: - obj_type: generator - location: - row: 5 - column: 4 - end_location: - row: 5 - column: 35 - fix: ~ + fix: + content: + - "{x % 2 == 0 for x in nums}" + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 36 parent: ~ - kind: UnnecessaryMap: obj_type: dict location: - row: 6 + row: 7 column: 0 end_location: - row: 6 + row: 7 column: 36 - fix: ~ + fix: + content: + - "{v: v**2 for v in nums}" + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 36 parent: ~ - kind: UnnecessaryMap: obj_type: generator location: - row: 6 - column: 5 + row: 8 + column: 0 end_location: - row: 6 + row: 8 + column: 26 + fix: + content: + - "(\"const\" for _ in nums)" + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 26 + parent: ~ +- kind: + UnnecessaryMap: + obj_type: generator + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 24 + fix: + content: + - (3.0 for _ in nums) + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 24 + parent: ~ +- kind: + UnnecessaryMap: + obj_type: generator + location: + row: 10 + column: 12 + end_location: + row: 10 + column: 63 + fix: + content: + - "(x in nums and \"1\" or \"0\" for x in range(123))" + location: + row: 10 + column: 12 + end_location: + row: 10 + column: 63 + parent: ~ +- kind: + UnnecessaryMap: + obj_type: generator + location: + row: 11 + column: 4 + end_location: + row: 11 + column: 44 + fix: + content: + - "(isinstance(v, dict) for v in nums)" + location: + row: 11 + column: 4 + end_location: + row: 11 + column: 44 + parent: ~ +- kind: + UnnecessaryMap: + obj_type: generator + location: + row: 12 + column: 13 + end_location: + row: 12 column: 35 + fix: + content: + - (v for v in nums) + location: + row: 12 + column: 13 + end_location: + row: 12 + column: 35 + parent: ~ +- kind: + UnnecessaryMap: + obj_type: generator + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 24 fix: ~ parent: ~ diff --git a/docs/rules/unnecessary-double-cast-or-process.md b/docs/rules/unnecessary-double-cast-or-process.md new file mode 100644 index 0000000000..bde28534d8 --- /dev/null +++ b/docs/rules/unnecessary-double-cast-or-process.md @@ -0,0 +1,40 @@ +# unnecessary-double-cast-or-process (C414) + +Derived from the **flake8-comprehensions** linter. + +Autofix is always available. + +## What it does +Checks for unnecessary `list`, `reversed`, `set`, `sorted`, and `tuple` +call within `list`, `set`, `sorted`, and `tuple` calls. + +## Why is this bad? +It's unnecessary to double-cast or double-process iterables by wrapping +the listed functions within an additional `list`, `set`, `sorted`, or +`tuple` call. Doing so is redundant and can be confusing for readers. + +## Examples +```python +list(tuple(iterable)) +``` + +Use instead: +```python +list(iterable) +``` + +This rule applies to a variety of functions, including `list`, `reversed`, +`set`, `sorted`, and `tuple`. For example: +* Instead of `list(list(iterable))`, use `list(iterable)`. +* Instead of `list(tuple(iterable))`, use `list(iterable)`. +* Instead of `tuple(list(iterable))`, use `tuple(iterable)`. +* Instead of `tuple(tuple(iterable))`, use `tuple(iterable)`. +* Instead of `set(set(iterable))`, use `set(iterable)`. +* Instead of `set(list(iterable))`, use `set(iterable)`. +* Instead of `set(tuple(iterable))`, use `set(iterable)`. +* Instead of `set(sorted(iterable))`, use `set(iterable)`. +* Instead of `set(reversed(iterable))`, use `set(iterable)`. +* Instead of `sorted(list(iterable))`, use `sorted(iterable)`. +* Instead of `sorted(tuple(iterable))`, use `sorted(iterable)`. +* Instead of `sorted(sorted(iterable))`, use `sorted(iterable)`. +* Instead of `sorted(reversed(iterable))`, use `sorted(iterable)`. \ No newline at end of file diff --git a/docs/rules/unnecessary-map.md b/docs/rules/unnecessary-map.md new file mode 100644 index 0000000000..847f2a4533 --- /dev/null +++ b/docs/rules/unnecessary-map.md @@ -0,0 +1,32 @@ +# unnecessary-map (C417) + +Derived from the **flake8-comprehensions** linter. + +Autofix is sometimes available. + +## What it does +Checks for unnecessary `map` calls with `lambda` functions. + +## Why is this bad? +Using `map(func, iterable)` when `func` is a `lambda` is slower than +using a generator expression or a comprehension, as the latter approach +avoids the function call overhead, in addition to being more readable. + +## Examples +```python +map(lambda x: x + 1, iterable) +``` + +Use instead: +```python +(x + 1 for x in iterable) +``` + +This rule also applies to `map` calls within `list`, `set`, and `dict` +calls. For example: +* Instead of `list(map(lambda num: num * 2, nums))`, use + `[num * 2 for num in nums]`. +* Instead of `set(map(lambda num: num % 2 == 0, nums))`, use + `{num % 2 == 0 for num in nums}`. +* Instead of `dict(map(lambda v: (v, v ** 2), values))`, use + `{v: v ** 2 for v in values}`. \ No newline at end of file