From 06a78d0bd00ad394ae6de818511fbb1ad236be49 Mon Sep 17 00:00:00 2001
From: Victor Hugo Gomes <labatata101@gmail.com>
Date: Sun, 22 Jun 2025 21:32:57 -0300
Subject: [PATCH] [`pylint`] Fix `PLC1802` autofix creating a syntax error and
 mark autofix as unsafe if there's comments in the `len` call (#18836)

<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
-->

## Summary
I've also found another bug while fixing this, where the diagnostic
would not trigger if the `len` call argument variable was shadowed. This
fixed a few false negatives in the test cases.
Example:
```python
fruits = []
fruits = []
if len(fruits):  # comment
    ...
```

Fixes #18811
Fixes #18812
<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan
Add regression test
<!-- How was it tested? -->

---------

Co-authored-by: Charlie Marsh <crmarsh416@gmail.com>
---
 .../test/fixtures/pylint/len_as_condition.py  |  16 ++-
 .../src/rules/pylint/rules/len_test.rs        |  37 ++++--
 ...t__tests__PLC1802_len_as_condition.py.snap | 111 ++++++++++++++++++
 3 files changed, 154 insertions(+), 10 deletions(-)

diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/len_as_condition.py b/crates/ruff_linter/resources/test/fixtures/pylint/len_as_condition.py
index 973e14386c..fbafe48183 100644
--- a/crates/ruff_linter/resources/test/fixtures/pylint/len_as_condition.py
+++ b/crates/ruff_linter/resources/test/fixtures/pylint/len_as_condition.py
@@ -168,7 +168,7 @@ def github_issue_1879():
     def function_returning_function(r):
         return function_returning_generator(r)
 
-    assert len(function_returning_list(z))  # [PLC1802] differs from pylint 
+    assert len(function_returning_list(z))  # [PLC1802] differs from pylint
     assert len(function_returning_int(z))
     # This should raise a PLC1802 once astroid can infer it
     # See https://github.com/pylint-dev/pylint/pull/3821#issuecomment-743771514
@@ -196,7 +196,7 @@ def f(cond:bool):
 def g(cond:bool):
     x = [1,2,3]
     if cond:
-        x = [4,5,6] 
+        x = [4,5,6]
     if len(x): # this should be addressed
         print(x)
     del x
@@ -236,3 +236,15 @@ def j():
 # regression tests for https://github.com/astral-sh/ruff/issues/14690
 bool(len(ascii(1)))
 bool(len(sorted("")))
+
+# regression tests for https://github.com/astral-sh/ruff/issues/18811
+fruits = []
+if(len)(fruits):
+    ...
+
+# regression tests for https://github.com/astral-sh/ruff/issues/18812
+fruits = []
+if len(
+    fruits  # comment
+):
+    ...
diff --git a/crates/ruff_linter/src/rules/pylint/rules/len_test.rs b/crates/ruff_linter/src/rules/pylint/rules/len_test.rs
index 894b854cbf..1a664eadda 100644
--- a/crates/ruff_linter/src/rules/pylint/rules/len_test.rs
+++ b/crates/ruff_linter/src/rules/pylint/rules/len_test.rs
@@ -1,11 +1,13 @@
+use ruff_diagnostics::Applicability;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::{self as ast, Expr, ExprCall};
+use ruff_python_semantic::SemanticModel;
 use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
 use ruff_python_semantic::analyze::typing::find_binding_value;
-use ruff_python_semantic::{BindingId, SemanticModel};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
+use crate::fix::edits;
 use crate::fix::snippet::SourceCodeSnippet;
 use crate::{AlwaysFixableViolation, Edit, Fix};
 
@@ -41,6 +43,19 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
 ///     print(vegetables)
 /// ```
 ///
+/// ## Fix safety
+/// This rule's fix is marked as unsafe when the `len` call includes a comment,
+/// as the comment would be removed.
+///
+/// For example, the fix would be marked as unsafe in the following case:
+/// ```python
+/// fruits = []
+/// if len(
+///     fruits  # comment
+/// ):
+///     ...
+/// ```
+///
 /// ## References
 /// [PEP 8: Programming Recommendations](https://peps.python.org/pep-0008/#programming-recommendations)
 #[derive(ViolationMetadata)]
@@ -98,10 +113,17 @@ pub(crate) fn len_test(checker: &Checker, call: &ExprCall) {
             },
             call.range(),
         )
-        .set_fix(Fix::safe_edit(Edit::range_replacement(
-            replacement,
-            call.range(),
-        )));
+        .set_fix(Fix::applicable_edit(
+            Edit::range_replacement(
+                edits::pad(replacement, call.range(), checker.locator()),
+                call.range(),
+            ),
+            if checker.comment_ranges().intersects(call.range()) {
+                Applicability::Unsafe
+            } else {
+                Applicability::Safe
+            },
+        ));
 }
 
 fn is_indirect_sequence(expr: &Expr, semantic: &SemanticModel) -> bool {
@@ -110,12 +132,11 @@ fn is_indirect_sequence(expr: &Expr, semantic: &SemanticModel) -> bool {
     };
 
     let scope = semantic.current_scope();
-    let bindings: Vec<BindingId> = scope.get_all(name).collect();
-    let [binding_id] = bindings.as_slice() else {
+    let Some(binding_id) = scope.get(name) else {
         return false;
     };
 
-    let binding = semantic.binding(*binding_id);
+    let binding = semantic.binding(binding_id);
 
     // Attempt to find the binding's value
     let Some(binding_value) = find_binding_value(binding, semantic) else {
diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC1802_len_as_condition.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC1802_len_as_condition.py.snap
index 2b64669ae4..93d27a1605 100644
--- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC1802_len_as_condition.py.snap
+++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC1802_len_as_condition.py.snap
@@ -396,6 +396,47 @@ len_as_condition.py:130:12: PLC1802 [*] `len(set((w + 1) for w in set()))` used
 132 132 | 
 133 133 |     import numpy
 
+len_as_condition.py:193:8: PLC1802 [*] `len(x)` used as condition without comparison
+    |
+191 |     if cond:
+192 |         x = [4,5,6]
+193 |     if len(x): # this should be addressed
+    |        ^^^^^^ PLC1802
+194 |         print(x)
+    |
+    = help: Remove `len`
+
+ℹ Safe fix
+190 190 |     x = [1,2,3]
+191 191 |     if cond:
+192 192 |         x = [4,5,6]
+193     |-    if len(x): # this should be addressed
+    193 |+    if x: # this should be addressed
+194 194 |         print(x)
+195 195 | 
+196 196 | def g(cond:bool):
+
+len_as_condition.py:200:8: PLC1802 [*] `len(x)` used as condition without comparison
+    |
+198 |     if cond:
+199 |         x = [4,5,6]
+200 |     if len(x): # this should be addressed
+    |        ^^^^^^ PLC1802
+201 |         print(x)
+202 |     del x
+    |
+    = help: Remove `len`
+
+ℹ Safe fix
+197 197 |     x = [1,2,3]
+198 198 |     if cond:
+199 199 |         x = [4,5,6]
+200     |-    if len(x): # this should be addressed
+    200 |+    if x: # this should be addressed
+201 201 |         print(x)
+202 202 |     del x
+203 203 | 
+
 len_as_condition.py:214:8: PLC1802 [*] `len(x)` used as condition without comparison
     |
 212 |     def inner(x:int):
@@ -416,6 +457,26 @@ len_as_condition.py:214:8: PLC1802 [*] `len(x)` used as condition without compar
 216 216 | 
 217 217 | def redefined():
 
+len_as_condition.py:220:8: PLC1802 [*] `len(x)` used as condition without comparison
+    |
+218 |     x = 123
+219 |     x = [1, 2, 3]
+220 |     if len(x): # this should be addressed
+    |        ^^^^^^ PLC1802
+221 |         print(x)
+    |
+    = help: Remove `len`
+
+ℹ Safe fix
+217 217 | def redefined():
+218 218 |     x = 123
+219 219 |     x = [1, 2, 3]
+220     |-    if len(x): # this should be addressed
+    220 |+    if x: # this should be addressed
+221 221 |         print(x)
+222 222 | 
+223 223 | global_seq = [1, 2, 3]
+
 len_as_condition.py:233:8: PLC1802 [*] `len(x)` used as condition without comparison
     |
 231 |     if False:
@@ -452,6 +513,8 @@ len_as_condition.py:237:6: PLC1802 [*] `len(ascii(1))` used as condition without
 237     |-bool(len(ascii(1)))
     237 |+bool(ascii(1))
 238 238 | bool(len(sorted("")))
+239 239 | 
+240 240 | # regression tests for https://github.com/astral-sh/ruff/issues/18811
 
 len_as_condition.py:238:6: PLC1802 [*] `len(sorted(""))` used as condition without comparison
     |
@@ -459,6 +522,8 @@ len_as_condition.py:238:6: PLC1802 [*] `len(sorted(""))` used as condition witho
 237 | bool(len(ascii(1)))
 238 | bool(len(sorted("")))
     |      ^^^^^^^^^^^^^^^ PLC1802
+239 |
+240 | # regression tests for https://github.com/astral-sh/ruff/issues/18811
     |
     = help: Remove `len`
 
@@ -468,3 +533,49 @@ len_as_condition.py:238:6: PLC1802 [*] `len(sorted(""))` used as condition witho
 237 237 | bool(len(ascii(1)))
 238     |-bool(len(sorted("")))
     238 |+bool(sorted(""))
+239 239 | 
+240 240 | # regression tests for https://github.com/astral-sh/ruff/issues/18811
+241 241 | fruits = []
+
+len_as_condition.py:242:3: PLC1802 [*] `len(fruits)` used as condition without comparison
+    |
+240 | # regression tests for https://github.com/astral-sh/ruff/issues/18811
+241 | fruits = []
+242 | if(len)(fruits):
+    |   ^^^^^^^^^^^^^ PLC1802
+243 |     ...
+    |
+    = help: Remove `len`
+
+ℹ Safe fix
+239 239 | 
+240 240 | # regression tests for https://github.com/astral-sh/ruff/issues/18811
+241 241 | fruits = []
+242     |-if(len)(fruits):
+    242 |+if fruits:
+243 243 |     ...
+244 244 | 
+245 245 | # regression tests for https://github.com/astral-sh/ruff/issues/18812
+
+len_as_condition.py:247:4: PLC1802 [*] `len(fruits)` used as condition without comparison
+    |
+245 |   # regression tests for https://github.com/astral-sh/ruff/issues/18812
+246 |   fruits = []
+247 |   if len(
+    |  ____^
+248 | |     fruits  # comment
+249 | | ):
+    | |_^ PLC1802
+250 |       ...
+    |
+    = help: Remove `len`
+
+ℹ Unsafe fix
+244 244 | 
+245 245 | # regression tests for https://github.com/astral-sh/ruff/issues/18812
+246 246 | fruits = []
+247     |-if len(
+248     |-    fruits  # comment
+249     |-):
+    247 |+if fruits:
+250 248 |     ...