From a357a68fc96999c163f77302f6e017d01f399715 Mon Sep 17 00:00:00 2001
From: Jack O'Connor <oconnor663@gmail.com>
Date: Mon, 14 Jul 2025 14:40:58 -0700
Subject: [PATCH] distinguish references from definitions in `infer_nonlocal`

The initial implementation of `infer_nonlocal` landed in
https://github.com/astral-sh/ruff/pull/19112 fails to report an error
for this example:

```py
x = 1
def f():
    # This is only a usage of `x`, not a definition. It shouldn't be
    # enough to make the `nonlocal` statement below allowed.
    print(x)
    def g():
        nonlocal x
```

Fix this by continuing to walk enclosing scopes when the place we've
found isn't bound, declared, or `nonlocal`.
---
 .../resources/mdtest/scopes/nonlocal.md            |  6 ++++++
 .../ty_python_semantic/src/semantic_index/place.rs |  2 +-
 crates/ty_python_semantic/src/types/infer.rs       | 14 ++++++++++----
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md b/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md
index a30bfa219a..5456d962ea 100644
--- a/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md
+++ b/crates/ty_python_semantic/resources/mdtest/scopes/nonlocal.md
@@ -141,6 +141,12 @@ def f():
     global x
     def g():
         nonlocal x  # error: [invalid-syntax] "no binding for nonlocal `x` found"
+
+def f():
+    # A *use* of `x` in an enclosing scope isn't good enough. There needs to be a binding.
+    print(x)
+    def g():
+        nonlocal x  # error: [invalid-syntax] "no binding for nonlocal `x` found"
 ```
 
 A class-scoped `x` also doesn't work:
diff --git a/crates/ty_python_semantic/src/semantic_index/place.rs b/crates/ty_python_semantic/src/semantic_index/place.rs
index 39c6dcc7cf..f8962a6183 100644
--- a/crates/ty_python_semantic/src/semantic_index/place.rs
+++ b/crates/ty_python_semantic/src/semantic_index/place.rs
@@ -320,7 +320,7 @@ impl PlaceExprWithFlags {
         self.flags.contains(PlaceFlags::IS_USED)
     }
 
-    /// Is the place defined in its containing scope?
+    /// Is the place given a value in its containing scope?
     pub fn is_bound(&self) -> bool {
         self.flags.contains(PlaceFlags::IS_BOUND)
     }
diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs
index 94066916e9..c32cd62c97 100644
--- a/crates/ty_python_semantic/src/types/infer.rs
+++ b/crates/ty_python_semantic/src/types/infer.rs
@@ -4678,6 +4678,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                     // This scope doesn't define this name. Keep going.
                     continue;
                 };
+                let enclosing_place = enclosing_place_table.place_expr(enclosing_place_id);
                 // We've found a definition for this name in an enclosing function-like scope.
                 // Either this definition is the valid place this name refers to, or else we'll
                 // emit a syntax error. Either way, we won't walk any more enclosing scopes. Note
@@ -4688,14 +4689,19 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                 // `nonlocal` keyword can't refer to global variables (that's a `SyntaxError`), and
                 // it also can't refer to local variables in enclosing functions that are declared
                 // `global` (also a `SyntaxError`).
-                if self
-                    .index
-                    .symbol_is_global_in_scope(enclosing_place_id, enclosing_scope_file_id)
-                {
+                if enclosing_place.is_marked_global() {
                     // A "chain" of `nonlocal` statements is "broken" by a `global` statement. Stop
                     // looping and report that this `nonlocal` statement is invalid.
                     break;
                 }
+                if !enclosing_place.is_bound()
+                    && !enclosing_place.is_declared()
+                    && !enclosing_place.is_marked_nonlocal()
+                {
+                    debug_assert!(enclosing_place.is_used());
+                    // The name is only referenced here, not defined. Keep going.
+                    continue;
+                }
                 // We found a definition. We've checked that the name isn't `global` in this scope,
                 // but it's ok if it's `nonlocal`. If a "chain" of `nonlocal` statements fails to
                 // lead to a valid binding, the outermost one will be an error; we don't need to