Skip to content

Commit 0e8b83c

Browse files
ntBreAlexWaygood
andcommitted
Add a ScopeKind for the __class__ cell
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 code comment by @AlexWaygood, we don't fully model this behavior, opting always to create the `__class__` cell binding in a new `ScopeKind::ClassCell` around each method definition, without checking if any method in the class body actually refers to `__class__` or `super`. As such, this PR may only help with #18442 and not #19357. Test Plan -- Existing tests, plus (TODO) tests from #19783 and #19424, which tackled the issues above on a per-rule basis. Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
1 parent 0b6ce1c commit 0e8b83c

File tree

7 files changed

+93
-23
lines changed

7 files changed

+93
-23
lines changed

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

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,10 @@ impl SemanticSyntaxContext for Checker<'_> {
703703
match scope.kind {
704704
ScopeKind::Class(_) | ScopeKind::Lambda(_) => return false,
705705
ScopeKind::Function(ast::StmtFunctionDef { is_async, .. }) => return *is_async,
706-
ScopeKind::Generator { .. } | ScopeKind::Module | ScopeKind::Type => {}
706+
ScopeKind::Generator { .. }
707+
| ScopeKind::Module
708+
| ScopeKind::Type
709+
| ScopeKind::ClassCell => {}
707710
}
708711
}
709712
false
@@ -714,7 +717,10 @@ impl SemanticSyntaxContext for Checker<'_> {
714717
match scope.kind {
715718
ScopeKind::Class(_) => return false,
716719
ScopeKind::Function(_) | ScopeKind::Lambda(_) => return true,
717-
ScopeKind::Generator { .. } | ScopeKind::Module | ScopeKind::Type => {}
720+
ScopeKind::Generator { .. }
721+
| ScopeKind::Module
722+
| ScopeKind::Type
723+
| ScopeKind::ClassCell => {}
718724
}
719725
}
720726
false
@@ -725,7 +731,7 @@ impl SemanticSyntaxContext for Checker<'_> {
725731
match scope.kind {
726732
ScopeKind::Class(_) | ScopeKind::Generator { .. } => return false,
727733
ScopeKind::Function(_) | ScopeKind::Lambda(_) => return true,
728-
ScopeKind::Module | ScopeKind::Type => {}
734+
ScopeKind::Module | ScopeKind::Type | ScopeKind::ClassCell => {}
729735
}
730736
}
731737
false
@@ -1092,6 +1098,60 @@ impl<'a> Visitor<'a> for Checker<'a> {
10921098
}
10931099
}
10941100

1101+
// Here we add the implicit scope surrounding a method which allows
1102+
// code in the method to access `__class__` at runtime. The closure sits
1103+
// in between the class scope and the function scope.
1104+
//
1105+
// Parameter defaults in methods cannot access `__class__`:
1106+
//
1107+
// ```pycon
1108+
// >>> class Bar:
1109+
// ... def method(self, x=__class__): ...
1110+
// ...
1111+
// Traceback (most recent call last):
1112+
// File "<python-input-6>", line 1, in <module>
1113+
// class Bar:
1114+
// def method(self, x=__class__): ...
1115+
// File "<python-input-6>", line 2, in Bar
1116+
// def method(self, x=__class__): ...
1117+
// ^^^^^^^^^
1118+
// NameError: name '__class__' is not defined
1119+
// ```
1120+
//
1121+
// However, type parameters in methods *can* access `__class__`:
1122+
//
1123+
// ```pycon
1124+
// >>> class Foo:
1125+
// ... def bar[T: __class__](): ...
1126+
// ...
1127+
// >>> Foo.bar.__type_params__[0].__bound__
1128+
// <class '__main__.Foo'>
1129+
// ```
1130+
//
1131+
// Note that this is still not 100% accurate! At runtime, the implicit `__class__`
1132+
// closure is only added if the name `super` (has to be a name -- `builtins.super`
1133+
// and similar don't count!) or the name `__class__` is used in any method of the
1134+
// class. However, accurately emulating that would be both complex and probably
1135+
// quite expensive unless we moved to a double-traversal of each scope similar to
1136+
// ty. It would also only matter in extreme and unlikely edge cases. So we ignore
1137+
// that subtlety for now.
1138+
//
1139+
// See <https://docs.python.org/3/reference/datamodel.html#creating-the-class-object>.
1140+
let added_dunder_class_scope = if self.semantic.current_scope().kind.is_class() {
1141+
self.semantic.push_scope(ScopeKind::ClassCell);
1142+
let binding_id = self.semantic.push_binding(
1143+
TextRange::default(),
1144+
BindingKind::ClassCell,
1145+
BindingFlags::empty(),
1146+
);
1147+
self.semantic
1148+
.current_scope_mut()
1149+
.add("__class__", binding_id);
1150+
true
1151+
} else {
1152+
false
1153+
};
1154+
10951155
self.semantic.push_scope(ScopeKind::Type);
10961156

10971157
if let Some(type_params) = type_params {
@@ -1155,6 +1215,9 @@ impl<'a> Visitor<'a> for Checker<'a> {
11551215
self.semantic.pop_scope(); // Function scope
11561216
self.semantic.pop_definition();
11571217
self.semantic.pop_scope(); // Type parameter scope
1218+
if added_dunder_class_scope {
1219+
self.semantic.pop_scope(); // `__class__` cell closure scope
1220+
}
11581221
self.add_binding(
11591222
name,
11601223
stmt.identifier(),

crates/ruff_linter/src/renamer.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,10 @@ impl Renamer {
354354
))
355355
}
356356
// Avoid renaming builtins and other "special" bindings.
357-
BindingKind::FutureImport | BindingKind::Builtin | BindingKind::Export(_) => None,
357+
BindingKind::FutureImport
358+
| BindingKind::Builtin
359+
| BindingKind::Export(_)
360+
| BindingKind::ClassCell => None,
358361
// By default, replace the binding's name with the target name.
359362
BindingKind::Annotation
360363
| BindingKind::Argument

crates/ruff_linter/src/rules/pyflakes/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,7 @@ mod tests {
737737

738738
/// A re-implementation of the Pyflakes test runner.
739739
/// Note that all tests marked with `#[ignore]` should be considered TODOs.
740+
#[track_caller]
740741
fn flakes(contents: &str, expected: &[Rule]) {
741742
let contents = dedent(contents);
742743
let source_type = PySourceType::default();

crates/ruff_linter/src/rules/pylint/rules/non_ascii_name.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ pub(crate) fn non_ascii_name(checker: &Checker, binding: &Binding) {
7373
| BindingKind::SubmoduleImport(_)
7474
| BindingKind::Deletion
7575
| BindingKind::ConditionalDeletion(_)
76-
| BindingKind::UnboundException(_) => {
76+
| BindingKind::UnboundException(_)
77+
| BindingKind::ClassCell => {
7778
return;
7879
}
7980
};

crates/ruff_python_semantic/src/binding.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,11 +446,17 @@ impl Ranged for Binding<'_> {
446446
/// ID uniquely identifying a [Binding] in a program.
447447
///
448448
/// Using a `u32` to identify [Binding]s should be sufficient because Ruff only supports documents with a
449-
/// size smaller than or equal to `u32::max`. A document with the size of `u32::max` must have fewer than `u32::max`
449+
/// size smaller than or equal to `u32::MAX`. A document with the size of `u32::MAX` must have fewer than `u32::MAX`
450450
/// bindings because bindings must be separated by whitespace (and have an assignment).
451451
#[newtype_index]
452452
pub struct BindingId;
453453

454+
impl BindingId {
455+
pub const fn dunder_class_id() -> Self {
456+
BindingId::from_u32(u32::MAX)
457+
}
458+
}
459+
454460
/// The bindings in a program.
455461
///
456462
/// Bindings are indexed by [`BindingId`]
@@ -672,6 +678,9 @@ pub enum BindingKind<'a> {
672678
/// Stores the ID of the binding that was shadowed in the enclosing
673679
/// scope, if any.
674680
UnboundException(Option<BindingId>),
681+
682+
/// TODO
683+
ClassCell,
675684
}
676685

677686
bitflags! {

crates/ruff_python_semantic/src/model.rs

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -404,22 +404,11 @@ impl<'a> SemanticModel<'a> {
404404
}
405405
}
406406

407-
let mut seen_function = false;
408407
let mut import_starred = false;
409408
let mut class_variables_visible = true;
410409
for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() {
411410
let scope = &self.scopes[scope_id];
412411
if scope.kind.is_class() {
413-
// Allow usages of `__class__` within methods, e.g.:
414-
//
415-
// ```python
416-
// class Foo:
417-
// def __init__(self):
418-
// print(__class__)
419-
// ```
420-
if seen_function && matches!(name.id.as_str(), "__class__") {
421-
return ReadResult::ImplicitGlobal;
422-
}
423412
// Do not allow usages of class symbols unless it is the immediate parent
424413
// (excluding type scopes), e.g.:
425414
//
@@ -442,7 +431,8 @@ impl<'a> SemanticModel<'a> {
442431
// Allow class variables to be visible for an additional scope level
443432
// when a type scope is seen — this covers the type scope present between
444433
// function and class definitions and their parent class scope.
445-
class_variables_visible = scope.kind.is_type() && index == 0;
434+
class_variables_visible =
435+
(scope.kind.is_type() && index == 0) || (scope.kind.is_class_cell() && index == 1);
446436

447437
if let Some(binding_id) = scope.get(name.id.as_str()) {
448438
// Mark the binding as used.
@@ -614,7 +604,6 @@ impl<'a> SemanticModel<'a> {
614604
}
615605
}
616606

617-
seen_function |= scope.kind.is_function();
618607
import_starred = import_starred || scope.uses_star_imports();
619608
}
620609

@@ -1353,11 +1342,11 @@ impl<'a> SemanticModel<'a> {
13531342
self.scopes[scope_id].parent
13541343
}
13551344

1356-
/// Returns the first parent of the given [`Scope`] that is not of [`ScopeKind::Type`], if any.
1345+
/// Returns the first parent of the given [`Scope`] that is not of [`ScopeKind::Type`] or [`ScopeKind::ClassCell`], if any.
13571346
pub fn first_non_type_parent_scope(&self, scope: &Scope) -> Option<&Scope<'a>> {
13581347
let mut current_scope = scope;
13591348
while let Some(parent) = self.parent_scope(current_scope) {
1360-
if parent.kind.is_type() {
1349+
if matches!(parent.kind, ScopeKind::Type | ScopeKind::ClassCell) {
13611350
current_scope = parent;
13621351
} else {
13631352
return Some(parent);
@@ -1366,11 +1355,14 @@ impl<'a> SemanticModel<'a> {
13661355
None
13671356
}
13681357

1369-
/// Returns the first parent of the given [`ScopeId`] that is not of [`ScopeKind::Type`], if any.
1358+
/// Returns the first parent of the given [`ScopeId`] that is not of [`ScopeKind::Type`] or [`ScopeKind::ClassCell`], if any.
13701359
pub fn first_non_type_parent_scope_id(&self, scope_id: ScopeId) -> Option<ScopeId> {
13711360
let mut current_scope_id = scope_id;
13721361
while let Some(parent_id) = self.parent_scope_id(current_scope_id) {
1373-
if self.scopes[parent_id].kind.is_type() {
1362+
if matches!(
1363+
self.scopes[parent_id].kind,
1364+
ScopeKind::Type | ScopeKind::ClassCell
1365+
) {
13741366
current_scope_id = parent_id;
13751367
} else {
13761368
return Some(parent_id);

crates/ruff_python_semantic/src/scope.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ bitflags! {
169169
#[derive(Debug, is_macro::Is)]
170170
pub enum ScopeKind<'a> {
171171
Class(&'a ast::StmtClassDef),
172+
ClassCell,
172173
Function(&'a ast::StmtFunctionDef),
173174
Generator {
174175
kind: GeneratorKind,

0 commit comments

Comments
 (0)