mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 22:01:18 +00:00
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`.
This commit is contained in:
parent
00e7d1ffd6
commit
a357a68fc9
3 changed files with 17 additions and 5 deletions
|
@ -141,6 +141,12 @@ def f():
|
||||||
global x
|
global x
|
||||||
def g():
|
def g():
|
||||||
nonlocal x # error: [invalid-syntax] "no binding for nonlocal `x` found"
|
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:
|
A class-scoped `x` also doesn't work:
|
||||||
|
|
|
@ -320,7 +320,7 @@ impl PlaceExprWithFlags {
|
||||||
self.flags.contains(PlaceFlags::IS_USED)
|
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 {
|
pub fn is_bound(&self) -> bool {
|
||||||
self.flags.contains(PlaceFlags::IS_BOUND)
|
self.flags.contains(PlaceFlags::IS_BOUND)
|
||||||
}
|
}
|
||||||
|
|
|
@ -4678,6 +4678,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
// This scope doesn't define this name. Keep going.
|
// This scope doesn't define this name. Keep going.
|
||||||
continue;
|
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.
|
// 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
|
// 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
|
// 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
|
// `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
|
// it also can't refer to local variables in enclosing functions that are declared
|
||||||
// `global` (also a `SyntaxError`).
|
// `global` (also a `SyntaxError`).
|
||||||
if self
|
if enclosing_place.is_marked_global() {
|
||||||
.index
|
|
||||||
.symbol_is_global_in_scope(enclosing_place_id, enclosing_scope_file_id)
|
|
||||||
{
|
|
||||||
// A "chain" of `nonlocal` statements is "broken" by a `global` statement. Stop
|
// A "chain" of `nonlocal` statements is "broken" by a `global` statement. Stop
|
||||||
// looping and report that this `nonlocal` statement is invalid.
|
// looping and report that this `nonlocal` statement is invalid.
|
||||||
break;
|
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,
|
// 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
|
// 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
|
// lead to a valid binding, the outermost one will be an error; we don't need to
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue