diff --git a/README.md b/README.md index faef76d6a47f4..08ceb808a6a30 100644 --- a/README.md +++ b/README.md @@ -1036,9 +1036,9 @@ For more, see [flake8-comprehensions](https://pypi.org/project/flake8-comprehens | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | -| C400 | unnecessary-generator-list | Unnecessary generator (rewrite as a `list` comprehension) | 🛠 | -| C401 | unnecessary-generator-set | Unnecessary generator (rewrite as a `set` comprehension) | 🛠 | -| C402 | unnecessary-generator-dict | Unnecessary generator (rewrite as a `dict` comprehension) | 🛠 | +| C400 | [unnecessary-generator-list](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-generator-list.md) | Unnecessary generator (rewrite as a `list` comprehension) | 🛠 | +| C401 | [unnecessary-generator-set](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-generator-set.md) | Unnecessary generator (rewrite as a `set` comprehension) | 🛠 | +| C402 | [unnecessary-generator-dict](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-generator-dict.md) | Unnecessary generator (rewrite as a `dict` comprehension) | 🛠 | | C403 | unnecessary-list-comprehension-set | Unnecessary `list` comprehension (rewrite as a `set` comprehension) | 🛠 | | C404 | unnecessary-list-comprehension-dict | Unnecessary `list` comprehension (rewrite as a `dict` comprehension) | 🛠 | | C405 | unnecessary-literal-set | Unnecessary `{obj_type}` literal (rewrite as a `set` literal) | 🛠 | diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C401.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C401.py index fd4637cb4366a..f38ca1e8737ab 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C401.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C401.py @@ -2,7 +2,9 @@ x = set( x for x in range(3) ) - +y = f'{set(a if a < 6 else 0 for a in range(3))}' +_ = '{}'.format(set(a if a < 6 else 0 for a in range(3))) +print(f'Hello {set(a for a in range(3))} World') def set(*args, **kwargs): return None diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C402.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C402.py index fa4db2511733c..20db252700f48 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C402.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C402.py @@ -3,3 +3,5 @@ (x, x) for x in range(3) ) dict(((x, x) for x in range(3)), z=3) +y = f'{dict((x, x) for x in range(3))}' +print(f'Hello {dict((x, x) for x in range(3))} World') diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py index 8abbdab6c812b..e0353b49c3dad 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py @@ -11,6 +11,10 @@ all(map(lambda v: isinstance(v, dict), nums)) filter(func, map(lambda v: v, nums)) +# When inside f-string, then the fix should be surrounded by whitespace +_ = f"{set(map(lambda x: x % 2 == 0, nums))}" +_ = f"{dict(map(lambda v: (v, v**2), 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)`. diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 5bead3b1cc69f..14596b849b89a 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -2434,12 +2434,22 @@ where } if self.settings.rules.enabled(&Rule::UnnecessaryGeneratorSet) { flake8_comprehensions::rules::unnecessary_generator_set( - self, expr, func, args, keywords, + self, + expr, + self.current_expr_parent().map(Into::into), + func, + args, + keywords, ); } if self.settings.rules.enabled(&Rule::UnnecessaryGeneratorDict) { flake8_comprehensions::rules::unnecessary_generator_dict( - self, expr, func, args, keywords, + self, + expr, + self.current_expr_parent().map(Into::into), + func, + args, + keywords, ); } if self diff --git a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs index 90b93e73aa341..b115986aac124 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs @@ -79,6 +79,7 @@ pub fn fix_unnecessary_generator_set( locator: &Locator, stylist: &Stylist, expr: &rustpython_parser::ast::Expr, + parent: Option<&rustpython_parser::ast::Expr>, ) -> Result { // Expr(Call(GeneratorExp)))) -> Expr(SetComp))) let module_text = locator.slice_source_code_range(&Range::from_located(expr)); @@ -113,8 +114,18 @@ pub fn fix_unnecessary_generator_set( }; tree.codegen(&mut state); + let mut content = state.to_string(); + + // If the expression is embedded in an f-string, surround it with spaces to avoid + // syntax errors. + if let Some(parent_element) = parent { + if let &rustpython_parser::ast::ExprKind::FormattedValue { .. } = &parent_element.node { + content = format!(" {content} "); + } + } + Ok(Fix::replacement( - state.to_string(), + content, expr.location, expr.end_location.unwrap(), )) @@ -126,6 +137,7 @@ pub fn fix_unnecessary_generator_dict( locator: &Locator, stylist: &Stylist, expr: &rustpython_parser::ast::Expr, + parent: Option<&rustpython_parser::ast::Expr>, ) -> Result { let module_text = locator.slice_source_code_range(&Range::from_located(expr)); let mut tree = match_module(module_text)?; @@ -176,8 +188,18 @@ pub fn fix_unnecessary_generator_dict( }; tree.codegen(&mut state); + let mut content = state.to_string(); + + // If the expression is embedded in an f-string, surround it with spaces to avoid + // syntax errors. + if let Some(parent_element) = parent { + if let &rustpython_parser::ast::ExprKind::FormattedValue { .. } = &parent_element.node { + content = format!(" {content} "); + } + } + Ok(Fix::replacement( - state.to_string(), + content, expr.location, expr.end_location.unwrap(), )) @@ -921,6 +943,7 @@ pub fn fix_unnecessary_map( locator: &Locator, stylist: &Stylist, expr: &rustpython_parser::ast::Expr, + parent: Option<&rustpython_parser::ast::Expr>, kind: &str, ) -> Result { let module_text = locator.slice_source_code_range(&Range::from_located(expr)); @@ -1061,8 +1084,22 @@ pub fn fix_unnecessary_map( }; tree.codegen(&mut state); + let mut content = state.to_string(); + + // If the expression is embedded in an f-string, surround it with spaces to avoid + // syntax errors. + if kind == "set" || kind == "dict" { + if let Some(parent_element) = parent { + if let &rustpython_parser::ast::ExprKind::FormattedValue { .. } = + &parent_element.node + { + content = format!(" {content} "); + } + } + } + Ok(Fix::replacement( - state.to_string(), + content, expr.location, expr.end_location.unwrap(), )) diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_dict.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_dict.rs index 3b8ad8bb0f8a7..46322bac26ec4 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_dict.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_dict.rs @@ -10,6 +10,24 @@ use crate::rules::flake8_comprehensions::fixes; use crate::violation::AlwaysAutofixableViolation; define_violation!( + /// ## What it does + /// Checks for unnecessary generators that can be rewritten as `dict` + /// comprehensions. + /// + /// ## Why is this bad? + /// It is unnecessary to use `dict` around a generator expression, since + /// there are equivalent comprehensions for these types. Using a + /// comprehension is clearer and more idiomatic. + /// + /// ## Examples + /// ```python + /// dict((x, f(x)) for x in foo) + /// ``` + /// + /// Use instead: + /// ```python + /// {x: f(x) for x in foo} + /// ``` pub struct UnnecessaryGeneratorDict; ); impl AlwaysAutofixableViolation for UnnecessaryGeneratorDict { @@ -27,6 +45,7 @@ impl AlwaysAutofixableViolation for UnnecessaryGeneratorDict { pub fn unnecessary_generator_dict( checker: &mut Checker, expr: &Expr, + parent: Option<&Expr>, func: &Expr, args: &[Expr], keywords: &[Keyword], @@ -44,6 +63,7 @@ pub fn unnecessary_generator_dict( checker.locator, checker.stylist, expr, + parent, ) { Ok(fix) => { diagnostic.amend(fix); diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs index 147cb8ea2d78d..a9defc36165fc 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs @@ -10,6 +10,24 @@ use crate::rules::flake8_comprehensions::fixes; use crate::violation::AlwaysAutofixableViolation; define_violation!( + /// ## What it does + /// Checks for unnecessary generators that can be rewritten as `list` + /// comprehensions. + /// + /// ## Why is this bad? + /// It is unnecessary to use `list` around a generator expression, since + /// there are equivalent comprehensions for these types. Using a + /// comprehension is clearer and more idiomatic. + /// + /// ## Examples + /// ```python + /// list(f(x) for x in foo) + /// ``` + /// + /// Use instead: + /// ```python + /// [f(x) for x in foo] + /// ``` pub struct UnnecessaryGeneratorList; ); impl AlwaysAutofixableViolation for UnnecessaryGeneratorList { diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_set.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_set.rs index 0a8a8f7ad7ecb..bb9236457f882 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_set.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_generator_set.rs @@ -10,6 +10,24 @@ use crate::rules::flake8_comprehensions::fixes; use crate::violation::AlwaysAutofixableViolation; define_violation!( + /// ## What it does + /// Checks for unnecessary generators that can be rewritten as `set` + /// comprehensions. + /// + /// ## Why is this bad? + /// It is unnecessary to use `set` around a generator expression, since + /// there are equivalent comprehensions for these types. Using a + /// comprehension is clearer and more idiomatic. + /// + /// ## Examples + /// ```python + /// set(f(x) for x in foo) + /// ``` + /// + /// Use instead: + /// ```python + /// {f(x) for x in foo} + /// ``` pub struct UnnecessaryGeneratorSet; ); impl AlwaysAutofixableViolation for UnnecessaryGeneratorSet { @@ -27,6 +45,7 @@ impl AlwaysAutofixableViolation for UnnecessaryGeneratorSet { pub fn unnecessary_generator_set( checker: &mut Checker, expr: &Expr, + parent: Option<&Expr>, func: &Expr, args: &[Expr], keywords: &[Keyword], @@ -40,7 +59,12 @@ pub fn unnecessary_generator_set( if let ExprKind::GeneratorExp { .. } = argument { let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorSet, Range::from_located(expr)); if checker.patch(diagnostic.kind.rule()) { - match fixes::fix_unnecessary_generator_set(checker.locator, checker.stylist, expr) { + match fixes::fix_unnecessary_generator_set( + checker.locator, + checker.stylist, + expr, + parent, + ) { Ok(fix) => { diagnostic.amend(fix); } 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 456cb77b449f2..22c6e94876df5 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs @@ -110,6 +110,7 @@ pub fn unnecessary_map( checker.locator, checker.stylist, expr, + parent, "generator", ) { Ok(fix) => { @@ -141,6 +142,7 @@ pub fn unnecessary_map( checker.locator, checker.stylist, expr, + parent, id, ) { Ok(fix) => { @@ -173,6 +175,7 @@ pub fn unnecessary_map( checker.locator, checker.stylist, expr, + parent, id, ) { Ok(fix) => { diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C401_C401.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C401_C401.py.snap index 857fd2c2f745d..2eb34a4d58aa2 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C401_C401.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C401_C401.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: @@ -40,4 +40,58 @@ expression: diagnostics row: 4 column: 1 parent: ~ +- kind: + UnnecessaryGeneratorSet: ~ + location: + row: 5 + column: 7 + end_location: + row: 5 + column: 48 + fix: + content: + - " {a if a < 6 else 0 for a in range(3)} " + location: + row: 5 + column: 7 + end_location: + row: 5 + column: 48 + parent: ~ +- kind: + UnnecessaryGeneratorSet: ~ + location: + row: 6 + column: 16 + end_location: + row: 6 + column: 57 + fix: + content: + - "{a if a < 6 else 0 for a in range(3)}" + location: + row: 6 + column: 16 + end_location: + row: 6 + column: 57 + parent: ~ +- kind: + UnnecessaryGeneratorSet: ~ + location: + row: 7 + column: 15 + end_location: + row: 7 + column: 39 + fix: + content: + - " {a for a in range(3)} " + location: + row: 7 + column: 15 + end_location: + row: 7 + column: 39 + parent: ~ diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C402_C402.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C402_C402.py.snap index 22424ba5df3ee..af2729e4967ef 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C402_C402.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C402_C402.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: @@ -40,4 +40,40 @@ expression: diagnostics row: 4 column: 1 parent: ~ +- kind: + UnnecessaryGeneratorDict: ~ + location: + row: 6 + column: 7 + end_location: + row: 6 + column: 37 + fix: + content: + - " {x: x for x in range(3)} " + location: + row: 6 + column: 7 + end_location: + row: 6 + column: 37 + parent: ~ +- kind: + UnnecessaryGeneratorDict: ~ + location: + row: 7 + column: 15 + end_location: + row: 7 + column: 45 + fix: + content: + - " {x: x for x in range(3)} " + location: + row: 7 + column: 15 + end_location: + row: 7 + column: 45 + 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 5ce90483c79c6..86a38f45b29a6 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 @@ -192,14 +192,52 @@ expression: diagnostics row: 12 column: 35 parent: ~ +- kind: + UnnecessaryMap: + obj_type: set + location: + row: 15 + column: 7 + end_location: + row: 15 + column: 43 + fix: + content: + - " {x % 2 == 0 for x in nums} " + location: + row: 15 + column: 7 + end_location: + row: 15 + column: 43 + parent: ~ +- kind: + UnnecessaryMap: + obj_type: dict + location: + row: 16 + column: 7 + end_location: + row: 16 + column: 43 + fix: + content: + - " {v: v**2 for v in nums} " + location: + row: 16 + column: 7 + end_location: + row: 16 + column: 43 + parent: ~ - kind: UnnecessaryMap: obj_type: generator location: - row: 17 + row: 21 column: 0 end_location: - row: 17 + row: 21 column: 24 fix: ~ parent: ~ diff --git a/docs/rules/unnecessary-generator-dict.md b/docs/rules/unnecessary-generator-dict.md new file mode 100644 index 0000000000000..c8220429cddaa --- /dev/null +++ b/docs/rules/unnecessary-generator-dict.md @@ -0,0 +1,24 @@ +# unnecessary-generator-dict (C402) + +Derived from the **flake8-comprehensions** linter. + +Autofix is always available. + +## What it does +Checks for unnecessary generators that can be rewritten as `dict` +comprehensions. + +## Why is this bad? +It is unnecessary to use `dict` around a generator expression, since +there are equivalent comprehensions for these types. Using a +comprehension is clearer and more idiomatic. + +## Examples +```python +dict((x, f(x)) for x in foo) +``` + +Use instead: +```python +{x: f(x) for x in foo} +``` \ No newline at end of file diff --git a/docs/rules/unnecessary-generator-list.md b/docs/rules/unnecessary-generator-list.md new file mode 100644 index 0000000000000..df595115eb915 --- /dev/null +++ b/docs/rules/unnecessary-generator-list.md @@ -0,0 +1,24 @@ +# unnecessary-generator-list (C400) + +Derived from the **flake8-comprehensions** linter. + +Autofix is always available. + +## What it does +Checks for unnecessary generators that can be rewritten as `list` +comprehensions. + +## Why is this bad? +It is unnecessary to use `list` around a generator expression, since +there are equivalent comprehensions for these types. Using a +comprehension is clearer and more idiomatic. + +## Examples +```python +list(f(x) for x in foo) +``` + +Use instead: +```python +[f(x) for x in foo] +``` \ No newline at end of file diff --git a/docs/rules/unnecessary-generator-set.md b/docs/rules/unnecessary-generator-set.md new file mode 100644 index 0000000000000..ea5bc2c593b7d --- /dev/null +++ b/docs/rules/unnecessary-generator-set.md @@ -0,0 +1,24 @@ +# unnecessary-generator-set (C401) + +Derived from the **flake8-comprehensions** linter. + +Autofix is always available. + +## What it does +Checks for unnecessary generators that can be rewritten as `set` +comprehensions. + +## Why is this bad? +It is unnecessary to use `set` around a generator expression, since +there are equivalent comprehensions for these types. Using a +comprehension is clearer and more idiomatic. + +## Examples +```python +set(f(x) for x in foo) +``` + +Use instead: +```python +{f(x) for x in foo} +``` \ No newline at end of file