From c0bb83b88279f5ea21a3b9e8910d23e8437c89b5 Mon Sep 17 00:00:00 2001 From: chiri Date: Thu, 5 Jun 2025 17:57:08 +0300 Subject: [PATCH] [`perflint`] fix missing parentheses for lambda and ternary conditions (PERF401, PERF403) (#18412) Closes #18405 --- .../test/fixtures/perflint/PERF401.py | 12 +++++ .../test/fixtures/perflint/PERF403.py | 13 +++++ .../rules/manual_dict_comprehension.rs | 9 ++-- .../rules/manual_list_comprehension.rs | 11 +++-- ...__perflint__tests__PERF401_PERF401.py.snap | 22 +++++++++ ...__perflint__tests__PERF403_PERF403.py.snap | 20 ++++++++ ...t__tests__preview__PERF401_PERF401.py.snap | 48 +++++++++++++++++++ ...t__tests__preview__PERF403_PERF403.py.snap | 46 ++++++++++++++++++ 8 files changed, 172 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py index 263f0ff6c6..880ca4c09a 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py @@ -266,3 +266,15 @@ def f(): result = list() # this should be replaced with a comprehension for i in values: result.append(i + 1) # PERF401 + +def f(): + src = [1] + dst = [] + + for i in src: + if True if True else False: + dst.append(i) + + for i in src: + if lambda: 0: + dst.append(i) diff --git a/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py b/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py index 89afca39ff..9264af4f0f 100644 --- a/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py +++ b/crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py @@ -151,3 +151,16 @@ def foo(): result = {} for idx, name in indices, fruit: result[name] = idx # PERF403 + + +def foo(): + src = (("x", 1),) + dst = {} + + for k, v in src: + if True if True else False: + dst[k] = v + + for k, v in src: + if lambda: 0: + dst[k] = v \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs b/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs index 298f62af96..be8c6feb0f 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs @@ -346,10 +346,11 @@ fn convert_to_dict_comprehension( // since if the assignment expression appears // internally (e.g. as an operand in a boolean // operation) then it will already be parenthesized. - if test.is_named_expr() { - format!(" if ({})", locator.slice(test.range())) - } else { - format!(" if {}", locator.slice(test.range())) + match test { + Expr::Named(_) | Expr::If(_) | Expr::Lambda(_) => { + format!(" if ({})", locator.slice(test.range())) + } + _ => format!(" if {}", locator.slice(test.range())), } } None => String::new(), diff --git a/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs b/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs index 2566ecb770..da95d98ad3 100644 --- a/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs +++ b/crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs @@ -358,7 +358,7 @@ fn convert_to_list_extend( fix_type: ComprehensionType, binding: &Binding, for_stmt: &ast::StmtFor, - if_test: Option<&ast::Expr>, + if_test: Option<&Expr>, to_append: &Expr, checker: &Checker, ) -> Result { @@ -374,10 +374,11 @@ fn convert_to_list_extend( // since if the assignment expression appears // internally (e.g. as an operand in a boolean // operation) then it will already be parenthesized. - if test.is_named_expr() { - format!(" if ({})", locator.slice(test.range())) - } else { - format!(" if {}", locator.slice(test.range())) + match test { + Expr::Named(_) | Expr::If(_) | Expr::Lambda(_) => { + format!(" if ({})", locator.slice(test.range())) + } + _ => format!(" if {}", locator.slice(test.range())), } } None => String::new(), diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap index 92d7d6df85..d451379bcf 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap @@ -219,5 +219,27 @@ PERF401.py:268:9: PERF401 Use a list comprehension to create a transformed list 267 | for i in values: 268 | result.append(i + 1) # PERF401 | ^^^^^^^^^^^^^^^^^^^^ PERF401 +269 | +270 | def f(): | = help: Replace for loop with list comprehension + +PERF401.py:276:13: PERF401 Use a list comprehension to create a transformed list + | +274 | for i in src: +275 | if True if True else False: +276 | dst.append(i) + | ^^^^^^^^^^^^^ PERF401 +277 | +278 | for i in src: + | + = help: Replace for loop with list comprehension + +PERF401.py:280:13: PERF401 Use `list.extend` to create a transformed list + | +278 | for i in src: +279 | if lambda: 0: +280 | dst.append(i) + | ^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list.extend diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap index 2615a1772d..f398eae1da 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap @@ -128,3 +128,23 @@ PERF403.py:153:9: PERF403 Use a dictionary comprehension instead of a for-loop | ^^^^^^^^^^^^^^^^^^ PERF403 | = help: Replace for loop with dict comprehension + +PERF403.py:162:13: PERF403 Use a dictionary comprehension instead of a for-loop + | +160 | for k, v in src: +161 | if True if True else False: +162 | dst[k] = v + | ^^^^^^^^^^ PERF403 +163 | +164 | for k, v in src: + | + = help: Replace for loop with dict comprehension + +PERF403.py:166:13: PERF403 Use a dictionary comprehension instead of a for-loop + | +164 | for k, v in src: +165 | if lambda: 0: +166 | dst[k] = v + | ^^^^^^^^^^ PERF403 + | + = help: Replace for loop with dict comprehension diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap index 35f6a6bd92..d5b614bfe9 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap @@ -517,6 +517,8 @@ PERF401.py:268:9: PERF401 [*] Use a list comprehension to create a transformed l 267 | for i in values: 268 | result.append(i + 1) # PERF401 | ^^^^^^^^^^^^^^^^^^^^ PERF401 +269 | +270 | def f(): | = help: Replace for loop with list comprehension @@ -529,3 +531,49 @@ PERF401.py:268:9: PERF401 [*] Use a list comprehension to create a transformed l 268 |- result.append(i + 1) # PERF401 266 |+ # this should be replaced with a comprehension 267 |+ result = [i + 1 for i in values] # PERF401 +269 268 | +270 269 | def f(): +271 270 | src = [1] + +PERF401.py:276:13: PERF401 [*] Use a list comprehension to create a transformed list + | +274 | for i in src: +275 | if True if True else False: +276 | dst.append(i) + | ^^^^^^^^^^^^^ PERF401 +277 | +278 | for i in src: + | + = help: Replace for loop with list comprehension + +ℹ Unsafe fix +269 269 | +270 270 | def f(): +271 271 | src = [1] +272 |- dst = [] +273 272 | +274 |- for i in src: +275 |- if True if True else False: +276 |- dst.append(i) + 273 |+ dst = [i for i in src if (True if True else False)] +277 274 | +278 275 | for i in src: +279 276 | if lambda: 0: + +PERF401.py:280:13: PERF401 [*] Use `list.extend` to create a transformed list + | +278 | for i in src: +279 | if lambda: 0: +280 | dst.append(i) + | ^^^^^^^^^^^^^ PERF401 + | + = help: Replace for loop with list.extend + +ℹ Unsafe fix +275 275 | if True if True else False: +276 276 | dst.append(i) +277 277 | +278 |- for i in src: +279 |- if lambda: 0: +280 |- dst.append(i) + 278 |+ dst.extend(i for i in src if (lambda: 0)) diff --git a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap index 9e01e0db30..b83358171c 100644 --- a/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap +++ b/crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap @@ -305,3 +305,49 @@ PERF403.py:153:9: PERF403 [*] Use a dictionary comprehension instead of a for-lo 152 |- for idx, name in indices, fruit: 153 |- result[name] = idx # PERF403 151 |+ result = {name: idx for idx, name in (indices, fruit)} # PERF403 +154 152 | +155 153 | +156 154 | def foo(): + +PERF403.py:162:13: PERF403 [*] Use a dictionary comprehension instead of a for-loop + | +160 | for k, v in src: +161 | if True if True else False: +162 | dst[k] = v + | ^^^^^^^^^^ PERF403 +163 | +164 | for k, v in src: + | + = help: Replace for loop with dict comprehension + +ℹ Unsafe fix +155 155 | +156 156 | def foo(): +157 157 | src = (("x", 1),) +158 |- dst = {} +159 158 | +160 |- for k, v in src: +161 |- if True if True else False: +162 |- dst[k] = v + 159 |+ dst = {k: v for k, v in src if (True if True else False)} +163 160 | +164 161 | for k, v in src: +165 162 | if lambda: 0: + +PERF403.py:166:13: PERF403 [*] Use `dict.update` instead of a for-loop + | +164 | for k, v in src: +165 | if lambda: 0: +166 | dst[k] = v + | ^^^^^^^^^^ PERF403 + | + = help: Replace for loop with `dict.update` + +ℹ Unsafe fix +161 161 | if True if True else False: +162 162 | dst[k] = v +163 163 | +164 |- for k, v in src: +165 |- if lambda: 0: +166 |- dst[k] = v + 164 |+ dst.update({k: v for k, v in src if (lambda: 0)})