mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 18:58:04 +00:00
[pylint
] Detect global
declarations in module scope (PLE0118
) (#17411)
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 # try.py 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 ``` I'm not sure this is the best or most elegant solution, but it was a quick fix that passed all of our tests. Test Plan -- New PLE0118 test case. [this comment]: https://github.com/astral-sh/ruff/issues/7633#issuecomment-1740424031 [load-before-global-declaration (PLE0118)]: https://docs.astral.sh/ruff/rules/load-before-global-declaration/#load-before-global-declaration-ple0118
This commit is contained in:
parent
3f84e75e20
commit
6d3b1d13d6
4 changed files with 60 additions and 19 deletions
|
@ -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
|
||||
|
|
|
@ -2067,8 +2067,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);
|
||||
}
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
||||
|
|
|
@ -1503,7 +1503,30 @@ 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.
|
||||
// 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()
|
||||
|
@ -1523,6 +1546,7 @@ impl<'a> SemanticModel<'a> {
|
|||
self.global_scope_mut().add(name, id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self.scopes[self.scope_id].set_globals_id(self.globals.push(globals));
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue