mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-03 15:14:42 +00:00
[ty] Disallow illegal uses of ClassVar
(#19483)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
## Summary It was faster to implement this then to write the ticket: Disallow `ClassVar` annotations almost everywhere outside of class body scopes. ## Test Plan New Markdown tests
This commit is contained in:
parent
9180cd094d
commit
6d4687c9af
3 changed files with 95 additions and 17 deletions
|
@ -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
|
||||
|
||||
# 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
|
||||
|
|
|
@ -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]: ...
|
||||
|
|
|
@ -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",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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);
|
||||
}
|
||||
|
@ -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());
|
||||
|
@ -4296,11 +4338,11 @@ 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(
|
||||
|
@ -4308,7 +4350,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
self.file(),
|
||||
&name_expr.id,
|
||||
) {
|
||||
declared_ty.inner = Type::SpecialForm(special_form);
|
||||
declared.inner = Type::SpecialForm(special_form);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -4327,7 +4369,7 @@ 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
|
||||
};
|
||||
|
@ -4335,7 +4377,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
target.into(),
|
||||
definition,
|
||||
&DeclaredAndInferredType::MightBeDifferent {
|
||||
declared_ty,
|
||||
declared_ty: declared,
|
||||
inferred_ty,
|
||||
},
|
||||
);
|
||||
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue