Skip to content

Commit 5140fa3

Browse files
committed
[pylint] Detect global declarations in module scope (PLE0118)
Summary -- While going through the syntax errors in [this comment], I was surprised to see the error `name 'x' is assigned to before global declaration`, which corresponds to [load-before-global-declaration (PLE0118)] and has also been reimplemented as a syntax error (#17135). However, it looks like neither of the implementations consider `global` declarations in the top-level module scope, which is a syntax error in CPython: ```python x = None global x ``` ```shell > python -m compileall -f try.py Compiling 'try.py'... *** File "try.py", line 2 global x ^^^^^^^^ SyntaxError: name 'x' is assigned to before global declaration ``` Test Plan -- New PLE0118 test case. We should also check the codspeed results carefully here. `Globals::from_body` visits all of the statements in `body`, so this might be a really bad idea, but it looked like the simplest fix. [this comment]: #7633 (comment) [load-before-global-declaration (PLE0118)]: https://docs.astral.sh/ruff/rules/load-before-global-declaration/#load-before-global-declaration-ple0118
1 parent 312a487 commit 5140fa3

4 files changed

Lines changed: 39 additions & 18 deletions

File tree

crates/ruff_linter/resources/test/fixtures/pylint/load_before_global_declaration.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,8 @@ def f():
156156
def f():
157157
global x
158158
print(f"{x=}")
159+
160+
161+
# surprisingly still an error, global in module scope
162+
x = None
163+
global x

crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,6 +1971,10 @@ impl<'a> Visitor<'a> for Checker<'a> {
19711971
// Step 4: Analysis
19721972
analyze::suite(body, self);
19731973

1974+
if let Some(globals) = Globals::from_body(body) {
1975+
self.semantic.set_globals(globals);
1976+
}
1977+
19741978
// Step 2: Traversal
19751979
for stmt in body {
19761980
self.visit_stmt(stmt);

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0118_load_before_global_declaration.py.snap

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,11 @@ load_before_global_declaration.py:113:14: PLE0118 Name `x` is used prior to glob
123123
| ^ PLE0118
124124
114 | global x
125125
|
126+
127+
load_before_global_declaration.py:162:1: PLE0118 Name `x` is used prior to global declaration on line 163
128+
|
129+
161 | # surprisingly still an error, global in module scope
130+
162 | x = None
131+
| ^ PLE0118
132+
163 | global x
133+
|

crates/ruff_python_semantic/src/model.rs

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,24 +1503,28 @@ impl<'a> SemanticModel<'a> {
15031503

15041504
/// Set the [`Globals`] for the current [`Scope`].
15051505
pub fn set_globals(&mut self, globals: Globals<'a>) {
1506-
// If any global bindings don't already exist in the global scope, add them.
1507-
for (name, range) in globals.iter() {
1508-
if self
1509-
.global_scope()
1510-
.get(name)
1511-
.is_none_or(|binding_id| self.bindings[binding_id].is_unbound())
1512-
{
1513-
let id = self.bindings.push(Binding {
1514-
kind: BindingKind::Assignment,
1515-
range: *range,
1516-
references: Vec::new(),
1517-
scope: ScopeId::global(),
1518-
source: self.node_id,
1519-
context: self.execution_context(),
1520-
exceptions: self.exceptions(),
1521-
flags: BindingFlags::empty(),
1522-
});
1523-
self.global_scope_mut().add(name, id);
1506+
// If any global bindings don't already exist in the global scope, add them, unless we are
1507+
// also in the global scope, in which case avoid adding them to flag
1508+
// `load-before-global-declaration` (PLE0118).
1509+
if !self.at_top_level() {
1510+
for (name, range) in globals.iter() {
1511+
if self
1512+
.global_scope()
1513+
.get(name)
1514+
.is_none_or(|binding_id| self.bindings[binding_id].is_unbound())
1515+
{
1516+
let id = self.bindings.push(Binding {
1517+
kind: BindingKind::Assignment,
1518+
range: *range,
1519+
references: Vec::new(),
1520+
scope: ScopeId::global(),
1521+
source: self.node_id,
1522+
context: self.execution_context(),
1523+
exceptions: self.exceptions(),
1524+
flags: BindingFlags::empty(),
1525+
});
1526+
self.global_scope_mut().add(name, id);
1527+
}
15241528
}
15251529
}
15261530

0 commit comments

Comments
 (0)