diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F841_0.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F841_0.py index bde7153696..52dd297a0e 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F841_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F841_0.py @@ -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 diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/nonlocal_without_binding.py b/crates/ruff_linter/resources/test/fixtures/pylint/nonlocal_without_binding.py index 154ee23bec..4a45556a77 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/nonlocal_without_binding.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/nonlocal_without_binding.py @@ -44,3 +44,8 @@ def f(): def g(): nonlocal x x = 2 + +# OK +class A: + def method(self): + nonlocal __class__ diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 1437b1655d..906fe2491b 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -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(), diff --git a/crates/ruff_linter/src/renamer.rs b/crates/ruff_linter/src/renamer.rs index d31793dc74..d4c587142d 100644 --- a/crates/ruff_linter/src/renamer.rs +++ b/crates/ruff_linter/src/renamer.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 2aa8104d6b..12b7d5ce86 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -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(); diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F841_F841_0.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F841_F841_0.py.snap index 2361f61f36..b03ac3b0dd 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F841_F841_0.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F841_F841_0.py.snap @@ -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. diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f841_dummy_variable_rgx.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f841_dummy_variable_rgx.snap index 747f7ba421..7caaa6ef15 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f841_dummy_variable_rgx.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__f841_dummy_variable_rgx.snap @@ -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. diff --git a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs index 2458f418a4..6d66ac2345 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs @@ -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; } }; diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index e762d5d95c..9254a5b528 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -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), + + /// 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 for more + /// details. + DunderClassCell, } bitflags! { diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 3d8d39febe..16d57622cc 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -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 { 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 diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index 0ac31f26cc..f44ac019a5 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -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 "", line 1, in + /// class Bar: + /// def method(self, x=__class__): ... + /// File "", 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__ + /// + /// ``` + /// + /// 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 . + DunderClassCell, Function(&'a ast::StmtFunctionDef), Generator { kind: GeneratorKind,