[flake8-return] Fix false-positive for variables used inside nested functions in RET504

crates/ruff_linter/resources/test/fixtures/flake8_return/RET504.py

@@ -410,17 +410,38 @@ def foo():
 
 # See: https://github.com/astral-sh/ruff/issues/10732
 def func(a: dict[str, int]) -> list[dict[str, int]]:
-    services: list[dict[str, int]]
     if "services" in a:
         services = a["services"]
         return services
 
 
-# See: https://github.com/astral-sh/ruff/issues/10732
-def func(a: dict[str, int]) -> list[dict[str, int]]:
-    if "services" in a:
-        services = a["services"]
-        return services
+# See: https://github.com/astral-sh/ruff/issues/14052
+def outer() -> list[object]:
+    @register
+    async def inner() -> None:
+        print(layout)
+
+    layout = [...]
+    return layout
+
+def outer() -> list[object]:
+    with open("") as f:
+        async def inner() -> None:
+            print(layout)
+
+    layout = [...]
+    return layout
+
+
+def outer() -> list[object]:
+    def inner():
+        with open("") as f:
+            async def inner_inner() -> None:
+                print(layout)
+
+    layout = [...]
+    return layout
+
 
 # See: https://github.com/astral-sh/ruff/issues/18411
 def f():

crates/ruff_linter/src/checkers/ast/analyze/bindings.rs

@@ -4,8 +4,8 @@ use crate::Fix;
 use crate::checkers::ast::Checker;
 use crate::codes::Rule;
 use crate::rules::{
-    flake8_import_conventions, flake8_pyi, flake8_pytest_style, flake8_type_checking, pyflakes,
-    pylint, pyupgrade, refurb, ruff,
+    flake8_import_conventions, flake8_pyi, flake8_pytest_style, flake8_return,
+    flake8_type_checking, pyflakes, pylint, pyupgrade, refurb, ruff,
 };
 
 /// Run lint rules over the [`Binding`]s.
@@ -25,11 +25,20 @@ pub(crate) fn bindings(checker: &Checker) {
         Rule::ForLoopWrites,
         Rule::CustomTypeVarForSelf,
         Rule::PrivateTypeParameter,
+        Rule::UnnecessaryAssign,
     ]) {
         return;
     }
 
     for (binding_id, binding) in checker.semantic.bindings.iter_enumerated() {
+        if checker.enabled(Rule::UnnecessaryAssign) {
+            if binding.kind.is_function_definition() {
+                flake8_return::rules::unnecessary_assign(
+                    checker,
+                    binding.statement(checker.semantic()).unwrap(),
+                );
+            }
+        }
         if checker.enabled(Rule::UnusedVariable) {
             if binding.kind.is_bound_exception()
                 && binding.is_unused()

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -218,7 +218,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 Rule::UnnecessaryReturnNone,
                 Rule::ImplicitReturnValue,
                 Rule::ImplicitReturn,
-                Rule::UnnecessaryAssign,
                 Rule::SuperfluousElseReturn,
                 Rule::SuperfluousElseRaise,
                 Rule::SuperfluousElseContinue,

crates/ruff_linter/src/rules/flake8_return/rules/function.rs

@@ -556,7 +556,18 @@ fn implicit_return(checker: &Checker, function_def: &ast::StmtFunctionDef, stmt:
 }
 
 /// RET504
-fn unnecessary_assign(checker: &Checker, stack: &Stack) {
+pub(crate) fn unnecessary_assign(checker: &Checker, function_stmt: &Stmt) {
+    let Stmt::FunctionDef(function_def) = function_stmt else {
+        unreachable!();
+    };
+    let Some(stack) = create_stack(checker, function_def) else {
+        return;
+    };
+
+    if !result_exists(&stack.returns) {
+        return;
+    }
+
     for (assign, return_, stmt) in &stack.assignment_return {
         // Identify, e.g., `return x`.
         let Some(value) = return_.value.as_ref() else {
@@ -600,6 +611,25 @@ fn unnecessary_assign(checker: &Checker, stack: &Stack) {
             continue;
         }
 
+        let Some(assigned_binding) = checker
+            .semantic()
+            .bindings
+            .iter()
+            .filter(|binding| binding.kind.is_assignment())
+            .find(|binding| binding.name(checker.source()) == assigned_id.as_str())
+        else {
+            continue;
+        };
+        // Check if there's any reference made to `assigned_binding` in another scope, e.g, nested
+        // functions. If there is, ignore them.
+        if assigned_binding
+            .references()
+            .map(|reference_id| checker.semantic().reference(reference_id))
+            .any(|reference| reference.scope_id() != assigned_binding.scope)
+        {
+            continue;
+        }
+
         let mut diagnostic = checker.report_diagnostic(
             UnnecessaryAssign {
                 name: assigned_id.to_string(),
@@ -682,24 +712,21 @@ fn superfluous_elif_else(checker: &Checker, stack: &Stack) {
     }
 }
 
-/// Run all checks from the `flake8-return` plugin.
-pub(crate) fn function(checker: &Checker, function_def: &ast::StmtFunctionDef) {
-    let ast::StmtFunctionDef {
-        decorator_list,
-        returns,
-        body,
-        ..
-    } = function_def;
+fn create_stack<'a>(
+    checker: &'a Checker,
+    function_def: &'a ast::StmtFunctionDef,
+) -> Option<Stack<'a>> {
+    let ast::StmtFunctionDef { body, .. } = function_def;
 
     // Find the last statement in the function.
     let Some(last_stmt) = body.last() else {
         // Skip empty functions.
-        return;
+        return None;
     };
 
     // Skip functions that consist of a single return statement.
     if body.len() == 1 && matches!(last_stmt, Stmt::Return(_)) {
-        return;
+        return None;
     }
 
     // Traverse the function body, to collect the stack.
@@ -713,9 +740,29 @@ pub(crate) fn function(checker: &Checker, function_def: &ast::StmtFunctionDef) {
 
     // Avoid false positives for generators.
     if stack.is_generator {
-        return;
+        return None;
     }
 
+    Some(stack)
+}
+
+/// Run all checks from the `flake8-return` plugin, but `RET504` which is ran
+/// after the semantic model is fully built.
+pub(crate) fn function(checker: &Checker, function_def: &ast::StmtFunctionDef) {
+    let ast::StmtFunctionDef {
+        decorator_list,
+        returns,
+        body,
+        ..
+    } = function_def;
+
+    let Some(stack) = create_stack(checker, function_def) else {
+        return;
+    };
+
+    // SAFETY: `create_stack` checks if the function has the last statement.
+    let last_stmt = body.last().unwrap();
+
     if checker.any_enabled(&[
         Rule::SuperfluousElseReturn,
         Rule::SuperfluousElseRaise,
@@ -738,10 +785,6 @@ pub(crate) fn function(checker: &Checker, function_def: &ast::StmtFunctionDef) {
         if checker.enabled(Rule::ImplicitReturn) {
             implicit_return(checker, function_def, last_stmt);
         }
-
-        if checker.enabled(Rule::UnnecessaryAssign) {
-            unnecessary_assign(checker, &stack);
-        }
     } else {
         if checker.enabled(Rule::UnnecessaryReturnNone) {
             // Skip functions that have a return annotation that is not `None`.

crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET504_RET504.py.snap

@@ -241,65 +241,63 @@ RET504.py:400:12: RET504 [*] Unnecessary assignment to `y` before `return` state
 402 401 | 
 403 402 | def foo():
 
-RET504.py:423:16: RET504 [*] Unnecessary assignment to `services` before `return` statement
+RET504.py:415:16: RET504 [*] Unnecessary assignment to `services` before `return` statement
     |
-421 |     if "services" in a:
-422 |         services = a["services"]
-423 |         return services
+413 |     if "services" in a:
+414 |         services = a["services"]
+415 |         return services
     |                ^^^^^^^^ RET504
-424 |
-425 | # See: https://github.com/astral-sh/ruff/issues/18411
     |
     = help: Remove unnecessary assignment
 
 ℹ Unsafe fix
-419 419 | # See: https://github.com/astral-sh/ruff/issues/10732
-420 420 | def func(a: dict[str, int]) -> list[dict[str, int]]:
-421 421 |     if "services" in a:
-422     |-        services = a["services"]
-423     |-        return services
-    422 |+        return a["services"]
-424 423 | 
-425 424 | # See: https://github.com/astral-sh/ruff/issues/18411
-426 425 | def f():
+411 411 | # See: https://github.com/astral-sh/ruff/issues/10732
+412 412 | def func(a: dict[str, int]) -> list[dict[str, int]]:
+413 413 |     if "services" in a:
+414     |-        services = a["services"]
+415     |-        return services
+    414 |+        return a["services"]
+416 415 | 
+417 416 | 
+418 417 | # See: https://github.com/astral-sh/ruff/issues/14052
 
-RET504.py:429:12: RET504 [*] Unnecessary assignment to `x` before `return` statement
+RET504.py:450:12: RET504 [*] Unnecessary assignment to `x` before `return` statement
     |
-427 |     (#=
-428 |     x) = 1
-429 |     return x
+448 |     (#=
+449 |     x) = 1
+450 |     return x
     |            ^ RET504
-430 |
-431 | def f():
+451 |
+452 | def f():
     |
     = help: Remove unnecessary assignment
 
 ℹ Unsafe fix
-424 424 | 
-425 425 | # See: https://github.com/astral-sh/ruff/issues/18411
-426 426 | def f():
-427     |-    (#=
-428     |-    x) = 1
-429     |-    return x
-    427 |+    return 1
-430 428 | 
-431 429 | def f():
-432 430 |     x = (1
+445 445 | 
+446 446 | # See: https://github.com/astral-sh/ruff/issues/18411
+447 447 | def f():
+448     |-    (#=
+449     |-    x) = 1
+450     |-    return x
+    448 |+    return 1
+451 449 | 
+452 450 | def f():
+453 451 |     x = (1
 
-RET504.py:434:12: RET504 [*] Unnecessary assignment to `x` before `return` statement
+RET504.py:455:12: RET504 [*] Unnecessary assignment to `x` before `return` statement
     |
-432 |     x = (1
-433 |               )
-434 |     return x
+453 |     x = (1
+454 |               )
+455 |     return x
     |            ^ RET504
     |
     = help: Remove unnecessary assignment
 
 ℹ Unsafe fix
-429 429 |     return x
-430 430 | 
-431 431 | def f():
-432     |-    x = (1
-    432 |+    return (1
-433 433 |               )
-434     |-    return x
+450 450 |     return x
+451 451 | 
+452 452 | def f():
+453     |-    x = (1
+    453 |+    return (1
+454 454 |               )
+455     |-    return x