Skip to content

[ty] Revert protocol ClassVar declaration requirement #27726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
AlexWaygood wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[ty] Revert protocol ClassVar declaration requirement #27726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 12 additions & 72 deletions crates/ty_python_semantic/resources/mdtest/protocols.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,12 +776,12 @@ static_assert(is_assignable_to(Qux, HasXWithDefault))
class HasClassVarX(Protocol):
x: ClassVar[int]

static_assert(not is_subtype_of(FooWithZero, HasClassVarX))
static_assert(not is_assignable_to(FooWithZero, HasClassVarX))
static_assert(is_subtype_of(FooWithZero, HasClassVarX))
static_assert(is_assignable_to(FooWithZero, HasClassVarX))

# An instance declaration does not become a class variable without an explicit qualifier.
static_assert(not is_subtype_of(Foo, HasClassVarX))
static_assert(not is_assignable_to(Foo, HasClassVarX))
# TODO: these should pass

@charliermarsh charliermarsh Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain whether this is still a TODO? (And, if so, why revert?)

static_assert(not is_subtype_of(Foo, HasClassVarX)) # error: [static-assert-error]
static_assert(not is_assignable_to(Foo, HasClassVarX)) # error: [static-assert-error]

static_assert(not is_subtype_of(Qux, HasClassVarX))
static_assert(not is_assignable_to(Qux, HasClassVarX))
Expand Down Expand Up @@ -2052,16 +2052,14 @@ static_assert(is_assignable_to(UsesMeta, HasX))

If a protocol `ClassVarX` has a `ClassVar` attribute member `x` with type `int`, this indicates that
the non-callable attribute must be readable with the same type through both an inhabitant of
`ClassVarX` and the type of that inhabitant. An implementing class must declare the member as a
`ClassVar`; an instance attribute does not satisfy the requirement merely because it has a default
value in the class body:
`ClassVarX` and the type of that inhabitant:

`classvars.py`:

```py
from typing import Any, ClassVar, Protocol, final
from ty_extensions import Intersection, static_assert
from ty_extensions._internal import TypeOf, is_assignable_to, is_disjoint_from, is_subtype_of
from typing import Any, ClassVar, Protocol
from ty_extensions import static_assert
from ty_extensions._internal import is_subtype_of, is_assignable_to

class ClassVarXProto(Protocol):
x: ClassVar[int]
Expand All @@ -2074,14 +2072,9 @@ def f(obj: ClassVarXProto):
class InstanceAttrX:
x: int

static_assert(not is_assignable_to(InstanceAttrX, ClassVarXProto))
static_assert(not is_subtype_of(InstanceAttrX, ClassVarXProto))

class InstanceAttrXWithDefault:
x: int = 42

static_assert(not is_assignable_to(InstanceAttrXWithDefault, ClassVarXProto))
static_assert(not is_subtype_of(InstanceAttrXWithDefault, ClassVarXProto))
# TODO: these should pass
static_assert(not is_assignable_to(InstanceAttrX, ClassVarXProto)) # error: [static-assert-error]
static_assert(not is_subtype_of(InstanceAttrX, ClassVarXProto)) # error: [static-assert-error]

class PropertyX:
@property
Expand All @@ -2097,14 +2090,6 @@ class ClassVarX:
static_assert(is_assignable_to(ClassVarX, ClassVarXProto))
static_assert(is_subtype_of(ClassVarX, ClassVarXProto))

class InheritedClassVarX(ClassVarX):
x = 1

static_assert(is_assignable_to(InheritedClassVarX, ClassVarXProto))
static_assert(is_subtype_of(InheritedClassVarX, ClassVarXProto))
static_assert(is_assignable_to(TypeOf[InheritedClassVarX], type[ClassVarXProto]))
static_assert(is_subtype_of(TypeOf[InheritedClassVarX], type[ClassVarXProto]))

class XMeta(type):
def x(cls) -> str:
return ""
Expand Down Expand Up @@ -2134,51 +2119,6 @@ class NotHashable:

static_assert(is_assignable_to(NotHashable, NotHashableProto))
static_assert(is_subtype_of(NotHashable, NotHashableProto))

class Descriptor:
def __get__(self, instance: object, owner: type) -> "Descriptor":
return self

def __set__(self, instance: object, value: "Descriptor") -> None: ...

class HasClassDescriptor(Protocol):
descriptor: ClassVar[Descriptor]

class DescriptorImplementation:
descriptor: ClassVar[Descriptor] = Descriptor()

static_assert(is_assignable_to(DescriptorImplementation, HasClassDescriptor))
static_assert(is_subtype_of(DescriptorImplementation, HasClassDescriptor))

@final
class FinalInstanceAttrX:
x: int = 42

@final
class FinalClassVarX:
x: ClassVar[int] = 42

static_assert(is_disjoint_from(FinalInstanceAttrX, ClassVarXProto))
static_assert(not is_disjoint_from(InstanceAttrXWithDefault, ClassVarXProto))
static_assert(not is_disjoint_from(FinalClassVarX, ClassVarXProto))

def impossible(value: Intersection[FinalInstanceAttrX, ClassVarXProto]) -> None:
reveal_type(value) # revealed: Never

implementation: ClassVarXProto = InstanceAttrX() # snapshot: invalid-assignment
```

```snapshot
error[invalid-assignment]: Object of type `InstanceAttrX` is not assignable to `ClassVarXProto`
--> src/classvars.py:107:34
|
107 | implementation: ClassVarXProto = InstanceAttrX() # snapshot: invalid-assignment
| -------------- ^^^^^^^^^^^^^^^ Incompatible value of type `InstanceAttrX`
| |
| Declared type
info: type `InstanceAttrX` is not assignable to protocol `ClassVarXProto`
info: └── protocol member `x` is incompatible
info: └── protocol member `x` is an instance variable on type `InstanceAttrX`, but a class variable is required
```

This is mentioned by the
Expand Down
6 changes: 3 additions & 3 deletions crates/ty_python_semantic/src/types/overrides.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ fn method_override_types<'db>(

/// Whether an attribute declaration is a class variable or an instance variable.
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, get_size2::GetSize)]
pub(super) enum VariableKind {
enum VariableKind {
/// A variable annotated with `ClassVar`.
Class,
/// An instance variable, including an unannotated class-body assignment.
Expand Down Expand Up @@ -1124,7 +1124,7 @@ fn superclass_variable_kind<'db>(
/// ```
#[allow(clippy::needless_pass_by_value)]
#[salsa::tracked(returns(copy), heap_size=ruff_memory_usage::heap_size)]
pub(super) fn effective_superclass_variable_kind<'db>(
fn effective_superclass_variable_kind<'db>(
db: &'db dyn Db,
superclass: ClassType<'db>,
name: Name,
Expand Down Expand Up @@ -1158,7 +1158,7 @@ pub(super) fn effective_superclass_variable_kind<'db>(
superclass_scope,
superclass_symbol_id,
superclass.own_class_member(db, env, None, &name).inner,
superclass.own_instance_member(db, env, &name).inner,
Type::instance(db, env, superclass).member(db, env, &name),
);

if superclass_variable_kind == Some(VariableKind::Instance)
Expand Down
83 changes: 0 additions & 83 deletions crates/ty_python_semantic/src/types/protocol_class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::types::attribute_write::{
ProtocolMemberWriteRequirement, attribute_write_requirement,
};
use crate::types::call::{CallArguments, CallDunderError};
use crate::types::overrides::{VariableKind, effective_superclass_variable_kind};
use crate::types::relation::{DisjointnessChecker, TypeRelationChecker};
use crate::types::visitor::any_over_type;
use crate::types::{TypeContext, UpcastPolicy};
Expand Down Expand Up @@ -1839,65 +1838,6 @@ impl<'a, 'db> ProtocolMember<'a, 'db> {
self.data.qualifiers
}

/// Returns whether an instance declaration conflicts with a required writable class variable.
///
/// An unannotated assignment preserves an inherited `ClassVar`; an explicit instance
/// annotation does not:
///
/// ```python
/// from typing import ClassVar
///
/// class Base:
/// value: ClassVar[int]
///
/// class Valid(Base):
/// value = 1
///
/// class Invalid(Base):
/// value: int = 1
/// ```
///
/// Inspect declarations before descriptor binding, and ignore synthesized members without
/// source provenance.
pub(super) fn has_incompatible_class_variable_declaration(
&self,
db: &'db dyn Db,
env: &ProgramEnvironment<'db>,
ty: Type<'db>,
) -> bool {
let qualifiers = self.qualifiers();
qualifiers.contains(TypeQualifiers::CLASS_VAR)
&& !qualifiers.contains(TypeQualifiers::FINAL)
&& ty
.nominal_class(db, env)
.or_else(|| {
if !is_class_object_type(ty) {
return None;
}

ty.to_meta_type(db, env)
.to_instance_approximation(db, env)?
.nominal_class(db, env)
})
.is_some_and(|class| {
effective_superclass_variable_kind(db, class, Name::new(self.name))
== Some(VariableKind::Instance)
&& [
class
.class_member(db, env, self.name, MemberLookupPolicy::default())
.place,
class.instance_member(db, env, self.name).place,
]
.into_iter()
.any(|place| {
matches!(
place,
Place::Defined(defined) if defined.provenance != Provenance::Unknown
)
})
})
}

fn is_method(&self) -> bool {
matches!(self.data.kind, ProtocolMemberKind::Method(..))
}
Expand Down Expand Up @@ -2965,18 +2905,6 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
required: ProtocolMemberAccess<'db>,
access: ProtocolMemberAccessMode,
) -> ConstraintSet<'db, 'c> {
if access == ProtocolMemberAccessMode::Class
&& member.has_incompatible_class_variable_declaration(db, self.env, ty)
{
if let Some(context) = self.report_context() {
context.push(ErrorContext::ProtocolMemberClassVarMismatch {
member_name: member.name.into(),
ty,
});
}
return self.never();
}

if access == ProtocolMemberAccessMode::Class
&& member.is_instance_method()
&& required.read.is_some()
Expand Down Expand Up @@ -3050,17 +2978,6 @@ impl<'c, 'db> TypeRelationChecker<'_, 'c, 'db> {
let instance_access =
member.implementation_access(db, env, ty, ProtocolMemberAccessMode::Instance);
if let Some(context) = self.report_context() {
if member.has_incompatible_class_variable_declaration(db, env, ty) {
context.push(ErrorContext::ProtocolMemberClassVarMismatch {
member_name: member.name.into(),
ty,
});
context.push(ErrorContext::ProtocolMemberIncompatible {
member_name: member.name.into(),
});
return self.never();
}

let instance_read_missing = instance_access.read.is_some()
&& protocol_member_read_type(
db,
Expand Down
8 changes: 0 additions & 8 deletions crates/ty_python_semantic/src/types/relation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2920,14 +2920,6 @@ impl<'a, 'c, 'db> DisjointnessChecker<'a, 'c, 'db> {
db, &member, other,
)
})
.or(db, self.constraints, || {
ConstraintSet::from_bool(
self.constraints,
member.has_incompatible_class_variable_declaration(
db, env, other,
),
)
})
})
})
}
Expand Down
9 changes: 0 additions & 9 deletions crates/ty_python_semantic/src/types/relation_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,6 @@ pub(crate) enum ErrorContext<'db> {
member_name: Name,
ty: Type<'db>,
},
ProtocolMemberClassVarMismatch {
member_name: Name,
ty: Type<'db>,
},
ProtocolSpecialMethodNotDefinedOnMetaType,
ProtocolMemberIncompatible {
member_name: Name,
Expand Down Expand Up @@ -453,11 +449,6 @@ impl<'db> ErrorContext<'db> {
"protocol member `{member_name}` is not defined on type `{}`",
ty.display(db, env),
),
Self::ProtocolMemberClassVarMismatch { member_name, ty } => format!(
"protocol member `{member_name}` is an instance variable on type `{}`, \
but a class variable is required",
ty.display(db, env),
),
Self::ProtocolSpecialMethodNotDefinedOnMetaType => {
"special methods must be defined on the meta-type when matching a protocol"
.to_string()
Expand Down
Loading