mirror of https://github.com/astral-sh/ruff
Move undefined-local into a post-model-building pass (#5928)
## Summary Similar to #5852 and a bunch of related PRs -- trying to move rules that rely on point-in-time semantic analysis to _after_ the semantic model building.
This commit is contained in:
parent
2cde9b8aa6
commit
bcec2f0c4c
|
|
@ -25,3 +25,17 @@ def dec(x):
|
||||||
def f():
|
def f():
|
||||||
dec = 1
|
dec = 1
|
||||||
return dec
|
return dec
|
||||||
|
|
||||||
|
|
||||||
|
class Class:
|
||||||
|
def f(self):
|
||||||
|
print(my_var)
|
||||||
|
my_var = 1
|
||||||
|
|
||||||
|
|
||||||
|
class Class:
|
||||||
|
my_var = 0
|
||||||
|
|
||||||
|
def f(self):
|
||||||
|
print(my_var)
|
||||||
|
my_var = 1
|
||||||
|
|
|
||||||
|
|
@ -2359,9 +2359,6 @@ where
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
ExprContext::Store => {
|
ExprContext::Store => {
|
||||||
if self.enabled(Rule::UndefinedLocal) {
|
|
||||||
pyflakes::rules::undefined_local(self, id);
|
|
||||||
}
|
|
||||||
if self.enabled(Rule::NonLowercaseVariableInFunction) {
|
if self.enabled(Rule::NonLowercaseVariableInFunction) {
|
||||||
if self.semantic.scope().kind.is_any_function() {
|
if self.semantic.scope().kind.is_any_function() {
|
||||||
// Ignore globals.
|
// Ignore globals.
|
||||||
|
|
@ -4383,16 +4380,13 @@ impl<'a> Checker<'a> {
|
||||||
.scopes
|
.scopes
|
||||||
.ancestors(scope_id)
|
.ancestors(scope_id)
|
||||||
.skip(1)
|
.skip(1)
|
||||||
|
.filter(|scope| scope.kind.is_function() || scope.kind.is_module())
|
||||||
.find_map(|scope| scope.get(name))
|
.find_map(|scope| scope.get(name))
|
||||||
{
|
{
|
||||||
// Otherwise, if there's an existing binding in a parent scope, mark it as shadowed.
|
// Otherwise, if there's an existing binding in a parent scope, mark it as shadowed.
|
||||||
let binding = self.semantic.binding(binding_id);
|
self.semantic
|
||||||
let shadowed = self.semantic.binding(shadowed_id);
|
.shadowed_bindings
|
||||||
if binding.redefines(shadowed) {
|
.insert(binding_id, shadowed_id);
|
||||||
self.semantic
|
|
||||||
.shadowed_bindings
|
|
||||||
.insert(binding_id, shadowed_id);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add the binding to the scope.
|
// Add the binding to the scope.
|
||||||
|
|
@ -4878,6 +4872,7 @@ impl<'a> Checker<'a> {
|
||||||
Rule::TypingOnlyStandardLibraryImport,
|
Rule::TypingOnlyStandardLibraryImport,
|
||||||
Rule::TypingOnlyThirdPartyImport,
|
Rule::TypingOnlyThirdPartyImport,
|
||||||
Rule::UnusedImport,
|
Rule::UnusedImport,
|
||||||
|
Rule::UndefinedLocal,
|
||||||
]) {
|
]) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -4924,6 +4919,11 @@ impl<'a> Checker<'a> {
|
||||||
for scope_id in self.deferred.scopes.iter().rev().copied() {
|
for scope_id in self.deferred.scopes.iter().rev().copied() {
|
||||||
let scope = &self.semantic.scopes[scope_id];
|
let scope = &self.semantic.scopes[scope_id];
|
||||||
|
|
||||||
|
// F823
|
||||||
|
if self.enabled(Rule::UndefinedLocal) {
|
||||||
|
pyflakes::rules::undefined_local(self, scope_id, scope, &mut diagnostics);
|
||||||
|
}
|
||||||
|
|
||||||
// PLW0602
|
// PLW0602
|
||||||
if self.enabled(Rule::GlobalVariableNotAssigned) {
|
if self.enabled(Rule::GlobalVariableNotAssigned) {
|
||||||
for (name, binding_id) in scope.bindings() {
|
for (name, binding_id) in scope.bindings() {
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ use std::string::ToString;
|
||||||
|
|
||||||
use ruff_diagnostics::{Diagnostic, Violation};
|
use ruff_diagnostics::{Diagnostic, Violation};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_semantic::{Scope, ScopeId};
|
||||||
|
|
||||||
use crate::checkers::ast::Checker;
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
|
@ -32,7 +33,7 @@ use crate::checkers::ast::Checker;
|
||||||
/// ```
|
/// ```
|
||||||
#[violation]
|
#[violation]
|
||||||
pub struct UndefinedLocal {
|
pub struct UndefinedLocal {
|
||||||
name: String,
|
pub name: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Violation for UndefinedLocal {
|
impl Violation for UndefinedLocal {
|
||||||
|
|
@ -44,55 +45,35 @@ impl Violation for UndefinedLocal {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// F823
|
/// F823
|
||||||
pub(crate) fn undefined_local(checker: &mut Checker, name: &str) {
|
pub(crate) fn undefined_local(
|
||||||
// If the name hasn't already been defined in the current scope...
|
checker: &Checker,
|
||||||
let current = checker.semantic().scope();
|
scope_id: ScopeId,
|
||||||
if !current.kind.is_any_function() || current.has(name) {
|
scope: &Scope,
|
||||||
return;
|
diagnostics: &mut Vec<Diagnostic>,
|
||||||
}
|
) {
|
||||||
|
if scope.kind.is_any_function() {
|
||||||
let Some(parent) = current.parent else {
|
for (name, binding_id) in scope.bindings() {
|
||||||
return;
|
// If the variable shadows a binding in a parent scope...
|
||||||
};
|
if let Some(shadowed_id) = checker.semantic().shadowed_binding(binding_id) {
|
||||||
|
let shadowed = checker.semantic().binding(shadowed_id);
|
||||||
// For every function and module scope above us...
|
// And that binding was referenced in the current scope...
|
||||||
let local_access = checker
|
if let Some(range) = shadowed.references().find_map(|reference_id| {
|
||||||
.semantic()
|
|
||||||
.scopes
|
|
||||||
.ancestors(parent)
|
|
||||||
.find_map(|scope| {
|
|
||||||
if !(scope.kind.is_any_function() || scope.kind.is_module()) {
|
|
||||||
return None;
|
|
||||||
}
|
|
||||||
|
|
||||||
// If the name was defined in that scope...
|
|
||||||
if let Some(binding) = scope
|
|
||||||
.get(name)
|
|
||||||
.map(|binding_id| checker.semantic().binding(binding_id))
|
|
||||||
{
|
|
||||||
// And has already been accessed in the current scope...
|
|
||||||
if let Some(range) = binding.references().find_map(|reference_id| {
|
|
||||||
let reference = checker.semantic().reference(reference_id);
|
let reference = checker.semantic().reference(reference_id);
|
||||||
if checker.semantic().is_current_scope(reference.scope_id()) {
|
if reference.scope_id() == scope_id {
|
||||||
Some(reference.range())
|
Some(reference.range())
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
}) {
|
}) {
|
||||||
// Then it's probably an error.
|
// Then it's probably an error.
|
||||||
return Some(range);
|
diagnostics.push(Diagnostic::new(
|
||||||
|
UndefinedLocal {
|
||||||
|
name: name.to_string(),
|
||||||
|
},
|
||||||
|
range,
|
||||||
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
None
|
|
||||||
});
|
|
||||||
|
|
||||||
if let Some(location) = local_access {
|
|
||||||
checker.diagnostics.push(Diagnostic::new(
|
|
||||||
UndefinedLocal {
|
|
||||||
name: name.to_string(),
|
|
||||||
},
|
|
||||||
location,
|
|
||||||
));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -8,4 +8,21 @@ F823.py:6:5: F823 Local variable `my_var` referenced before assignment
|
||||||
| ^^^^^^ F823
|
| ^^^^^^ F823
|
||||||
|
|
|
|
||||||
|
|
||||||
|
F823.py:32:15: F823 Local variable `my_var` referenced before assignment
|
||||||
|
|
|
||||||
|
30 | class Class:
|
||||||
|
31 | def f(self):
|
||||||
|
32 | print(my_var)
|
||||||
|
| ^^^^^^ F823
|
||||||
|
33 | my_var = 1
|
||||||
|
|
|
||||||
|
|
||||||
|
F823.py:40:15: F823 Local variable `my_var` referenced before assignment
|
||||||
|
|
|
||||||
|
39 | def f(self):
|
||||||
|
40 | print(my_var)
|
||||||
|
| ^^^^^^ F823
|
||||||
|
41 | my_var = 1
|
||||||
|
|
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue