diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/load_before_global_declaration.py b/crates/ruff_linter/resources/test/fixtures/pylint/load_before_global_declaration.py index 5ca672e1a1a43..fdbe9cc9b8760 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/load_before_global_declaration.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/load_before_global_declaration.py @@ -156,3 +156,8 @@ def f(): def f(): global x print(f"{x=}") + + +# surprisingly still an error, global in module scope +x = None +global x diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index ebff124383a04..cb327d974e382 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -2057,8 +2057,12 @@ impl<'a> Visitor<'a> for Checker<'a> { } impl<'a> Checker<'a> { - /// Visit a [`Module`]. Returns `true` if the module contains a module-level docstring. + /// Visit a [`Module`]. fn visit_module(&mut self, python_ast: &'a Suite) { + // Extract any global bindings from the module body. + if let Some(globals) = Globals::from_body(python_ast) { + self.semantic.set_globals(globals); + } analyze::module(python_ast, self); } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0118_load_before_global_declaration.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0118_load_before_global_declaration.py.snap index 5d1994780df2a..adabada987380 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0118_load_before_global_declaration.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0118_load_before_global_declaration.py.snap @@ -123,3 +123,11 @@ load_before_global_declaration.py:113:14: PLE0118 Name `x` is used prior to glob | ^ PLE0118 114 | global x | + +load_before_global_declaration.py:162:1: PLE0118 Name `x` is used prior to global declaration on line 163 + | +161 | # surprisingly still an error, global in module scope +162 | x = None + | ^ PLE0118 +163 | global x + | diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index d24060b7c41ca..70c26cd615898 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1503,24 +1503,48 @@ impl<'a> SemanticModel<'a> { /// Set the [`Globals`] for the current [`Scope`]. pub fn set_globals(&mut self, globals: Globals<'a>) { - // If any global bindings don't already exist in the global scope, add them. - for (name, range) in globals.iter() { - if self - .global_scope() - .get(name) - .is_none_or(|binding_id| self.bindings[binding_id].is_unbound()) - { - let id = self.bindings.push(Binding { - kind: BindingKind::Assignment, - range: *range, - references: Vec::new(), - scope: ScopeId::global(), - source: self.node_id, - context: self.execution_context(), - exceptions: self.exceptions(), - flags: BindingFlags::empty(), - }); - self.global_scope_mut().add(name, id); + // If any global bindings don't already exist in the global scope, add them, unless we are + // also in the global scope, where we don't want these to count as definitions for rules + // like `undefined-name` (F821). For example, adding bindings in the top-level scope causes + // a false negative in cases like this: + // + // ```python + // global x + // + // def f(): + // print(x) # F821 false negative + // ``` + // + // On the other hand, failing to add bindings in non-top-level scopes causes false + // positives: + // + // ```python + // def f(): + // global foo + // import foo + // + // def g(): + // foo.is_used() # F821 false positive + // ``` + if !self.at_top_level() { + for (name, range) in globals.iter() { + if self + .global_scope() + .get(name) + .is_none_or(|binding_id| self.bindings[binding_id].is_unbound()) + { + let id = self.bindings.push(Binding { + kind: BindingKind::Assignment, + range: *range, + references: Vec::new(), + scope: ScopeId::global(), + source: self.node_id, + context: self.execution_context(), + exceptions: self.exceptions(), + flags: BindingFlags::empty(), + }); + self.global_scope_mut().add(name, id); + } } }