Use "manual" fixability for E731 in shadowed context (#5430)

## Summary

This PR makes E731 a "manual" fix in one other context: when the lambda
is shadowing another variable in the scope. Function declarations (with
shadowing) cause issues for type checkers, and so rewriting an
annotation, e.g., in branches of an `if` statement can lead to failures.

Closes #5421.

crates/ruff/resources/test/fixtures/pycodestyle/E731.py

@@ -1,51 +1,135 @@
-#: E731
-f = lambda x: 2 * x
-#: E731
-f = lambda x: 2 * x
-#: E731
-while False:
-    this = lambda y, z: 2 * x
-#: E731
-f = lambda: (yield 1)
-#: E731
-f = lambda: (yield from g())
-#: E731
-class F:
+def scope():
+    # E731
     f = lambda x: 2 * x
 
 
-f = object()
-f.method = lambda: "Method"
-f = {}
-f["a"] = lambda x: x**2
-f = []
-f.append(lambda x: x**2)
-f = g = lambda x: x**2
-lambda: "no-op"
+def scope():
+    # E731
+    f = lambda x: 2 * x
+
+
+def scope():
+    # E731
+    while False:
+        this = lambda y, z: 2 * x
+
+
+def scope():
+    # E731
+    f = lambda: (yield 1)
+
+
+def scope():
+    # E731
+    f = lambda: (yield from g())
+
+
+def scope():
+    # OK
+    f = object()
+    f.method = lambda: "Method"
+
+
+def scope():
+    # OK
+    f = {}
+    f["a"] = lambda x: x**2
+
+
+def scope():
+    # OK
+    f = []
+    f.append(lambda x: x**2)
+
+
+def scope():
+    # OK
+    f = g = lambda x: x**2
+
+
+def scope():
+    # OK
+    lambda: "no-op"
+
+
+class Scope:
+    # E731
+    f = lambda x: 2 * x
+
+
+class Scope:
+    from typing import Callable
+
+    # E731
+    f: Callable[[int], int] = lambda x: 2 * x
 
-# Annotated
-from typing import Callable, ParamSpec
 
-P = ParamSpec("P")
+def scope():
+    # E731
+    from typing import Callable
+
+    x: Callable[[int], int]
+    if True:
+        x = lambda: 1
+    else:
+        x = lambda: 2
+    return x
+
+
+def scope():
+    # E731
+
+    from typing import Callable, ParamSpec
+
+    # ParamSpec cannot be used in this context, so do not preserve the annotation.
+    P = ParamSpec("P")
+    f: Callable[P, int] = lambda *args: len(args)
+
+
+def scope():
+    # E731
+
+    from typing import Callable
+
+    f: Callable[[], None] = lambda: None
+
+
+def scope():
+    # E731
+
+    from typing import Callable
+
+    f: Callable[..., None] = lambda a, b: None
+
+
+def scope():
+    # E731
+
+    from typing import Callable
+
+    f: Callable[[int], int] = lambda x: 2 * x
 
-# ParamSpec cannot be used in this context, so do not preserve the annotation.
-f: Callable[P, int] = lambda *args: len(args)
-f: Callable[[], None] = lambda: None
-f: Callable[..., None] = lambda a, b: None
-f: Callable[[int], int] = lambda x: 2 * x
 
 # Let's use the `Callable` type from `collections.abc` instead.
-from collections.abc import Callable
+def scope():
+    # E731
+
+    from collections.abc import Callable
+
+    f: Callable[[str, int], str] = lambda a, b: a * b
+
+
+def scope():
+    # E731
+
+    from collections.abc import Callable
 
-f: Callable[[str, int], str] = lambda a, b: a * b
-f: Callable[[str, int], tuple[str, int]] = lambda a, b: (a, b)
-f: Callable[[str, int, list[str]], list[str]] = lambda a, b, /, c: [*c, a * b]
+    f: Callable[[str, int], tuple[str, int]] = lambda a, b: (a, b)
 
 
-# Override `Callable`
-class Callable:
-    pass
+def scope():
+    # E731
 
+    from collections.abc import Callable
 
-# Do not copy the annotation from here on out.
-f: Callable[[str, int], str] = lambda a, b: a * b
+    f: Callable[[str, int, list[str]], list[str]] = lambda a, b, /, c: [*c, a * b]

crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs

@@ -63,57 +63,69 @@ pub(crate) fn lambda_assignment(
     annotation: Option<&Expr>,
     stmt: &Stmt,
 ) {
-    if let Expr::Name(ast::ExprName { id, .. }) = target {
-        if let Expr::Lambda(ast::ExprLambda { args, body, .. }) = value {
-            let mut diagnostic = Diagnostic::new(
-                LambdaAssignment {
-                    name: id.to_string(),
-                },
-                stmt.range(),
-            );
-
-            // If the assignment is in a class body, it might not be safe
-            // to replace it because the assignment might be
-            // carrying a type annotation that will be used by some
-            // package like dataclasses, which wouldn't consider the
-            // rewritten function definition to be equivalent.
-            // See https://github.com/astral-sh/ruff/issues/3046
-            if checker.patch(diagnostic.kind.rule())
-                && !checker.semantic().scope().kind.is_class()
-                && !has_leading_content(stmt.start(), checker.locator)
-                && !has_trailing_content(stmt.end(), checker.locator)
+    let Expr::Name(ast::ExprName { id, .. }) = target else {
+        return;
+    };
+
+    let Expr::Lambda(ast::ExprLambda { args, body, .. }) = value else {
+        return;
+    };
+
+    let mut diagnostic = Diagnostic::new(
+        LambdaAssignment {
+            name: id.to_string(),
+        },
+        stmt.range(),
+    );
+
+    if checker.patch(diagnostic.kind.rule()) {
+        if !has_leading_content(stmt.start(), checker.locator)
+            && !has_trailing_content(stmt.end(), checker.locator)
+        {
+            let first_line = checker.locator.line(stmt.start());
+            let indentation = leading_indentation(first_line);
+            let mut indented = String::new();
+            for (idx, line) in function(
+                id,
+                args,
+                body,
+                annotation,
+                checker.semantic(),
+                checker.generator(),
+            )
+            .universal_newlines()
+            .enumerate()
             {
-                let first_line = checker.locator.line(stmt.start());
-                let indentation = leading_indentation(first_line);
-                let mut indented = String::new();
-                for (idx, line) in function(
-                    id,
-                    args,
-                    body,
-                    annotation,
-                    checker.semantic(),
-                    checker.generator(),
-                )
-                .universal_newlines()
-                .enumerate()
-                {
-                    if idx == 0 {
-                        indented.push_str(&line);
-                    } else {
-                        indented.push_str(checker.stylist.line_ending().as_str());
-                        indented.push_str(indentation);
-                        indented.push_str(&line);
-                    }
+                if idx == 0 {
+                    indented.push_str(&line);
+                } else {
+                    indented.push_str(checker.stylist.line_ending().as_str());
+                    indented.push_str(indentation);
+                    indented.push_str(&line);
                 }
+            }
+
+            // If the assignment is in a class body, it might not be safe to replace it because the
+            // assignment might be carrying a type annotation that will be used by some package like
+            // dataclasses, which wouldn't consider the rewritten function definition to be
+            // equivalent. Similarly, if the lambda is shadowing a variable in the current scope,
+            // rewriting it as a function declaration may break type-checking.
+            // See: https://github.com/astral-sh/ruff/issues/3046
+            // See: https://github.com/astral-sh/ruff/issues/5421
+            if (annotation.is_some() && checker.semantic().scope().kind.is_class())
+                || checker.semantic().scope().has(id)
+            {
+                diagnostic.set_fix(Fix::manual(Edit::range_replacement(indented, stmt.range())));
+            } else {
                 diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
                     indented,
                     stmt.range(),
                 )));
             }
-
-            checker.diagnostics.push(diagnostic);
         }
     }
+
+    checker.diagnostics.push(diagnostic);
 }
 
 /// Extract the argument types and return type from a `Callable` annotation.