mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 05:14:52 +00:00
Add a ScopeKind
for the __class__
cell (#20048)
Summary -- This PR aims to resolve (or help to resolve) #18442 and #19357 by encoding the CPython semantics around the `__class__` cell in our semantic model. Namely, > `__class__` is an implicit closure reference created by the compiler if any methods in a class body refer to either `__class__` or super. from the Python [docs](https://docs.python.org/3/reference/datamodel.html#creating-the-class-object). As noted in the variant docs by @AlexWaygood, we don't fully model this behavior, opting always to create the `__class__` cell binding in a new `ScopeKind::DunderClassCell` around each method definition, without checking if any method in the class body actually refers to `__class__` or `super`. As such, this PR fixes #18442 but not #19357. Test Plan -- Existing tests, plus the tests from #19783, which now pass without any rule-specific code. Note that we opted not to alter the behavior of F841 here because flagging `__class__` in these cases still seems helpful. See the discussion in https://github.com/astral-sh/ruff/pull/20048#discussion_r2296252395 and in the test comments for more information. --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com> Co-authored-by: Mikko Leppänen <mleppan23@gmail.com>
This commit is contained in:
parent
911d5cc973
commit
bc7274d148
11 changed files with 287 additions and 37 deletions
|
@ -151,3 +151,39 @@ def f():
|
|||
pass
|
||||
except Exception as _:
|
||||
pass
|
||||
|
||||
|
||||
# OK, `__class__` in this case is not the special `__class__` cell, so we don't
|
||||
# emit a diagnostic. (It has its own special semantics -- see
|
||||
# https://github.com/astral-sh/ruff/pull/20048#discussion_r2298338048 -- but
|
||||
# those aren't relevant here.)
|
||||
class A:
|
||||
__class__ = 1
|
||||
|
||||
|
||||
# The following three cases are flagged because they declare local `__class__`
|
||||
# variables that don't refer to the special `__class__` cell.
|
||||
class A:
|
||||
def set_class(self, cls):
|
||||
__class__ = cls # F841
|
||||
|
||||
|
||||
class A:
|
||||
class B:
|
||||
def set_class(self, cls):
|
||||
__class__ = cls # F841
|
||||
|
||||
|
||||
class A:
|
||||
def foo():
|
||||
class B:
|
||||
print(__class__)
|
||||
def set_class(self, cls):
|
||||
__class__ = cls # F841
|
||||
|
||||
|
||||
# OK, the `__class__` cell is nonlocal and declared as such.
|
||||
class NonlocalDunderClass:
|
||||
def foo():
|
||||
nonlocal __class__
|
||||
__class__ = 1
|
||||
|
|
|
@ -44,3 +44,8 @@ def f():
|
|||
def g():
|
||||
nonlocal x
|
||||
x = 2
|
||||
|
||||
# OK
|
||||
class A:
|
||||
def method(self):
|
||||
nonlocal __class__
|
||||
|
|
|
@ -703,7 +703,10 @@ impl SemanticSyntaxContext for Checker<'_> {
|
|||
match scope.kind {
|
||||
ScopeKind::Class(_) | ScopeKind::Lambda(_) => return false,
|
||||
ScopeKind::Function(ast::StmtFunctionDef { is_async, .. }) => return *is_async,
|
||||
ScopeKind::Generator { .. } | ScopeKind::Module | ScopeKind::Type => {}
|
||||
ScopeKind::Generator { .. }
|
||||
| ScopeKind::Module
|
||||
| ScopeKind::Type
|
||||
| ScopeKind::DunderClassCell => {}
|
||||
}
|
||||
}
|
||||
false
|
||||
|
@ -714,7 +717,10 @@ impl SemanticSyntaxContext for Checker<'_> {
|
|||
match scope.kind {
|
||||
ScopeKind::Class(_) => return false,
|
||||
ScopeKind::Function(_) | ScopeKind::Lambda(_) => return true,
|
||||
ScopeKind::Generator { .. } | ScopeKind::Module | ScopeKind::Type => {}
|
||||
ScopeKind::Generator { .. }
|
||||
| ScopeKind::Module
|
||||
| ScopeKind::Type
|
||||
| ScopeKind::DunderClassCell => {}
|
||||
}
|
||||
}
|
||||
false
|
||||
|
@ -725,7 +731,7 @@ impl SemanticSyntaxContext for Checker<'_> {
|
|||
match scope.kind {
|
||||
ScopeKind::Class(_) | ScopeKind::Generator { .. } => return false,
|
||||
ScopeKind::Function(_) | ScopeKind::Lambda(_) => return true,
|
||||
ScopeKind::Module | ScopeKind::Type => {}
|
||||
ScopeKind::Module | ScopeKind::Type | ScopeKind::DunderClassCell => {}
|
||||
}
|
||||
}
|
||||
false
|
||||
|
@ -1092,6 +1098,24 @@ impl<'a> Visitor<'a> for Checker<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
// Here we add the implicit scope surrounding a method which allows code in the
|
||||
// method to access `__class__` at runtime. See the `ScopeKind::DunderClassCell`
|
||||
// docs for more information.
|
||||
let added_dunder_class_scope = if self.semantic.current_scope().kind.is_class() {
|
||||
self.semantic.push_scope(ScopeKind::DunderClassCell);
|
||||
let binding_id = self.semantic.push_binding(
|
||||
TextRange::default(),
|
||||
BindingKind::DunderClassCell,
|
||||
BindingFlags::empty(),
|
||||
);
|
||||
self.semantic
|
||||
.current_scope_mut()
|
||||
.add("__class__", binding_id);
|
||||
true
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
self.semantic.push_scope(ScopeKind::Type);
|
||||
|
||||
if let Some(type_params) = type_params {
|
||||
|
@ -1155,6 +1179,9 @@ impl<'a> Visitor<'a> for Checker<'a> {
|
|||
self.semantic.pop_scope(); // Function scope
|
||||
self.semantic.pop_definition();
|
||||
self.semantic.pop_scope(); // Type parameter scope
|
||||
if added_dunder_class_scope {
|
||||
self.semantic.pop_scope(); // `__class__` cell closure scope
|
||||
}
|
||||
self.add_binding(
|
||||
name,
|
||||
stmt.identifier(),
|
||||
|
|
|
@ -354,7 +354,10 @@ impl Renamer {
|
|||
))
|
||||
}
|
||||
// Avoid renaming builtins and other "special" bindings.
|
||||
BindingKind::FutureImport | BindingKind::Builtin | BindingKind::Export(_) => None,
|
||||
BindingKind::FutureImport
|
||||
| BindingKind::Builtin
|
||||
| BindingKind::Export(_)
|
||||
| BindingKind::DunderClassCell => None,
|
||||
// By default, replace the binding's name with the target name.
|
||||
BindingKind::Annotation
|
||||
| BindingKind::Argument
|
||||
|
|
|
@ -737,6 +737,7 @@ mod tests {
|
|||
|
||||
/// A re-implementation of the Pyflakes test runner.
|
||||
/// Note that all tests marked with `#[ignore]` should be considered TODOs.
|
||||
#[track_caller]
|
||||
fn flakes(contents: &str, expected: &[Rule]) {
|
||||
let contents = dedent(contents);
|
||||
let source_type = PySourceType::default();
|
||||
|
|
|
@ -251,3 +251,63 @@ F841 Local variable `value` is assigned to but never used
|
|||
128 | print(key)
|
||||
|
|
||||
help: Remove assignment to unused variable `value`
|
||||
|
||||
F841 [*] Local variable `__class__` is assigned to but never used
|
||||
--> F841_0.py:168:9
|
||||
|
|
||||
166 | class A:
|
||||
167 | def set_class(self, cls):
|
||||
168 | __class__ = cls # F841
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
help: Remove assignment to unused variable `__class__`
|
||||
|
||||
ℹ Unsafe fix
|
||||
165 165 | # variables that don't refer to the special `__class__` cell.
|
||||
166 166 | class A:
|
||||
167 167 | def set_class(self, cls):
|
||||
168 |- __class__ = cls # F841
|
||||
168 |+ pass # F841
|
||||
169 169 |
|
||||
170 170 |
|
||||
171 171 | class A:
|
||||
|
||||
F841 [*] Local variable `__class__` is assigned to but never used
|
||||
--> F841_0.py:174:13
|
||||
|
|
||||
172 | class B:
|
||||
173 | def set_class(self, cls):
|
||||
174 | __class__ = cls # F841
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
help: Remove assignment to unused variable `__class__`
|
||||
|
||||
ℹ Unsafe fix
|
||||
171 171 | class A:
|
||||
172 172 | class B:
|
||||
173 173 | def set_class(self, cls):
|
||||
174 |- __class__ = cls # F841
|
||||
174 |+ pass # F841
|
||||
175 175 |
|
||||
176 176 |
|
||||
177 177 | class A:
|
||||
|
||||
F841 [*] Local variable `__class__` is assigned to but never used
|
||||
--> F841_0.py:182:17
|
||||
|
|
||||
180 | print(__class__)
|
||||
181 | def set_class(self, cls):
|
||||
182 | __class__ = cls # F841
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
help: Remove assignment to unused variable `__class__`
|
||||
|
||||
ℹ Unsafe fix
|
||||
179 179 | class B:
|
||||
180 180 | print(__class__)
|
||||
181 181 | def set_class(self, cls):
|
||||
182 |- __class__ = cls # F841
|
||||
182 |+ pass # F841
|
||||
183 183 |
|
||||
184 184 |
|
||||
185 185 | # OK, the `__class__` cell is nonlocal and declared as such.
|
||||
|
|
|
@ -289,3 +289,65 @@ help: Remove assignment to unused variable `_`
|
|||
152 |- except Exception as _:
|
||||
152 |+ except Exception:
|
||||
153 153 | pass
|
||||
154 154 |
|
||||
155 155 |
|
||||
|
||||
F841 [*] Local variable `__class__` is assigned to but never used
|
||||
--> F841_0.py:168:9
|
||||
|
|
||||
166 | class A:
|
||||
167 | def set_class(self, cls):
|
||||
168 | __class__ = cls # F841
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
help: Remove assignment to unused variable `__class__`
|
||||
|
||||
ℹ Unsafe fix
|
||||
165 165 | # variables that don't refer to the special `__class__` cell.
|
||||
166 166 | class A:
|
||||
167 167 | def set_class(self, cls):
|
||||
168 |- __class__ = cls # F841
|
||||
168 |+ pass # F841
|
||||
169 169 |
|
||||
170 170 |
|
||||
171 171 | class A:
|
||||
|
||||
F841 [*] Local variable `__class__` is assigned to but never used
|
||||
--> F841_0.py:174:13
|
||||
|
|
||||
172 | class B:
|
||||
173 | def set_class(self, cls):
|
||||
174 | __class__ = cls # F841
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
help: Remove assignment to unused variable `__class__`
|
||||
|
||||
ℹ Unsafe fix
|
||||
171 171 | class A:
|
||||
172 172 | class B:
|
||||
173 173 | def set_class(self, cls):
|
||||
174 |- __class__ = cls # F841
|
||||
174 |+ pass # F841
|
||||
175 175 |
|
||||
176 176 |
|
||||
177 177 | class A:
|
||||
|
||||
F841 [*] Local variable `__class__` is assigned to but never used
|
||||
--> F841_0.py:182:17
|
||||
|
|
||||
180 | print(__class__)
|
||||
181 | def set_class(self, cls):
|
||||
182 | __class__ = cls # F841
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
help: Remove assignment to unused variable `__class__`
|
||||
|
||||
ℹ Unsafe fix
|
||||
179 179 | class B:
|
||||
180 180 | print(__class__)
|
||||
181 181 | def set_class(self, cls):
|
||||
182 |- __class__ = cls # F841
|
||||
182 |+ pass # F841
|
||||
183 183 |
|
||||
184 184 |
|
||||
185 185 | # OK, the `__class__` cell is nonlocal and declared as such.
|
||||
|
|
|
@ -73,7 +73,8 @@ pub(crate) fn non_ascii_name(checker: &Checker, binding: &Binding) {
|
|||
| BindingKind::SubmoduleImport(_)
|
||||
| BindingKind::Deletion
|
||||
| BindingKind::ConditionalDeletion(_)
|
||||
| BindingKind::UnboundException(_) => {
|
||||
| BindingKind::UnboundException(_)
|
||||
| BindingKind::DunderClassCell => {
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
|
|
@ -446,7 +446,7 @@ impl Ranged for Binding<'_> {
|
|||
/// ID uniquely identifying a [Binding] in a program.
|
||||
///
|
||||
/// Using a `u32` to identify [Binding]s should be sufficient because Ruff only supports documents with a
|
||||
/// size smaller than or equal to `u32::max`. A document with the size of `u32::max` must have fewer than `u32::max`
|
||||
/// size smaller than or equal to `u32::MAX`. A document with the size of `u32::MAX` must have fewer than `u32::MAX`
|
||||
/// bindings because bindings must be separated by whitespace (and have an assignment).
|
||||
#[newtype_index]
|
||||
pub struct BindingId;
|
||||
|
@ -672,6 +672,24 @@ pub enum BindingKind<'a> {
|
|||
/// Stores the ID of the binding that was shadowed in the enclosing
|
||||
/// scope, if any.
|
||||
UnboundException(Option<BindingId>),
|
||||
|
||||
/// A binding to `__class__` in the implicit closure created around every method in a class
|
||||
/// body, if any method refers to either `__class__` or `super`.
|
||||
///
|
||||
/// ```python
|
||||
/// class C:
|
||||
/// __class__ # NameError: name '__class__' is not defined
|
||||
///
|
||||
/// def f():
|
||||
/// print(__class__) # allowed
|
||||
///
|
||||
/// def g():
|
||||
/// nonlocal __class__ # also allowed because the scope is *not* the function scope
|
||||
/// ```
|
||||
///
|
||||
/// See <https://docs.python.org/3/reference/datamodel.html#creating-the-class-object> for more
|
||||
/// details.
|
||||
DunderClassCell,
|
||||
}
|
||||
|
||||
bitflags! {
|
||||
|
|
|
@ -404,22 +404,11 @@ impl<'a> SemanticModel<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
let mut seen_function = false;
|
||||
let mut import_starred = false;
|
||||
let mut class_variables_visible = true;
|
||||
for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() {
|
||||
let scope = &self.scopes[scope_id];
|
||||
if scope.kind.is_class() {
|
||||
// Allow usages of `__class__` within methods, e.g.:
|
||||
//
|
||||
// ```python
|
||||
// class Foo:
|
||||
// def __init__(self):
|
||||
// print(__class__)
|
||||
// ```
|
||||
if seen_function && matches!(name.id.as_str(), "__class__") {
|
||||
return ReadResult::ImplicitGlobal;
|
||||
}
|
||||
// Do not allow usages of class symbols unless it is the immediate parent
|
||||
// (excluding type scopes), e.g.:
|
||||
//
|
||||
|
@ -442,7 +431,13 @@ impl<'a> SemanticModel<'a> {
|
|||
// Allow class variables to be visible for an additional scope level
|
||||
// when a type scope is seen — this covers the type scope present between
|
||||
// function and class definitions and their parent class scope.
|
||||
class_variables_visible = scope.kind.is_type() && index == 0;
|
||||
//
|
||||
// Also allow an additional level beyond that to cover the implicit
|
||||
// `__class__` closure created around methods and enclosing the type scope.
|
||||
class_variables_visible = matches!(
|
||||
(scope.kind, index),
|
||||
(ScopeKind::Type, 0) | (ScopeKind::DunderClassCell, 1)
|
||||
);
|
||||
|
||||
if let Some(binding_id) = scope.get(name.id.as_str()) {
|
||||
// Mark the binding as used.
|
||||
|
@ -614,7 +609,6 @@ impl<'a> SemanticModel<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
seen_function |= scope.kind.is_function();
|
||||
import_starred = import_starred || scope.uses_star_imports();
|
||||
}
|
||||
|
||||
|
@ -658,21 +652,19 @@ impl<'a> SemanticModel<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
let mut seen_function = false;
|
||||
let mut class_variables_visible = true;
|
||||
for (index, scope_id) in self.scopes.ancestor_ids(scope_id).enumerate() {
|
||||
let scope = &self.scopes[scope_id];
|
||||
if scope.kind.is_class() {
|
||||
if seen_function && matches!(symbol, "__class__") {
|
||||
return None;
|
||||
}
|
||||
if !class_variables_visible {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
class_variables_visible = scope.kind.is_type() && index == 0;
|
||||
seen_function |= scope.kind.is_function();
|
||||
class_variables_visible = matches!(
|
||||
(scope.kind, index),
|
||||
(ScopeKind::Type, 0) | (ScopeKind::DunderClassCell, 1)
|
||||
);
|
||||
|
||||
if let Some(binding_id) = scope.get(symbol) {
|
||||
match self.bindings[binding_id].kind {
|
||||
|
@ -786,15 +778,15 @@ impl<'a> SemanticModel<'a> {
|
|||
}
|
||||
|
||||
if scope.kind.is_class() {
|
||||
if seen_function && matches!(symbol, "__class__") {
|
||||
return None;
|
||||
}
|
||||
if !class_variables_visible {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
class_variables_visible = scope.kind.is_type() && index == 0;
|
||||
class_variables_visible = matches!(
|
||||
(scope.kind, index),
|
||||
(ScopeKind::Type, 0) | (ScopeKind::DunderClassCell, 1)
|
||||
);
|
||||
seen_function |= scope.kind.is_function();
|
||||
|
||||
if let Some(binding_id) = scope.get(symbol) {
|
||||
|
@ -1353,11 +1345,12 @@ impl<'a> SemanticModel<'a> {
|
|||
self.scopes[scope_id].parent
|
||||
}
|
||||
|
||||
/// Returns the first parent of the given [`Scope`] that is not of [`ScopeKind::Type`], if any.
|
||||
/// Returns the first parent of the given [`Scope`] that is not of [`ScopeKind::Type`] or
|
||||
/// [`ScopeKind::DunderClassCell`], if any.
|
||||
pub fn first_non_type_parent_scope(&self, scope: &Scope) -> Option<&Scope<'a>> {
|
||||
let mut current_scope = scope;
|
||||
while let Some(parent) = self.parent_scope(current_scope) {
|
||||
if parent.kind.is_type() {
|
||||
if matches!(parent.kind, ScopeKind::Type | ScopeKind::DunderClassCell) {
|
||||
current_scope = parent;
|
||||
} else {
|
||||
return Some(parent);
|
||||
|
@ -1366,11 +1359,15 @@ impl<'a> SemanticModel<'a> {
|
|||
None
|
||||
}
|
||||
|
||||
/// Returns the first parent of the given [`ScopeId`] that is not of [`ScopeKind::Type`], if any.
|
||||
/// Returns the first parent of the given [`ScopeId`] that is not of [`ScopeKind::Type`] or
|
||||
/// [`ScopeKind::DunderClassCell`], if any.
|
||||
pub fn first_non_type_parent_scope_id(&self, scope_id: ScopeId) -> Option<ScopeId> {
|
||||
let mut current_scope_id = scope_id;
|
||||
while let Some(parent_id) = self.parent_scope_id(current_scope_id) {
|
||||
if self.scopes[parent_id].kind.is_type() {
|
||||
if matches!(
|
||||
self.scopes[parent_id].kind,
|
||||
ScopeKind::Type | ScopeKind::DunderClassCell
|
||||
) {
|
||||
current_scope_id = parent_id;
|
||||
} else {
|
||||
return Some(parent_id);
|
||||
|
@ -2649,16 +2646,16 @@ pub enum ReadResult {
|
|||
/// The `x` in `print(x)` is resolved to the binding of `x` in `x = 1`.
|
||||
Resolved(BindingId),
|
||||
|
||||
/// The read reference is resolved to a context-specific, implicit global (e.g., `__class__`
|
||||
/// The read reference is resolved to a context-specific, implicit global (e.g., `__qualname__`
|
||||
/// within a class scope).
|
||||
///
|
||||
/// For example, given:
|
||||
/// ```python
|
||||
/// class C:
|
||||
/// print(__class__)
|
||||
/// print(__qualname__)
|
||||
/// ```
|
||||
///
|
||||
/// The `__class__` in `print(__class__)` is resolved to the implicit global `__class__`.
|
||||
/// The `__qualname__` in `print(__qualname__)` is resolved to the implicit global `__qualname__`.
|
||||
ImplicitGlobal,
|
||||
|
||||
/// The read reference is unresolved, but at least one of the containing scopes contains a
|
||||
|
|
|
@ -166,9 +166,49 @@ bitflags! {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, is_macro::Is)]
|
||||
#[derive(Clone, Copy, Debug, is_macro::Is)]
|
||||
pub enum ScopeKind<'a> {
|
||||
Class(&'a ast::StmtClassDef),
|
||||
/// The implicit `__class__` scope surrounding a method which allows code in the
|
||||
/// method to access `__class__` at runtime. The closure sits in between the class
|
||||
/// scope and the function scope.
|
||||
///
|
||||
/// Parameter defaults in methods cannot access `__class__`:
|
||||
///
|
||||
/// ```pycon
|
||||
/// >>> class Bar:
|
||||
/// ... def method(self, x=__class__): ...
|
||||
/// ...
|
||||
/// Traceback (most recent call last):
|
||||
/// File "<python-input-6>", line 1, in <module>
|
||||
/// class Bar:
|
||||
/// def method(self, x=__class__): ...
|
||||
/// File "<python-input-6>", line 2, in Bar
|
||||
/// def method(self, x=__class__): ...
|
||||
/// ^^^^^^^^^
|
||||
/// NameError: name '__class__' is not defined
|
||||
/// ```
|
||||
///
|
||||
/// However, type parameters in methods *can* access `__class__`:
|
||||
///
|
||||
/// ```pycon
|
||||
/// >>> class Foo:
|
||||
/// ... def bar[T: __class__](): ...
|
||||
/// ...
|
||||
/// >>> Foo.bar.__type_params__[0].__bound__
|
||||
/// <class '__main__.Foo'>
|
||||
/// ```
|
||||
///
|
||||
/// Note that this is still not 100% accurate! At runtime, the implicit `__class__`
|
||||
/// closure is only added if the name `super` (has to be a name -- `builtins.super`
|
||||
/// and similar don't count!) or the name `__class__` is used in any method of the
|
||||
/// class. However, accurately emulating that would be both complex and probably
|
||||
/// quite expensive unless we moved to a double-traversal of each scope similar to
|
||||
/// ty. It would also only matter in extreme and unlikely edge cases. So we ignore
|
||||
/// that subtlety for now.
|
||||
///
|
||||
/// See <https://docs.python.org/3/reference/datamodel.html#creating-the-class-object>.
|
||||
DunderClassCell,
|
||||
Function(&'a ast::StmtFunctionDef),
|
||||
Generator {
|
||||
kind: GeneratorKind,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue