diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index 1df9dcabff..53bb0c4cf0 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -92,6 +92,7 @@ use crate::binding::pytest::PytestBindingInfo; use crate::binding::pytest::is_pytest_fixture_function; use crate::binding::scope::Exportable; use crate::binding::scope::FlowStyle; +use crate::binding::scope::MutableCaptureError; use crate::binding::scope::NameReadInfo; use crate::binding::scope::ScopeTrace; use crate::binding::scope::Scopes; @@ -1454,9 +1455,14 @@ impl<'a> BindingsBuilder<'a> { Binding::Forward(idx) } Err(error) => { - let should_suppress = matches!(kind, MutableCaptureKind::Nonlocal) + let should_suppress = (matches!(kind, MutableCaptureKind::Nonlocal) && self.scopes.in_module_or_class_top_level() - && !self.scopes.in_class_body(); + && !self.scopes.in_class_body()) + || (matches!(kind, MutableCaptureKind::Global) + && matches!(error, MutableCaptureError::NotFound) + && self + .scopes + .global_capture_self_defined(Hashed::new(&name.id))); if !should_suppress { self.error(name.range, ErrorKind::UnknownName, error.message(name)); } diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 75347bc767..379d9b39b7 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -265,6 +265,9 @@ enum StaticStyle { struct MutableCapture { kind: MutableCaptureKind, original: Result, MutableCaptureError>, + /// Does the declaring scope itself give the name a value (e.g. `global x; x = 1`)? + /// Such a capture creates the outer-scope name instead of requiring one to exist. + has_value_definition: bool, } impl MutableCapture { @@ -333,6 +336,7 @@ impl StaticStyle { Self::MutableCapture(MutableCapture { kind: *kind, original, + has_value_definition: definition.has_value_definition, }) } DefinitionStyle::Annotated(.., ann) => { @@ -2061,6 +2065,26 @@ impl Scopes { None } + /// Does a `global` declaration of `name` in the current function/method scope + /// come with a value definition in that same scope (e.g. `global x; x = 1`)? + /// Such a declaration legally creates the module-level name at runtime, so it + /// must not be reported as referencing a missing outer definition. + pub fn global_capture_self_defined(&self, name: Hashed<&Name>) -> bool { + let current = self.current(); + matches!(current.kind, ScopeKind::Function(_) | ScopeKind::Method(_)) + && matches!( + current.stat.0.get_hashed(name), + Some(StaticInfo { + style: StaticStyle::MutableCapture(MutableCapture { + kind: MutableCaptureKind::Global, + has_value_definition: true, + .. + }), + .. + }) + ) + } + /// Check if a name has a nonlocal binding in an enclosing scope. pub fn has_nonlocal_binding(&self, name: &str) -> bool { let name_obj = Name::new(name); diff --git a/pyrefly/lib/export/definitions.rs b/pyrefly/lib/export/definitions.rs index f199228a6f..2ca38d5699 100644 --- a/pyrefly/lib/export/definitions.rs +++ b/pyrefly/lib/export/definitions.rs @@ -114,6 +114,25 @@ pub struct Definition { /// True while every definition site is inside an `if __name__ == "__main__":` body. /// Such names resolve in-module but are not importable, so the export surface excludes them. pub main_guard_only: bool, + /// True if any definition site assigns or imports a value, as opposed to merely + /// declaring the name (`global`/`nonlocal`) or deleting it. Only meaningful when + /// `style` is `MutableCapture`: a capture whose own scope defines a value + /// (`global x; x = 1`) legally creates the outer-scope name, so it needs no + /// pre-existing outer definition. + pub has_value_definition: bool, +} + +/// Does this definition style give the name a value in the current scope? +/// `global`/`nonlocal` declarations and `del` do not; implicit globals are +/// injected module-level, not defined here. +fn is_value_definition(style: &DefinitionStyle) -> bool { + !matches!( + style, + DefinitionStyle::MutableCapture(..) + | DefinitionStyle::Delete + | DefinitionStyle::ImplicitGlobal + | DefinitionStyle::ImportInvalidRelative + ) } impl Definition { @@ -126,6 +145,7 @@ impl Definition { fn merge(&mut self, other: DefinitionStyle, range: TextRange, in_main_guard: bool) { self.main_guard_only &= in_main_guard; + self.has_value_definition |= is_value_definition(&other); // To ensure binding code cannot produce invalid lookups, we ensure that // `self.style` and `self.range` always match. if other < self.style { @@ -351,6 +371,7 @@ impl Definitions { docstring_range: None, last_range: TextRange::default(), main_guard_only: false, + has_value_definition: false, }, ); } @@ -439,6 +460,7 @@ impl DefinitionsBuilder { return; } let in_main_guard = self.in_main_guard; + let has_value_definition = is_value_definition(&style); match self.inner.definitions.entry(x.clone()) { Entry::Occupied(mut e) => { e.get_mut().merge(style, range, in_main_guard); @@ -451,6 +473,7 @@ impl DefinitionsBuilder { docstring_range: body.and_then(Docstring::range_from_stmts), last_range: range, main_guard_only: in_main_guard, + has_value_definition, }); } } diff --git a/pyrefly/lib/test/scope.rs b/pyrefly/lib/test/scope.rs index 536acf9d52..1cc8a0541e 100644 --- a/pyrefly/lib/test/scope.rs +++ b/pyrefly/lib/test/scope.rs @@ -397,6 +397,79 @@ global a # E: Could not find name `a` "#, ); +testcase!( + test_global_assign_without_module_definition, + r#" +def set_workload_id(value: str) -> None: + global workload_id + workload_id = value + + +set_workload_id("test") +assert globals()["workload_id"] == "test" +"#, +); + +testcase!( + test_global_read_without_definition_still_errors, + r#" +def f() -> None: + global a # E: Could not find name `a` + print(a) +"#, +); + +testcase!( + test_global_assign_nested_in_if, + r#" +def f(cond: bool) -> None: + global a + if cond: + a = 1 + + +def g(cond: bool) -> None: + if cond: + global b + b = 2 +"#, +); + +testcase!( + test_global_del_does_not_define, + r#" +def f() -> None: + global z # E: Could not find name `z` + del z +"#, +); + +testcase!( + test_global_assign_before_declaration_still_errors, + r#" +def f() -> None: + x = 1 + global x # E: `x` was assigned in the current scope before the global declaration +"#, +); + +testcase!( + test_global_assign_at_module_top_level_still_errors, + r#" +global a # E: Could not find name `a` +a = 1 +"#, +); + +testcase!( + test_global_assign_in_class_body_still_errors, + r#" +class C: + global cx # E: Could not find name `cx` + cx = 1 +"#, +); + testcase!( test_nonlocal_not_found, r#"