From fb15da56948171532c65d4e079453fda0cfce308 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 18 Jan 2025 13:51:35 +0100 Subject: [PATCH] [red-knot] Add support for `typing.ClassVar` (#15550) ## Summary Add support for `typing.ClassVar`, i.e. emit a diagnostic in this scenario: ```py from typing import ClassVar class C: x: ClassVar[int] = 1 c = C() c.x = 3 # error: "Cannot assign to pure class variable `x` from an instance of type `C`" ``` ## Test Plan - New tests for the `typing.ClassVar` qualifier - Fixed one TODO in `attributes.md` --- .../resources/mdtest/attributes.md | 6 +- .../mdtest/type_qualifiers/classvar.md | 93 ++++++ crates/red_knot_python_semantic/src/types.rs | 195 +++++++++--- .../src/types/diagnostic.rs | 39 ++- .../src/types/infer.rs | 280 +++++++++++++----- 5 files changed, 499 insertions(+), 114 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 24f0be14f3..2e352ed264 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -169,6 +169,10 @@ class C: pure_class_variable1: ClassVar[str] = "value in class body" pure_class_variable2: ClassVar = 1 + def method(self): + # TODO: this should be an error + self.pure_class_variable1 = "value set through instance" + reveal_type(C.pure_class_variable1) # revealed: str # TODO: this should be `Literal[1]`, or `Unknown | Literal[1]`. @@ -182,7 +186,7 @@ reveal_type(c_instance.pure_class_variable1) # revealed: str # TODO: Should be `Unknown | Literal[1]`. reveal_type(c_instance.pure_class_variable2) # revealed: Unknown -# TODO: should raise an error. It is not allowed to reassign a pure class variable on an instance. +# error: [invalid-attribute-access] "Cannot assign to ClassVar `pure_class_variable1` from an instance of type `C`" c_instance.pure_class_variable1 = "value set on instance" C.pure_class_variable1 = "overwritten on class" diff --git a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md new file mode 100644 index 0000000000..26060e685d --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -0,0 +1,93 @@ +# `typing.ClassVar` + +[`typing.ClassVar`] is a type qualifier that is used to indicate that a class variable may not be +written to from instances of that class. + +This test makes sure that we discover the type qualifier while inferring types from an annotation. +For more details on the semantics of pure class variables, see [this test](../attributes.md). + +## Basic + +```py +from typing import ClassVar, Annotated + +class C: + a: ClassVar[int] = 1 + b: Annotated[ClassVar[int], "the annotation for b"] = 1 + c: ClassVar[Annotated[int, "the annotation for c"]] = 1 + d: ClassVar = 1 + e: "ClassVar[int]" = 1 + +reveal_type(C.a) # revealed: int +reveal_type(C.b) # revealed: int +reveal_type(C.c) # revealed: int +# TODO: should be Unknown | Literal[1] +reveal_type(C.d) # revealed: Unknown +reveal_type(C.e) # revealed: int + +c = C() + +# error: [invalid-attribute-access] +c.a = 2 +# error: [invalid-attribute-access] +c.b = 2 +# error: [invalid-attribute-access] +c.c = 2 +# error: [invalid-attribute-access] +c.d = 2 +# error: [invalid-attribute-access] +c.e = 2 +``` + +## Conflicting type qualifiers + +We currently ignore conflicting qualifiers and simply union them, which is more conservative than +intersecting them. This means that we consider `a` to be a `ClassVar` here: + +```py +from typing import ClassVar + +def flag() -> bool: + return True + +class C: + if flag(): + a: ClassVar[int] = 1 + else: + a: str + +reveal_type(C.a) # revealed: int | str + +c = C() + +# error: [invalid-attribute-access] +c.a = 2 +``` + +## Too many arguments + +```py +class C: + # error: [invalid-type-form] "Type qualifier `typing.ClassVar` expects exactly one type parameter" + x: ClassVar[int, str] = 1 +``` + +## Illegal `ClassVar` in type expression + +```py +class C: + # error: [invalid-type-form] "Type qualifier `typing.ClassVar` is not allowed in type expressions (only in annotation expressions)" + x: ClassVar | int + + # error: [invalid-type-form] "Type qualifier `typing.ClassVar` is not allowed in type expressions (only in annotation expressions)" + y: int | ClassVar[str] +``` + +## Used outside of a class + +```py +# TODO: this should be an error +x: ClassVar[int] = 1 +``` + +[`typing.classvar`]: https://docs.python.org/3/library/typing.html#typing.ClassVar diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 0c3998976c..f8985ba336 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -1,6 +1,7 @@ use std::hash::Hash; use std::iter; +use bitflags::bitflags; use context::InferContext; use diagnostic::{report_not_iterable, report_not_iterable_possibly_unbound}; use indexmap::IndexSet; @@ -92,7 +93,7 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> // on inference from bindings. let declarations = use_def.public_declarations(symbol); - let declared = declarations_ty(db, declarations); + let declared = declarations_ty(db, declarations).map(|SymbolAndQualifiers(ty, _)| ty); match declared { // Symbol is declared, trust the declared type @@ -126,7 +127,7 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> Err((declared_ty, _)) => { // Intentionally ignore conflicting declared types; that's not our problem, // it's the problem of the module we are importing from. - declared_ty.into() + declared_ty.inner_ty().into() } } @@ -246,7 +247,7 @@ pub(crate) fn binding_ty<'db>(db: &'db dyn Db, definition: Definition<'db>) -> T } /// Infer the type of a declaration. -fn declaration_ty<'db>(db: &'db dyn Db, definition: Definition<'db>) -> Type<'db> { +fn declaration_ty<'db>(db: &'db dyn Db, definition: Definition<'db>) -> TypeAndQualifiers<'db> { let inference = infer_definition_types(db, definition); inference.declaration_ty(definition) } @@ -356,22 +357,50 @@ fn bindings_ty<'db>( } } +/// A type with declaredness information, and a set of type qualifiers. +/// +/// This is used to represent the result of looking up the declared type. Consider this +/// example: +/// ```py +/// class C: +/// if flag: +/// variable: ClassVar[int] +/// ``` +/// If we look up the declared type of `variable` in the scope of class `C`, we will get +/// the type `int`, a "declaredness" of [`Boundness::PossiblyUnbound`], and the information +/// that this comes with a [`TypeQualifiers::CLASS_VAR`] type qualifier. +pub(crate) struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers); + +impl SymbolAndQualifiers<'_> { + fn is_class_var(&self) -> bool { + self.1.contains(TypeQualifiers::CLASS_VAR) + } +} + +impl<'db> From> for SymbolAndQualifiers<'db> { + fn from(symbol: Symbol<'db>) -> Self { + SymbolAndQualifiers(symbol, TypeQualifiers::empty()) + } +} + +impl<'db> From> for SymbolAndQualifiers<'db> { + fn from(ty: Type<'db>) -> Self { + SymbolAndQualifiers(ty.into(), TypeQualifiers::empty()) + } +} + /// The result of looking up a declared type from declarations; see [`declarations_ty`]. -type DeclaredTypeResult<'db> = Result, (Type<'db>, Box<[Type<'db>]>)>; +type DeclaredTypeResult<'db> = + Result, (TypeAndQualifiers<'db>, Box<[Type<'db>]>)>; /// Build a declared type from a [`DeclarationsIterator`]. /// /// If there is only one declaration, or all declarations declare the same type, returns -/// `Ok(declared_type)`. If there are conflicting declarations, returns -/// `Err((union_of_declared_types, conflicting_declared_types))`. +/// `Ok(..)`. If there are conflicting declarations, returns an `Err(..)` variant with +/// a union of the declared types as well as a list of all conflicting types. /// -/// If undeclared is a possibility, `undeclared_ty` type will be part of the return type but it -/// will not be considered to be conflicting with any other types. -/// -/// # Panics -/// Will panic if there are no declarations and no `undeclared_ty` is provided. This is a logic -/// error, as any symbol with zero live declarations clearly must be undeclared, and the caller -/// should provide an `undeclared_ty`. +/// This function also returns declaredness information (see [`Symbol`]) and a set of +/// [`TypeQualifiers`] that have been specified on the declaration(s). fn declarations_ty<'db>( db: &'db dyn Db, declarations: DeclarationsIterator<'_, 'db>, @@ -408,14 +437,19 @@ fn declarations_ty<'db>( if let Some(first) = types.next() { let mut conflicting: Vec> = vec![]; let declared_ty = if let Some(second) = types.next() { - let mut builder = UnionBuilder::new(db).add(first); + let ty_first = first.inner_ty(); + let mut qualifiers = first.qualifiers(); + + let mut builder = UnionBuilder::new(db).add(ty_first); for other in std::iter::once(second).chain(types) { - if !first.is_equivalent_to(db, other) { - conflicting.push(other); + let other_ty = other.inner_ty(); + if !ty_first.is_equivalent_to(db, other_ty) { + conflicting.push(other_ty); } - builder = builder.add(other); + builder = builder.add(other_ty); + qualifiers = qualifiers.union(other.qualifiers()); } - builder.build() + TypeAndQualifiers::new(builder.build(), qualifiers) } else { first }; @@ -428,15 +462,20 @@ fn declarations_ty<'db>( Truthiness::Ambiguous => Boundness::PossiblyUnbound, }; - Ok(Symbol::Type(declared_ty, boundness)) + Ok(SymbolAndQualifiers( + Symbol::Type(declared_ty.inner_ty(), boundness), + declared_ty.qualifiers(), + )) } else { Err(( declared_ty, - std::iter::once(first).chain(conflicting).collect(), + std::iter::once(first.inner_ty()) + .chain(conflicting) + .collect(), )) } } else { - Ok(Symbol::Unbound) + Ok(Symbol::Unbound.into()) } } @@ -1661,7 +1700,10 @@ impl<'db> Type<'db> { (Some(KnownClass::VersionInfo), "minor") => { Type::IntLiteral(Program::get(db).python_version(db).minor.into()).into() } - _ => class.instance_member(db, name), + _ => { + let SymbolAndQualifiers(symbol, _) = class.instance_member(db, name); + symbol + } }, Type::Union(union) => { @@ -2285,11 +2327,18 @@ impl<'db> Type<'db> { invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareAnnotated], fallback_type: Type::unknown(), }), - Type::KnownInstance(KnownInstanceType::ClassVar) => { - // TODO: A bare `ClassVar` should rather be treated as if the symbol was not - // declared at all. - Ok(Type::unknown()) - } + Type::KnownInstance(KnownInstanceType::ClassVar) => Err(InvalidTypeExpressionError { + invalid_expressions: smallvec::smallvec![ + InvalidTypeExpression::ClassVarInTypeExpression + ], + fallback_type: Type::unknown(), + }), + Type::KnownInstance(KnownInstanceType::Final) => Err(InvalidTypeExpressionError { + invalid_expressions: smallvec::smallvec![ + InvalidTypeExpression::FinalInTypeExpression + ], + fallback_type: Type::unknown(), + }), Type::KnownInstance(KnownInstanceType::Literal) => Err(InvalidTypeExpressionError { invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareLiteral], fallback_type: Type::unknown(), @@ -2466,6 +2515,60 @@ impl std::fmt::Display for DynamicType { } } +bitflags! { + /// Type qualifiers that appear in an annotation expression. + #[derive(Copy, Clone, Debug, Eq, PartialEq)] + pub(crate) struct TypeQualifiers: u8 { + /// `typing.ClassVar` + const CLASS_VAR = 1 << 0; + /// `typing.Final` + const FINAL = 1 << 1; + } +} + +/// When inferring the type of an annotation expression, we can also encounter type qualifiers +/// such as `ClassVar` or `Final`. These do not affect the inferred type itself, but rather +/// control how a particular symbol can be accessed or modified. This struct holds a type and +/// a set of type qualifiers. +/// +/// Example: `Annotated[ClassVar[tuple[int]], "metadata"]` would have type `tuple[int]` and the +/// qualifier `ClassVar`. +#[derive(Clone, Debug, Copy, Eq, PartialEq)] +pub(crate) struct TypeAndQualifiers<'db> { + inner: Type<'db>, + qualifiers: TypeQualifiers, +} + +impl<'db> TypeAndQualifiers<'db> { + pub(crate) fn new(inner: Type<'db>, qualifiers: TypeQualifiers) -> Self { + Self { inner, qualifiers } + } + + /// Forget about type qualifiers and only return the inner type. + pub(crate) fn inner_ty(&self) -> Type<'db> { + self.inner + } + + /// Insert/add an additional type qualifier. + pub(crate) fn add_qualifier(&mut self, qualifier: TypeQualifiers) { + self.qualifiers |= qualifier; + } + + /// Return the set of type qualifiers. + pub(crate) fn qualifiers(&self) -> TypeQualifiers { + self.qualifiers + } +} + +impl<'db> From> for TypeAndQualifiers<'db> { + fn from(inner: Type<'db>) -> Self { + Self { + inner, + qualifiers: TypeQualifiers::empty(), + } + } +} + /// Error struct providing information on type(s) that were deemed to be invalid /// in a type expression context, and the type we should therefore fallback to /// for the problematic type expression. @@ -2499,6 +2602,10 @@ enum InvalidTypeExpression { BareAnnotated, /// `x: Literal` is invalid as an annotation BareLiteral, + /// The `ClassVar` type qualifier was used in a type expression + ClassVarInTypeExpression, + /// The `Final` type qualifier was used in a type expression + FinalInTypeExpression, } impl InvalidTypeExpression { @@ -2506,6 +2613,8 @@ impl InvalidTypeExpression { match self { Self::BareAnnotated => "`Annotated` requires at least two arguments when used in an annotation or type expression", Self::BareLiteral => "`Literal` requires at least one argument when used in a type expression", + Self::ClassVarInTypeExpression => "Type qualifier `typing.ClassVar` is not allowed in type expressions (only in annotation expressions)", + Self::FinalInTypeExpression => "Type qualifier `typing.Final` is not allowed in type expressions (only in annotation expressions)", } } } @@ -3980,15 +4089,16 @@ impl<'db> Class<'db> { /// defined attribute that is only present in a method (typically `__init__`). /// /// The attribute might also be defined in a superclass of this class. - pub(crate) fn instance_member(self, db: &'db dyn Db, name: &str) -> Symbol<'db> { + pub(crate) fn instance_member(self, db: &'db dyn Db, name: &str) -> SymbolAndQualifiers<'db> { for superclass in self.iter_mro(db) { match superclass { ClassBase::Dynamic(_) => { return todo_type!("instance attribute on class with dynamic base").into(); } ClassBase::Class(class) => { - let member = class.own_instance_member(db, name); - if !member.is_unbound() { + if let member @ SymbolAndQualifiers(Symbol::Type(_, _), _) = + class.own_instance_member(db, name) + { return member; } } @@ -4002,7 +4112,7 @@ impl<'db> Class<'db> { /// A helper function for `instance_member` that looks up the `name` attribute only on /// this class, not on its superclasses. - fn own_instance_member(self, db: &'db dyn Db, name: &str) -> Symbol<'db> { + fn own_instance_member(self, db: &'db dyn Db, name: &str) -> SymbolAndQualifiers<'db> { // TODO: There are many things that are not yet implemented here: // - `typing.ClassVar` and `typing.Final` // - Proper diagnostics @@ -4018,7 +4128,7 @@ impl<'db> Class<'db> { let declarations = use_def.public_declarations(symbol); match declarations_ty(db, declarations) { - Ok(Symbol::Type(declared_ty, _)) => { + Ok(SymbolAndQualifiers(Symbol::Type(declared_ty, _), qualifiers)) => { if let Some(function) = declared_ty.into_function_literal() { // TODO: Eventually, we are going to process all decorators correctly. This is // just a temporary heuristic to provide a broad categorization into properties @@ -4029,28 +4139,31 @@ impl<'db> Class<'db> { todo_type!("bound method").into() } } else { - Symbol::Type(declared_ty, Boundness::Bound) + SymbolAndQualifiers(Symbol::Type(declared_ty, Boundness::Bound), qualifiers) } } - Ok(Symbol::Unbound) => { + Ok(SymbolAndQualifiers(Symbol::Unbound, qualifiers)) => { let bindings = use_def.public_bindings(symbol); let inferred_ty = bindings_ty(db, bindings); match inferred_ty { - Symbol::Type(ty, _) => Symbol::Type( - UnionType::from_elements(db, [Type::unknown(), ty]), - Boundness::Bound, + Symbol::Type(ty, _) => SymbolAndQualifiers( + Symbol::Type( + UnionType::from_elements(db, [Type::unknown(), ty]), + Boundness::Bound, + ), + qualifiers, ), - Symbol::Unbound => Symbol::Unbound, + Symbol::Unbound => SymbolAndQualifiers(Symbol::Unbound, qualifiers), } } - Err((declared_ty, _)) => { + Err((declared_ty, _conflicting_declarations)) => { // Ignore conflicting declarations - declared_ty.into() + SymbolAndQualifiers(declared_ty.inner_ty().into(), declared_ty.qualifiers()) } } } else { - Symbol::Unbound + Symbol::Unbound.into() } } diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 80e8095025..9699197527 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -1,5 +1,4 @@ use super::context::InferContext; -use crate::declare_lint; use crate::lint::{Level, LintRegistryBuilder, LintStatus}; use crate::suppression::FileSuppressionId; use crate::types::string_annotation::{ @@ -7,7 +6,8 @@ use crate::types::string_annotation::{ IMPLICIT_CONCATENATED_STRING_TYPE_ANNOTATION, INVALID_SYNTAX_IN_FORWARD_ANNOTATION, RAW_STRING_TYPE_ANNOTATION, }; -use crate::types::{ClassLiteralType, Type}; +use crate::types::{ClassLiteralType, KnownInstanceType, Type}; +use crate::{declare_lint, Db}; use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity}; use ruff_db::files::File; use ruff_python_ast::{self as ast, AnyNodeRef}; @@ -59,6 +59,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&UNSUPPORTED_OPERATOR); registry.register_lint(&ZERO_STEPSIZE_IN_SLICE); registry.register_lint(&STATIC_ASSERT_ERROR); + registry.register_lint(&INVALID_ATTRIBUTE_ACCESS); // String annotations registry.register_lint(&BYTE_STRING_TYPE_ANNOTATION); @@ -750,6 +751,25 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Makes sure that instance attribute accesses are valid. + /// + /// ## Examples + /// ```python + /// class C: + /// var: ClassVar[int] = 1 + /// + /// C.var = 3 # okay + /// C().var = 3 # error: Cannot assign to class variable + /// ``` + pub(crate) static INVALID_ATTRIBUTE_ACCESS = { + summary: "Invalid attribute access", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + #[derive(Debug, Eq, PartialEq, Clone)] pub struct TypeCheckDiagnostic { pub(crate) id: DiagnosticId, @@ -1060,3 +1080,18 @@ pub(crate) fn report_base_with_incompatible_slots(context: &InferContext, node: format_args!("Class base has incompatible `__slots__`"), ); } + +pub(crate) fn report_invalid_arguments_to_annotated<'db>( + db: &'db dyn Db, + context: &InferContext<'db>, + subscript: &ast::ExprSubscript, +) { + context.report_lint( + &INVALID_TYPE_FORM, + subscript.into(), + format_args!( + "Special form `{}` expected at least 2 arguments (one type and at least one metadata element)", + KnownInstanceType::Annotated.repr(db) + ), + ); +} diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 17bf3caabb..e34790f111 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -51,9 +51,10 @@ use crate::semantic_index::SemanticIndex; use crate::stdlib::builtins_module_scope; use crate::types::call::{Argument, CallArguments}; use crate::types::diagnostic::{ - report_invalid_assignment, report_unresolved_module, TypeCheckDiagnostics, CALL_NON_CALLABLE, - CALL_POSSIBLY_UNBOUND_METHOD, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, - CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_BASE, + report_invalid_arguments_to_annotated, report_invalid_assignment, report_unresolved_module, + TypeCheckDiagnostics, CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD, + CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, + DUPLICATE_BASE, INCONSISTENT_MRO, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE, INVALID_CONTEXT_MANAGER, INVALID_DECLARATION, INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, INVALID_TYPE_VARIABLE_CONSTRAINTS, POSSIBLY_UNBOUND_ATTRIBUTE, POSSIBLY_UNBOUND_IMPORT, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT, UNSUPPORTED_OPERATOR, @@ -65,8 +66,9 @@ use crate::types::{ typing_extensions_symbol, Boundness, CallDunderResult, Class, ClassLiteralType, DynamicType, FunctionType, InstanceType, IntersectionBuilder, IntersectionType, IterationOutcome, KnownClass, KnownFunction, KnownInstanceType, MetaclassCandidate, MetaclassErrorKind, - SliceLiteralType, SubclassOfType, Symbol, Truthiness, TupleType, Type, TypeAliasType, - TypeArrayDisplay, TypeVarBoundOrConstraints, TypeVarInstance, UnionBuilder, UnionType, + SliceLiteralType, SubclassOfType, Symbol, SymbolAndQualifiers, Truthiness, TupleType, Type, + TypeAliasType, TypeAndQualifiers, TypeArrayDisplay, TypeQualifiers, TypeVarBoundOrConstraints, + TypeVarInstance, UnionBuilder, UnionType, }; use crate::unpack::Unpack; use crate::util::subscript::{PyIndex, PySlice}; @@ -113,7 +115,7 @@ fn infer_definition_types_cycle_recovery<'db>( let mut inference = TypeInference::empty(input.scope(db)); let category = input.category(db); if category.is_declaration() { - inference.declarations.insert(input, Type::unknown()); + inference.declarations.insert(input, Type::unknown().into()); } if category.is_binding() { inference.bindings.insert(input, Type::unknown()); @@ -241,8 +243,8 @@ pub(crate) struct TypeInference<'db> { /// The types of every binding in this region. bindings: FxHashMap, Type<'db>>, - /// The types of every declaration in this region. - declarations: FxHashMap, Type<'db>>, + /// The types and type qualifiers of every declaration in this region. + declarations: FxHashMap, TypeAndQualifiers<'db>>, /// The definitions that are deferred. deferred: FxHashSet>, @@ -281,7 +283,7 @@ impl<'db> TypeInference<'db> { } #[track_caller] - pub(crate) fn declaration_ty(&self, definition: Definition<'db>) -> Type<'db> { + pub(crate) fn declaration_ty(&self, definition: Definition<'db>) -> TypeAndQualifiers<'db> { self.declarations[&definition] } @@ -318,7 +320,7 @@ enum DeclaredAndInferredType<'db> { AreTheSame(Type<'db>), /// Declared and inferred types might be different, we need to check assignability. MightBeDifferent { - declared_ty: Type<'db>, + declared_ty: TypeAndQualifiers<'db>, inferred_ty: Type<'db>, }, } @@ -563,7 +565,9 @@ impl<'db> TypeInferenceBuilder<'db> { .filter_map(|(definition, ty)| { // Filter out class literals that result from imports if let DefinitionKind::Class(class) = definition.kind(self.db()) { - ty.into_class_literal().map(|ty| (ty.class, class.node())) + ty.inner_ty() + .into_class_literal() + .map(|ty| (ty.class, class.node())) } else { None } @@ -855,7 +859,7 @@ impl<'db> TypeInferenceBuilder<'db> { let declarations = use_def.declarations_at_binding(binding); let mut bound_ty = ty; let declared_ty = declarations_ty(self.db(), declarations) - .map(|s| s.ignore_possibly_unbound().unwrap_or(Type::unknown())) + .map(|SymbolAndQualifiers(s, _)| s.ignore_possibly_unbound().unwrap_or(Type::unknown())) .unwrap_or_else(|(ty, conflicting)| { // TODO point out the conflicting declarations in the diagnostic? let symbol_table = self.index.symbol_table(binding.file_scope(self.db())); @@ -868,7 +872,7 @@ impl<'db> TypeInferenceBuilder<'db> { conflicting.display(self.db()) ), ); - ty + ty.inner_ty() }); if !bound_ty.is_assignable_to(self.db(), declared_ty) { report_invalid_assignment(&self.context, node, declared_ty, bound_ty); @@ -879,7 +883,12 @@ impl<'db> TypeInferenceBuilder<'db> { self.types.bindings.insert(binding, bound_ty); } - fn add_declaration(&mut self, node: AnyNodeRef, declaration: Definition<'db>, ty: Type<'db>) { + fn add_declaration( + &mut self, + node: AnyNodeRef, + declaration: Definition<'db>, + ty: TypeAndQualifiers<'db>, + ) { debug_assert!(declaration.is_declaration(self.db())); let use_def = self.index.use_def_map(declaration.file_scope(self.db())); let prior_bindings = use_def.bindings_at_declaration(declaration); @@ -887,7 +896,7 @@ impl<'db> TypeInferenceBuilder<'db> { let inferred_ty = bindings_ty(self.db(), prior_bindings) .ignore_possibly_unbound() .unwrap_or(Type::Never); - let ty = if inferred_ty.is_assignable_to(self.db(), ty) { + let ty = if inferred_ty.is_assignable_to(self.db(), ty.inner_ty()) { ty } else { self.context.report_lint( @@ -895,11 +904,11 @@ impl<'db> TypeInferenceBuilder<'db> { node, format_args!( "Cannot declare type `{}` for inferred type `{}`", - ty.display(self.db()), + ty.inner_ty().display(self.db()), inferred_ty.display(self.db()) ), ); - Type::unknown() + Type::unknown().into() }; self.types.declarations.insert(declaration, ty); } @@ -913,23 +922,28 @@ impl<'db> TypeInferenceBuilder<'db> { debug_assert!(definition.is_binding(self.db())); debug_assert!(definition.is_declaration(self.db())); - let (declared_ty, inferred_ty) = match declared_and_inferred_ty { - DeclaredAndInferredType::AreTheSame(ty) => (ty, ty), + let (declared_ty, inferred_ty) = match *declared_and_inferred_ty { + DeclaredAndInferredType::AreTheSame(ty) => (ty.into(), ty), DeclaredAndInferredType::MightBeDifferent { declared_ty, inferred_ty, } => { - if inferred_ty.is_assignable_to(self.db(), *declared_ty) { + if inferred_ty.is_assignable_to(self.db(), declared_ty.inner_ty()) { (declared_ty, inferred_ty) } else { - report_invalid_assignment(&self.context, node, *declared_ty, *inferred_ty); + report_invalid_assignment( + &self.context, + node, + declared_ty.inner_ty(), + inferred_ty, + ); // if the assignment is invalid, fall back to assuming the annotation is correct - (declared_ty, declared_ty) + (declared_ty, declared_ty.inner_ty()) } } }; - self.types.declarations.insert(definition, *declared_ty); - self.types.bindings.insert(definition, *inferred_ty); + self.types.declarations.insert(definition, declared_ty); + self.types.bindings.insert(definition, inferred_ty); } fn add_unknown_declaration_with_binding( @@ -1220,7 +1234,7 @@ impl<'db> TypeInferenceBuilder<'db> { let declared_and_inferred_ty = if let Some(default_ty) = default_ty { if default_ty.is_assignable_to(self.db(), declared_ty) { DeclaredAndInferredType::MightBeDifferent { - declared_ty, + declared_ty: declared_ty.into(), inferred_ty: UnionType::from_elements(self.db(), [declared_ty, default_ty]), } } else if self.in_stub() @@ -2066,7 +2080,7 @@ impl<'db> TypeInferenceBuilder<'db> { ); // Handle various singletons. - if let Type::Instance(InstanceType { class }) = declared_ty { + if let Type::Instance(InstanceType { class }) = declared_ty.inner_ty() { if class.is_known(self.db(), KnownClass::SpecialForm) { if let Some(name_expr) = target.as_name_expr() { if let Some(known_instance) = KnownInstanceType::try_from_file_and_name( @@ -2074,7 +2088,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.file(), &name_expr.id, ) { - declared_ty = Type::KnownInstance(known_instance); + declared_ty.inner = Type::KnownInstance(known_instance); } } } @@ -2083,7 +2097,7 @@ impl<'db> TypeInferenceBuilder<'db> { if let Some(value) = value.as_deref() { let inferred_ty = self.infer_expression(value); let inferred_ty = if self.in_stub() && value.is_ellipsis_literal_expr() { - declared_ty + declared_ty.inner_ty() } else { inferred_ty }; @@ -3391,14 +3405,33 @@ impl<'db> TypeInferenceBuilder<'db> { fn infer_attribute_expression(&mut self, attribute: &ast::ExprAttribute) -> Type<'db> { let ast::ExprAttribute { value, - attr: _, + attr, range: _, ctx, } = attribute; match ctx { ExprContext::Load => self.infer_attribute_load(attribute), - ExprContext::Store | ExprContext::Del => { + ExprContext::Store => { + let value_ty = self.infer_expression(value); + + if let Type::Instance(instance) = value_ty { + let instance_member = instance.class.instance_member(self.db(), attr); + if instance_member.is_class_var() { + self.context.report_lint( + &INVALID_ATTRIBUTE_ACCESS, + attribute.into(), + format_args!( + "Cannot assign to ClassVar `{attr}` from an instance of type `{ty}`", + ty = value_ty.display(self.db()), + ), + ); + } + } + + Type::Never + } + ExprContext::Del => { self.infer_expression(value); Type::Never } @@ -4698,7 +4731,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, annotation: &ast::Expr, deferred_state: DeferredExpressionState, - ) -> Type<'db> { + ) -> TypeAndQualifiers<'db> { let previous_deferred_state = std::mem::replace(&mut self.deferred_state, deferred_state); let annotation_ty = self.infer_annotation_expression_impl(annotation); self.deferred_state = previous_deferred_state; @@ -4713,21 +4746,24 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, annotation: Option<&ast::Expr>, deferred_state: DeferredExpressionState, - ) -> Option> { + ) -> Option> { annotation.map(|expr| self.infer_annotation_expression(expr, deferred_state)) } /// Implementation of [`infer_annotation_expression`]. /// /// [`infer_annotation_expression`]: TypeInferenceBuilder::infer_annotation_expression - fn infer_annotation_expression_impl(&mut self, annotation: &ast::Expr) -> Type<'db> { + fn infer_annotation_expression_impl( + &mut self, + annotation: &ast::Expr, + ) -> TypeAndQualifiers<'db> { // https://typing.readthedocs.io/en/latest/spec/annotations.html#grammar-token-expression-grammar-annotation_expression let annotation_ty = match annotation { // String annotations: https://typing.readthedocs.io/en/latest/spec/annotations.html#string-annotations ast::Expr::StringLiteral(string) => self.infer_string_annotation_expression(string), // Annotation expressions also get special handling for `*args` and `**kwargs`. - ast::Expr::Starred(starred) => self.infer_starred_expression(starred), + ast::Expr::Starred(starred) => self.infer_starred_expression(starred).into(), ast::Expr::BytesLiteral(bytes) => { self.context.report_lint( @@ -4735,7 +4771,7 @@ impl<'db> TypeInferenceBuilder<'db> { bytes.into(), format_args!("Type expressions cannot use bytes literal"), ); - Type::unknown() + Type::unknown().into() } ast::Expr::FString(fstring) => { @@ -4745,21 +4781,126 @@ impl<'db> TypeInferenceBuilder<'db> { format_args!("Type expressions cannot use f-strings"), ); self.infer_fstring_expression(fstring); - Type::unknown() + Type::unknown().into() + } + + ast::Expr::Name(name) => match name.ctx { + ast::ExprContext::Load => { + let name_expr_ty = self.infer_name_expression(name); + match name_expr_ty { + Type::KnownInstance(KnownInstanceType::ClassVar) => { + TypeAndQualifiers::new(Type::unknown(), TypeQualifiers::CLASS_VAR) + } + Type::KnownInstance(KnownInstanceType::Final) => { + TypeAndQualifiers::new(Type::unknown(), TypeQualifiers::FINAL) + } + _ => name_expr_ty + .in_type_expression(self.db()) + .unwrap_or_else(|error| { + error.into_fallback_type(&self.context, annotation) + }) + .into(), + } + } + ast::ExprContext::Invalid => Type::unknown().into(), + ast::ExprContext::Store | ast::ExprContext::Del => todo_type!().into(), + }, + + ast::Expr::Subscript(subscript @ ast::ExprSubscript { value, slice, .. }) => { + let value_ty = self.infer_expression(value); + + let slice = &**slice; + + match value_ty { + Type::KnownInstance(KnownInstanceType::Annotated) => { + // This branch is similar to the corresponding branch in `infer_parameterized_known_instance_type_expression`, but + // `Annotated[…]` can appear both in annotation expressions and in type expressions, and needs to be handled slightly + // differently in each case (calling either `infer_type_expression_*` or `infer_annotation_expression_*`). + if let ast::Expr::Tuple(ast::ExprTuple { + elts: arguments, .. + }) = slice + { + if arguments.len() < 2 { + report_invalid_arguments_to_annotated( + self.db(), + &self.context, + subscript, + ); + } + + if let [inner_annotation, metadata @ ..] = &arguments[..] { + for element in metadata { + self.infer_expression(element); + } + + let inner_annotation_ty = + self.infer_annotation_expression_impl(inner_annotation); + + self.store_expression_type(slice, inner_annotation_ty.inner_ty()); + inner_annotation_ty + } else { + self.infer_type_expression(slice); + Type::unknown().into() + } + } else { + report_invalid_arguments_to_annotated( + self.db(), + &self.context, + subscript, + ); + self.infer_annotation_expression_impl(slice) + } + } + Type::KnownInstance( + known_instance @ (KnownInstanceType::ClassVar | KnownInstanceType::Final), + ) => match slice { + ast::Expr::Tuple(..) => { + self.context.report_lint( + &INVALID_TYPE_FORM, + subscript.into(), + format_args!( + "Type qualifier `{type_qualifier}` expects exactly one type parameter", + type_qualifier = known_instance.repr(self.db()), + ), + ); + Type::unknown().into() + } + _ => { + let mut type_and_qualifiers = + self.infer_annotation_expression_impl(slice); + match known_instance { + KnownInstanceType::ClassVar => { + type_and_qualifiers.add_qualifier(TypeQualifiers::CLASS_VAR); + } + KnownInstanceType::Final => { + type_and_qualifiers.add_qualifier(TypeQualifiers::FINAL); + } + _ => unreachable!(), + } + type_and_qualifiers + } + }, + _ => self + .infer_subscript_type_expression_no_store(subscript, slice, value_ty) + .into(), + } } // All other annotation expressions are (possibly) valid type expressions, so handle // them there instead. - type_expr => self.infer_type_expression_no_store(type_expr), + type_expr => self.infer_type_expression_no_store(type_expr).into(), }; - self.store_expression_type(annotation, annotation_ty); + self.store_expression_type(annotation, annotation_ty.inner_ty()); annotation_ty } /// Infer the type of a string annotation expression. - fn infer_string_annotation_expression(&mut self, string: &ast::ExprStringLiteral) -> Type<'db> { + fn infer_string_annotation_expression( + &mut self, + string: &ast::ExprStringLiteral, + ) -> TypeAndQualifiers<'db> { match parse_string_annotation(&self.context, string) { Some(parsed) => { // String annotations are always evaluated in the deferred context. @@ -4768,7 +4909,7 @@ impl<'db> TypeInferenceBuilder<'db> { DeferredExpressionState::InStringAnnotation, ) } - None => Type::unknown(), + None => Type::unknown().into(), } } } @@ -4855,16 +4996,7 @@ impl<'db> TypeInferenceBuilder<'db> { let value_ty = self.infer_expression(value); - match value_ty { - Type::ClassLiteral(class_literal_ty) => { - match class_literal_ty.class.known(self.db()) { - Some(KnownClass::Tuple) => self.infer_tuple_type_expression(slice), - Some(KnownClass::Type) => self.infer_subclass_of_type_expression(slice), - _ => self.infer_subscript_type_expression(subscript, value_ty), - } - } - _ => self.infer_subscript_type_expression(subscript, value_ty), - } + self.infer_subscript_type_expression_no_store(subscript, slice, value_ty) } ast::Expr::BinOp(binary) => { @@ -4980,6 +5112,22 @@ impl<'db> TypeInferenceBuilder<'db> { } } + fn infer_subscript_type_expression_no_store( + &mut self, + subscript: &ast::ExprSubscript, + slice: &ast::Expr, + value_ty: Type<'db>, + ) -> Type<'db> { + match value_ty { + Type::ClassLiteral(class_literal_ty) => match class_literal_ty.class.known(self.db()) { + Some(KnownClass::Tuple) => self.infer_tuple_type_expression(slice), + Some(KnownClass::Type) => self.infer_subclass_of_type_expression(slice), + _ => self.infer_subscript_type_expression(subscript, value_ty), + }, + _ => self.infer_subscript_type_expression(subscript, value_ty), + } + } + /// Infer the type of a string type expression. fn infer_string_type_expression(&mut self, string: &ast::ExprStringLiteral) -> Type<'db> { match parse_string_annotation(&self.context, string) { @@ -5163,22 +5311,11 @@ impl<'db> TypeInferenceBuilder<'db> { let arguments_slice = &*subscript.slice; match known_instance { KnownInstanceType::Annotated => { - let report_invalid_arguments = || { - self.context.report_lint( - &INVALID_TYPE_FORM, - subscript.into(), - format_args!( - "Special form `{}` expected at least 2 arguments (one type and at least one metadata element)", - known_instance.repr(self.db()) - ), - ); - }; - let ast::Expr::Tuple(ast::ExprTuple { elts: arguments, .. }) = arguments_slice else { - report_invalid_arguments(); + report_invalid_arguments_to_annotated(self.db(), &self.context, subscript); // `Annotated[]` with less than two arguments is an error at runtime. // However, we still treat `Annotated[T]` as `T` here for the purpose of @@ -5188,7 +5325,7 @@ impl<'db> TypeInferenceBuilder<'db> { }; if arguments.len() < 2 { - report_invalid_arguments(); + report_invalid_arguments_to_annotated(self.db(), &self.context, subscript); } let [type_expr, metadata @ ..] = &arguments[..] else { @@ -5345,13 +5482,16 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_type_expression(arguments_slice); todo_type!("`NotRequired[]` type qualifier") } - KnownInstanceType::ClassVar => { - let ty = self.infer_type_expression(arguments_slice); - ty - } - KnownInstanceType::Final => { - self.infer_type_expression(arguments_slice); - todo_type!("`Final[]` type qualifier") + KnownInstanceType::ClassVar | KnownInstanceType::Final => { + self.context.report_lint( + &INVALID_TYPE_FORM, + subscript.into(), + format_args!( + "Type qualifier `{}` is not allowed in type expressions (only in annotation expressions)", + known_instance.repr(self.db()) + ), + ); + self.infer_type_expression(arguments_slice) } KnownInstanceType::Required => { self.infer_type_expression(arguments_slice);