astral-sh · LaBatata101 · Apr 27, 2025 · Apr 27, 2025 · May 3, 2025 · May 3, 2025
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_return/RET504.py b/crates/ruff_linter/resources/test/fixtures/flake8_return/RET504.py
@@ -421,3 +421,30 @@ 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
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs
@@ -4,8 +4,8 @@ use ruff_text_size::Ranged;
 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()

diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs
@@ -214,7 +214,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 Rule::UnnecessaryReturnNone,
                 Rule::ImplicitReturnValue,
                 Rule::ImplicitReturn,
-                Rule::UnnecessaryAssign,
                 Rule::SuperfluousElseReturn,
                 Rule::SuperfluousElseRaise,
                 Rule::SuperfluousElseContinue,

diff --git a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs
@@ -562,7 +562,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 {
@@ -606,6 +617,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 = Diagnostic::new(
             UnnecessaryAssign {
                 name: assigned_id.to_string(),
@@ -754,24 +784,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.
@@ -785,9 +812,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,
@@ -810,10 +857,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`.

diff --git a/...s/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET504_RET504.py.snap b/...s/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET504_RET504.py.snap
@@ -1,6 +1,5 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_return/mod.rs
-snapshot_kind: text
 ---
 RET504.py:6:12: RET504 [*] Unnecessary assignment to `a` before `return` statement
   |
@@ -248,6 +247,8 @@ RET504.py:423:16: RET504 [*] Unnecessary assignment to `services` before `return
 422 |         services = a["services"]
 423 |         return services
     |                ^^^^^^^^ RET504
+424 |
+425 | # See: https://github.com/astral-sh/ruff/issues/14052
     |
     = help: Remove unnecessary assignment
 
@@ -258,3 +259,6 @@ RET504.py:423:16: RET504 [*] Unnecessary assignment to `services` before `return
 422     |-        services = a["services"]
 423     |-        return services
     422 |+        return a["services"]
+424 423 | 
+425 424 | # See: https://github.com/astral-sh/ruff/issues/14052
+426 425 | def outer() -> list[object]: