From 213d5d4a0d87689177dd90dab0ea4216488050b0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 29 May 2024 23:05:22 -0400 Subject: [PATCH] Strip parentheses around generators in C400 --- .../fixtures/flake8_comprehensions/C400.py | 5 ++ .../rules/unnecessary_generator_list.rs | 31 ++++++++- ...8_comprehensions__tests__C400_C400.py.snap | 67 ++++++++++++++++++- 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py index 16e29e4fb3364..5c347f72adf58 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C400.py @@ -11,6 +11,11 @@ x for x in range(3) ) +# Strip parentheses from inner generators. +list((2 * x for x in range(3))) +list(((2 * x for x in range(3)))) +list((((2 * x for x in range(3))))) + # Not built-in list. def list(*args, **kwargs): return None diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs index 73ea7269a4be4..5166fbdd3eff2 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs @@ -2,6 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::ExprGenerator; use ruff_text_size::{Ranged, TextSize}; @@ -26,12 +27,14 @@ use super::helpers; /// ```python /// list(f(x) for x in foo) /// list(x for x in foo) +/// list((x for x in foo)) /// ``` /// /// Use instead: /// ```python /// [f(x) for x in foo] /// list(foo) +/// list(foo) /// ``` /// /// ## Fix safety @@ -76,7 +79,10 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr } let Some(ExprGenerator { - elt, generators, .. + elt, + generators, + parenthesized, + .. }) = argument.as_generator_expr() else { return; @@ -125,7 +131,28 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr call.end(), ); - Fix::unsafe_edits(call_start, [call_end]) + // Remove the inner parentheses, if the expression is a generator. The easiest way to do + // this reliably is to use the printer. + if *parenthesized { + // The generator's range will include the innermost parentheses, but it could be + // surrounded by additional parentheses. + let range = parenthesized_range( + argument.into(), + (&call.arguments).into(), + checker.indexer().comment_ranges(), + checker.locator().contents(), + ) + .unwrap_or(argument.range()); + + // The generator always parenthesizes the expression; trim the parentheses. + let generator = checker.generator().expr(argument); + let generator = generator[1..generator.len() - 1].to_string(); + + let replacement = Edit::range_replacement(generator, range); + Fix::unsafe_edits(call_start, [call_end, replacement]) + } else { + Fix::unsafe_edits(call_start, [call_end]) + } }); checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap index afc151e684810..d64116390469b 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C400_C400.py.snap @@ -73,7 +73,7 @@ C400.py:10:5: C400 [*] Unnecessary generator (rewrite using `list()`) 12 | | ) | |_^ C400 13 | -14 | # Not built-in list. +14 | # Strip parentheses from inner generators. | = help: Rewrite using `list()` @@ -86,5 +86,66 @@ C400.py:10:5: C400 [*] Unnecessary generator (rewrite using `list()`) 12 |-) 10 |+x = list(range(3)) 13 11 | -14 12 | # Not built-in list. -15 13 | def list(*args, **kwargs): +14 12 | # Strip parentheses from inner generators. +15 13 | list((2 * x for x in range(3))) + +C400.py:15:1: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) + | +14 | # Strip parentheses from inner generators. +15 | list((2 * x for x in range(3))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400 +16 | list(((2 * x for x in range(3)))) +17 | list((((2 * x for x in range(3))))) + | + = help: Rewrite as a `list` comprehension + +ℹ Unsafe fix +12 12 | ) +13 13 | +14 14 | # Strip parentheses from inner generators. +15 |-list((2 * x for x in range(3))) + 15 |+[2 * x for x in range(3)] +16 16 | list(((2 * x for x in range(3)))) +17 17 | list((((2 * x for x in range(3))))) +18 18 | + +C400.py:16:1: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) + | +14 | # Strip parentheses from inner generators. +15 | list((2 * x for x in range(3))) +16 | list(((2 * x for x in range(3)))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400 +17 | list((((2 * x for x in range(3))))) + | + = help: Rewrite as a `list` comprehension + +ℹ Unsafe fix +13 13 | +14 14 | # Strip parentheses from inner generators. +15 15 | list((2 * x for x in range(3))) +16 |-list(((2 * x for x in range(3)))) + 16 |+[2 * x for x in range(3)] +17 17 | list((((2 * x for x in range(3))))) +18 18 | +19 19 | # Not built-in list. + +C400.py:17:1: C400 [*] Unnecessary generator (rewrite as a `list` comprehension) + | +15 | list((2 * x for x in range(3))) +16 | list(((2 * x for x in range(3)))) +17 | list((((2 * x for x in range(3))))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C400 +18 | +19 | # Not built-in list. + | + = help: Rewrite as a `list` comprehension + +ℹ Unsafe fix +14 14 | # Strip parentheses from inner generators. +15 15 | list((2 * x for x in range(3))) +16 16 | list(((2 * x for x in range(3)))) +17 |-list((((2 * x for x in range(3))))) + 17 |+[2 * x for x in range(3)] +18 18 | +19 19 | # Not built-in list. +20 20 | def list(*args, **kwargs):