Skip to content

[ty] Disallow illegal uses of ClassVar #19483

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

Merged
merged 2 commits into from
Jul 22, 2025
Merged

[ty] Disallow illegal uses of ClassVar #19483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83bf4e9
[ty] Disallow illegal uses of `ClassVar`
sharkdp Jul 22, 2025
6f55887
Fix copy-paste error
sharkdp Jul 22, 2025
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
40 changes: 38 additions & 2 deletions crates/ty_python_semantic/resources/mdtest/type_qualifiers/classvar.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,49 @@ class C:
y: int | ClassVar[str]
```

## Used outside of a class
## Illegal positions

```toml
[environment]
python-version = "3.12"
```

```py
from typing import ClassVar

# TODO: this should be an error
# error: [invalid-type-form] "`ClassVar` annotations are only allowed in class-body scopes"
x: ClassVar[int] = 1

class C:
def __init__(self) -> None:
# error: [invalid-type-form] "`ClassVar` annotations are not allowed for non-name targets"
self.x: ClassVar[int] = 1
Comment on lines +103 to +105
Copy link
Member

@AlexWaygood AlexWaygood Jul 22, 2025

Choose a reason for hiding this comment

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

Does the spec explicitly forbid something like this?

class C:
    @classmethod
    def f(cls):
        cls.x: ClassVar[int] = 1

Copy link
Contributor Author

@sharkdp sharkdp Jul 22, 2025

Choose a reason for hiding this comment

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

Does the spec explicitly forbid something like this?

It does not explicitly forbid it. It does say "ClassVar may be used in one of several forms:" and then proceeds to show two examples where ClassVar is used at the class-body level.

Mypy, pyright and pyrefly all reject your example for various reasons (including: "ClassVar" is not allowed in this context).

I also didn't find any references of code like this in the ecosystem.

Copy link
Member

@AlexWaygood AlexWaygood Jul 22, 2025

Choose a reason for hiding this comment

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

I think it would be reasonable to support that, but I also don't think it's high priority at all, especially if no other type checker does.

Copy link
Contributor Author

@sharkdp sharkdp Jul 22, 2025

Choose a reason for hiding this comment

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

By "support that", you mean: "tolerate that"? Because I'm not sure what the intention here would be? When would a cls.x: … = 1 attribute not be a (pure) class variable?

Copy link
Member

@AlexWaygood AlexWaygood Jul 22, 2025

Choose a reason for hiding this comment

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

I guess if it were declared with x: int = 42 in the class body, then that would make it an instance attribute with a class-level default, and cls.x = 56 in a class method would be mutating the default value of the instance attribute? That's a bit suspect, though; I'd consider it at least a code smell.

And I suppose we'd want to emit an error if it were declared as an instance variable in an instance method or the class body, but declared as a class variable in a classmethod


# error: [invalid-type-form] "`ClassVar` annotations are only allowed in class-body scopes"
y: ClassVar[int] = 1

# error: [invalid-type-form] "`ClassVar` is not allowed in function parameter annotations"
def f(x: ClassVar[int]) -> None:
pass

# error: [invalid-type-form] "`ClassVar` is not allowed in function parameter annotations"
def f[T](x: ClassVar[T]) -> T:
return x

# error: [invalid-type-form] "`ClassVar` is not allowed in function return type annotations"
def f() -> ClassVar[int]:
return 1

# error: [invalid-type-form] "`ClassVar` is not allowed in function return type annotations"
def f[T](x: T) -> ClassVar[T]:
return x

# TODO: this should be an error
class Foo(ClassVar[tuple[int]]): ...

# TODO: Show `Unknown` instead of `@Todo` type in the MRO; or ignore `ClassVar` and show the MRO as if `ClassVar` was not there
# revealed: tuple[<class 'Foo'>, @Todo(Inference of subscript on special form), <class 'object'>]
reveal_type(Foo.__mro__)
```

[`typing.classvar`]: https://docs.python.org/3/library/typing.html#typing.ClassVar
4 changes: 2 additions & 2 deletions crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ def f(ILLEGAL: Final[int]) -> None:
pass

# error: [invalid-type-form] "`Final` is not allowed in function parameter annotations"
def f[T](ILLEGAL: Final[int]) -> None:
pass
def f[T](ILLEGAL: Final[T]) -> T:
return ILLEGAL

# error: [invalid-type-form] "`Final` is not allowed in function return type annotations"
def f() -> Final[None]: ...
Expand Down
68 changes: 55 additions & 13 deletions crates/ty_python_semantic/src/types/infer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2653,6 +2653,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
);
}
}
if annotated.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, returns) {
builder.into_diagnostic(
"`ClassVar` is not allowed in function return type annotations",
);
}
}
}
}

Expand Down Expand Up @@ -2691,9 +2698,20 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
DeferredExpressionState::None,
);

if annotated.is_some_and(|annotated| annotated.qualifiers.contains(TypeQualifiers::FINAL)) {
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, parameter) {
builder.into_diagnostic("`Final` is not allowed in function parameter annotations");
if let Some(qualifiers) = annotated.map(|annotated| annotated.qualifiers) {
if qualifiers.contains(TypeQualifiers::FINAL) {
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, parameter) {
builder.into_diagnostic(
"`Final` is not allowed in function parameter annotations",
);
}
}
if qualifiers.contains(TypeQualifiers::CLASS_VAR) {
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, parameter) {
builder.into_diagnostic(
"`ClassVar` is not allowed in function parameter annotations",
);
}
}
}
}
Expand Down Expand Up @@ -4240,6 +4258,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
} = assignment;
let annotated =
self.infer_annotation_expression(annotation, DeferredExpressionState::None);

if annotated.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
if let Some(builder) = self
.context
.report_lint(&INVALID_TYPE_FORM, annotation.as_ref())
{
builder.into_diagnostic(
"`ClassVar` annotations are not allowed for non-name targets",
);
}
}

if let Some(value) = value {
self.infer_maybe_standalone_expression(value);
}
Expand All @@ -4266,18 +4296,30 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
let target = assignment.target(self.module());
let value = assignment.value(self.module());

let mut declared_ty = self.infer_annotation_expression(
let mut declared = self.infer_annotation_expression(
annotation,
DeferredExpressionState::from(self.defer_annotations()),
);

if declared.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
let current_scope_id = self.scope().file_scope_id(self.db());
let current_scope = self.index.scope(current_scope_id);
if current_scope.kind() != ScopeKind::Class {
if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, annotation) {
builder.into_diagnostic(
"`ClassVar` annotations are only allowed in class-body scopes",
);
}
}
}

if target
.as_name_expr()
.is_some_and(|name| &name.id == "TYPE_CHECKING")
{
if !KnownClass::Bool
.to_instance(self.db())
.is_assignable_to(self.db(), declared_ty.inner_type())
.is_assignable_to(self.db(), declared.inner_type())
{
// annotation not assignable from `bool` is an error
report_invalid_type_checking_constant(&self.context, target.into());
Expand All @@ -4296,19 +4338,19 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
// otherwise, assigning something other than `False` is an error
report_invalid_type_checking_constant(&self.context, target.into());
}
declared_ty.inner = Type::BooleanLiteral(true);
declared.inner = Type::BooleanLiteral(true);
}

// Handle various singletons.
if let Type::NominalInstance(instance) = declared_ty.inner_type() {
if let Type::NominalInstance(instance) = declared.inner_type() {
if instance.class.is_known(self.db(), KnownClass::SpecialForm) {
if let Some(name_expr) = target.as_name_expr() {
if let Some(special_form) = SpecialFormType::try_from_file_and_name(
self.db(),
self.file(),
&name_expr.id,
) {
declared_ty.inner = Type::SpecialForm(special_form);
declared.inner = Type::SpecialForm(special_form);
}
}
}
Expand All @@ -4327,15 +4369,15 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
{
Type::BooleanLiteral(true)
} else if self.in_stub() && value.is_ellipsis_literal_expr() {
declared_ty.inner_type()
declared.inner_type()
} else {
inferred_ty
};
self.add_declaration_with_binding(
target.into(),
definition,
&DeclaredAndInferredType::MightBeDifferent {
declared_ty,
declared_ty: declared,
inferred_ty,
},
);
Expand All @@ -4346,13 +4388,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
self.add_declaration_with_binding(
target.into(),
definition,
&DeclaredAndInferredType::AreTheSame(declared_ty.inner_type()),
&DeclaredAndInferredType::AreTheSame(declared.inner_type()),
);
} else {
self.add_declaration(target.into(), definition, declared_ty);
self.add_declaration(target.into(), definition, declared);
}

self.store_expression_type(target, declared_ty.inner_type());
self.store_expression_type(target, declared.inner_type());
}
}

Expand Down
Loading