From 3aef9bcd1d9305c2cb8410a0dac6b0f22ee76d0f Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 11:44:25 +0100 Subject: [PATCH 01/27] [red-knot] Understand 'typing.ClassVar' --- .../mdtest/type_qualifiers/classvar.md | 49 ++++ .../src/types/infer.rs | 209 +++++++++++++----- 2 files changed, 197 insertions(+), 61 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/type_qualifiers/classvar.md b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md new file mode 100644 index 00000000000000..e54b42ff5a97f0 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -0,0 +1,49 @@ +# `typing.ClassVar` + +[`typing.ClassVar`] is a type qualifier that is used to indicate that a class variable may not be +set on instances. For more details, see the [typing spec]. + +This test mostly makes sure that we "see" the type qualifier in an annotation. For more details on +the semantics, see the [test on attributes](../attributes.md) test. + +## Basic + +```py +from typing import ClassVar, Annotated + +class C: + x: ClassVar[int] = 1 + y: Annotated[ClassVar[int], "the annotation for y"] = 1 + z: "ClassVar[int]" = 1 + +reveal_type(C.x) # revealed: int +reveal_type(C.y) # revealed: int +reveal_type(C.z) # revealed: int + +c = C() + +# error: [invalid-attribute-access] +c.x = 2 +# error: [invalid-attribute-access] +c.y = 2 +# error: [invalid-attribute-access] +c.z = 2 +``` + +## Too many arguments + +```py +class C: + # error: [invalid-type-form] "Special form `typing.ClassVar` expected exactly one type parameter" + x: ClassVar[int, str] = 1 +``` + +## Used outside of a class + +```py +# TODO: this should be an error +x: ClassVar[int] = 1 +``` + +[typing spec]: https://typing.readthedocs.io/en/latest/spec/class-compat.html#classvar +[`typing.classvar`]: https://docs.python.org/3/library/typing.html#typing.ClassVar diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 17bf3caabb0b54..72ad4ab5fade91 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -28,6 +28,7 @@ //! definitions once the rest of the types in the scope have been inferred. use std::num::NonZeroU32; +use bitflags::bitflags; use itertools::{Either, Itertools}; use ruff_db::files::File; use ruff_db::parsed::parsed_module; @@ -323,6 +324,46 @@ enum DeclaredAndInferredType<'db> { }, } +bitflags! { + /// Type qualifiers that appear in an annotation expression. + #[derive(Copy, Clone, Debug, Eq, PartialEq)] + 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`. This struct holds the type and the corresponding qualifiers. +/// +/// Example: `Final[Annotated[ClassVar[tuple[int]], "metadata"]]` would have the type `tuple[int]` +/// and the qualifiers `CLASS_VAR | FINAL`. +struct QualifiedType<'db> { + inner: Type<'db>, + qualifiers: TypeQualifiers, +} + +impl<'db> QualifiedType<'db> { + fn ignore_qualifiers(self) -> Type<'db> { + self.inner + } + + fn add_qualifier(&mut self, qualifier: TypeQualifiers) { + self.qualifiers |= qualifier; + } +} + +impl<'db> From> for QualifiedType<'db> { + fn from(ty: Type<'db>) -> Self { + Self { + inner: ty, + qualifiers: TypeQualifiers::empty(), + } + } +} + /// Builder to infer all types in a region. /// /// A builder is used by creating it with [`new()`](TypeInferenceBuilder::new), and then calling @@ -1740,7 +1781,7 @@ impl<'db> TypeInferenceBuilder<'db> { let tuple = TupleType::new( self.db(), elts.iter() - .map(|expr| self.infer_type_expression(expr)) + .map(|expr| self.infer_type_expression(expr).inner) .collect::>(), ); let constraints = TypeVarBoundOrConstraints::Constraints(tuple); @@ -1749,11 +1790,13 @@ impl<'db> TypeInferenceBuilder<'db> { } } Some(expr) => Some(TypeVarBoundOrConstraints::UpperBound( - self.infer_type_expression(expr), + self.infer_type_expression(expr).inner, )), None => None, }; - let default_ty = self.infer_optional_type_expression(default.as_deref()); + let default_ty = self + .infer_optional_type_expression(default.as_deref()) + .map(|qualified_type| qualified_type.inner); let ty = Type::KnownInstance(KnownInstanceType::TypeVar(TypeVarInstance::new( self.db(), name.id.clone(), @@ -2060,10 +2103,12 @@ impl<'db> TypeInferenceBuilder<'db> { simple: _, } = assignment; - let mut declared_ty = self.infer_annotation_expression( - annotation, - DeferredExpressionState::from(self.are_all_types_deferred()), - ); + let mut declared_ty = self + .infer_annotation_expression( + annotation, + DeferredExpressionState::from(self.are_all_types_deferred()), + ) + .inner; // Handle various singletons. if let Type::Instance(InstanceType { class }) = declared_ty { @@ -2609,7 +2654,7 @@ impl<'db> TypeInferenceBuilder<'db> { .enumerate() .map(|(index, arg_or_keyword)| { let infer_argument_type = match parameter_expectations.expectation_at_index(index) { - ParameterExpectation::TypeExpression => Self::infer_type_expression, + ParameterExpectation::TypeExpression => Self::infer_pure_type_expression, ParameterExpectation::ValueExpression => Self::infer_expression, }; @@ -4698,7 +4743,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, annotation: &ast::Expr, deferred_state: DeferredExpressionState, - ) -> Type<'db> { + ) -> QualifiedType<'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 +4758,23 @@ 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) -> QualifiedType<'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), + ast::Expr::StringLiteral(string) => { + self.infer_string_annotation_expression(string).into() + } // 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 +4782,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,7 +4792,7 @@ impl<'db> TypeInferenceBuilder<'db> { format_args!("Type expressions cannot use f-strings"), ); self.infer_fstring_expression(fstring); - Type::unknown() + Type::unknown().into() } // All other annotation expressions are (possibly) valid type expressions, so handle @@ -4753,13 +4800,16 @@ impl<'db> TypeInferenceBuilder<'db> { type_expr => self.infer_type_expression_no_store(type_expr), }; - self.store_expression_type(annotation, annotation_ty); + self.store_expression_type(annotation, annotation_ty.inner); 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, + ) -> QualifiedType<'db> { match parse_string_annotation(&self.context, string) { Some(parsed) => { // String annotations are always evaluated in the deferred context. @@ -4768,18 +4818,23 @@ impl<'db> TypeInferenceBuilder<'db> { DeferredExpressionState::InStringAnnotation, ) } - None => Type::unknown(), + None => Type::unknown().into(), } } } /// Type expressions impl<'db> TypeInferenceBuilder<'db> { - /// Infer the type of a type expression. - fn infer_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { - let ty = self.infer_type_expression_no_store(expression); - self.store_expression_type(expression, ty); - ty + /// Infer the type of a type expression, including all type qualifiers + fn infer_type_expression(&mut self, expression: &ast::Expr) -> QualifiedType<'db> { + let qualified_ty = self.infer_type_expression_no_store(expression); + self.store_expression_type(expression, qualified_ty.inner); + qualified_ty + } + + /// Infer the type of a type expression, ignoring all type qualifiers + fn infer_pure_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { + self.infer_type_expression(expression).ignore_qualifiers() } /// Similar to [`infer_type_expression`], but accepts an optional type expression and returns @@ -4789,7 +4844,7 @@ impl<'db> TypeInferenceBuilder<'db> { fn infer_optional_type_expression( &mut self, expression: Option<&ast::Expr>, - ) -> Option> { + ) -> Option> { expression.map(|expr| self.infer_type_expression(expr)) } @@ -4800,7 +4855,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, expression: &ast::Expr, deferred_state: DeferredExpressionState, - ) -> Type<'db> { + ) -> QualifiedType<'db> { let previous_deferred_state = std::mem::replace(&mut self.deferred_state, deferred_state); let annotation_ty = self.infer_type_expression(expression); self.deferred_state = previous_deferred_state; @@ -4808,9 +4863,9 @@ impl<'db> TypeInferenceBuilder<'db> { } /// Infer the type of a type expression without storing the result. - fn infer_type_expression_no_store(&mut self, expression: &ast::Expr) -> Type<'db> { + fn infer_type_expression_no_store(&mut self, expression: &ast::Expr) -> QualifiedType<'db> { // https://typing.readthedocs.io/en/latest/spec/annotations.html#grammar-token-expression-grammar-type_expression - match expression { + let ty = match expression { ast::Expr::Name(name) => match name.ctx { ast::ExprContext::Load => self .infer_name_expression(name) @@ -4832,7 +4887,9 @@ impl<'db> TypeInferenceBuilder<'db> { ast::Expr::NoneLiteral(_literal) => Type::none(self.db()), // https://typing.readthedocs.io/en/latest/spec/annotations.html#string-annotations - ast::Expr::StringLiteral(string) => self.infer_string_type_expression(string), + ast::Expr::StringLiteral(string) => { + return self.infer_string_type_expression(string); + } // TODO: an Ellipsis literal *on its own* does not have any meaning in annotation // expressions, but is meaningful in the context of a number of special forms. @@ -4855,24 +4912,29 @@ impl<'db> TypeInferenceBuilder<'db> { let value_ty = self.infer_expression(value); - match value_ty { + let qualified_ty = 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), + Some(KnownClass::Tuple) => { + self.infer_tuple_type_expression(slice).into() + } + Some(KnownClass::Type) => { + self.infer_subclass_of_type_expression(slice).into() + } _ => self.infer_subscript_type_expression(subscript, value_ty), } } _ => self.infer_subscript_type_expression(subscript, value_ty), - } + }; + return qualified_ty; } ast::Expr::BinOp(binary) => { match binary.op { // PEP-604 unions are okay, e.g., `int | str` ast::Operator::BitOr => { - let left_ty = self.infer_type_expression(&binary.left); - let right_ty = self.infer_type_expression(&binary.right); + let left_ty = self.infer_pure_type_expression(&binary.left); + let right_ty = self.infer_pure_type_expression(&binary.right); UnionType::from_elements(self.db(), [left_ty, right_ty]) } // anything else is an invalid annotation: @@ -4977,11 +5039,16 @@ impl<'db> TypeInferenceBuilder<'db> { Type::unknown() } ast::Expr::IpyEscapeCommand(_) => todo!("Implement Ipy escape command support"), - } + }; + + ty.into() } /// Infer the type of a string type expression. - fn infer_string_type_expression(&mut self, string: &ast::ExprStringLiteral) -> Type<'db> { + fn infer_string_type_expression( + &mut self, + string: &ast::ExprStringLiteral, + ) -> QualifiedType<'db> { match parse_string_annotation(&self.context, string) { Some(parsed) => { // String annotations are always evaluated in the deferred context. @@ -4990,7 +5057,7 @@ impl<'db> TypeInferenceBuilder<'db> { DeferredExpressionState::InStringAnnotation, ) } - None => Type::unknown(), + None => Type::unknown().into(), } } @@ -5025,7 +5092,7 @@ impl<'db> TypeInferenceBuilder<'db> { let mut return_todo = false; for element in elements { - let element_ty = self.infer_type_expression(element); + let element_ty = self.infer_pure_type_expression(element); return_todo |= element_could_alter_type_of_whole_tuple(element, element_ty); element_types.push(element_ty); } @@ -5044,7 +5111,7 @@ impl<'db> TypeInferenceBuilder<'db> { ty } single_element => { - let single_element_ty = self.infer_type_expression(single_element); + let single_element_ty = self.infer_pure_type_expression(single_element); if element_could_alter_type_of_whole_tuple(single_element, single_element_ty) { todo_type!("full tuple[...] support") } else { @@ -5132,7 +5199,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, subscript: &ast::ExprSubscript, value_ty: Type<'db>, - ) -> Type<'db> { + ) -> QualifiedType<'db> { let ast::ExprSubscript { range: _, value: _, @@ -5146,11 +5213,11 @@ impl<'db> TypeInferenceBuilder<'db> { } Type::Dynamic(DynamicType::Todo(_)) => { self.infer_type_expression(slice); - value_ty + value_ty.into() } _ => { self.infer_type_expression(slice); - todo_type!("generics") + todo_type!("generics").into() } } } @@ -5159,9 +5226,9 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, subscript: &ast::ExprSubscript, known_instance: KnownInstanceType, - ) -> Type<'db> { + ) -> QualifiedType<'db> { let arguments_slice = &*subscript.slice; - match known_instance { + let ty = match known_instance { KnownInstanceType::Annotated => { let report_invalid_arguments = || { self.context.report_lint( @@ -5184,7 +5251,7 @@ impl<'db> TypeInferenceBuilder<'db> { // However, we still treat `Annotated[T]` as `T` here for the purpose of // giving better diagnostics later on. // Pyright also does this. Mypy doesn't; it falls back to `Any` instead. - return self.infer_type_expression(arguments_slice); + return self.infer_type_expression(arguments_slice).into(); }; if arguments.len() < 2 { @@ -5193,14 +5260,14 @@ impl<'db> TypeInferenceBuilder<'db> { let [type_expr, metadata @ ..] = &arguments[..] else { self.infer_type_expression(arguments_slice); - return Type::unknown(); + return Type::unknown().into(); }; for element in metadata { self.infer_expression(element); } - let ty = self.infer_type_expression(type_expr); + let ty = self.infer_pure_type_expression(type_expr); self.store_expression_type(arguments_slice, ty); ty } @@ -5223,19 +5290,19 @@ impl<'db> TypeInferenceBuilder<'db> { } } KnownInstanceType::Optional => { - let param_type = self.infer_type_expression(arguments_slice); + let param_type = self.infer_pure_type_expression(arguments_slice); UnionType::from_elements(self.db(), [param_type, Type::none(self.db())]) } KnownInstanceType::Union => match arguments_slice { ast::Expr::Tuple(t) => { let union_ty = UnionType::from_elements( self.db(), - t.iter().map(|elt| self.infer_type_expression(elt)), + t.iter().map(|elt| self.infer_pure_type_expression(elt)), ); self.store_expression_type(arguments_slice, union_ty); union_ty } - _ => self.infer_type_expression(arguments_slice), + _ => self.infer_pure_type_expression(arguments_slice), }, KnownInstanceType::TypeVar(_) => { self.infer_type_expression(arguments_slice); @@ -5264,7 +5331,7 @@ impl<'db> TypeInferenceBuilder<'db> { Type::unknown() } _ => { - let argument_type = self.infer_type_expression(arguments_slice); + let argument_type = self.infer_pure_type_expression(arguments_slice); argument_type.negate(self.db()) } }, @@ -5276,7 +5343,7 @@ impl<'db> TypeInferenceBuilder<'db> { elements .fold(IntersectionBuilder::new(self.db()), |builder, element| { - builder.add_positive(self.infer_type_expression(element)) + builder.add_positive(self.infer_pure_type_expression(element)) }) .build() } @@ -5345,14 +5412,32 @@ 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 => match arguments_slice { + ast::Expr::Tuple(_) => { + self.context.report_lint( + &INVALID_TYPE_FORM, + subscript.into(), + format_args!( + "Special form `{}` expected exactly one type parameter", + known_instance.repr(self.db()) + ), + ); + Type::unknown() + } + _ => { + let mut argument_type = self.infer_type_expression(arguments_slice); + match known_instance { + KnownInstanceType::ClassVar => { + argument_type.add_qualifier(TypeQualifiers::CLASS_VAR); + } + KnownInstanceType::Final => { + argument_type.add_qualifier(TypeQualifiers::FINAL); + } + _ => unreachable!(), + } + return argument_type; + } + }, KnownInstanceType::Required => { self.infer_type_expression(arguments_slice); todo_type!("`Required[]` type qualifier") @@ -5414,7 +5499,9 @@ impl<'db> TypeInferenceBuilder<'db> { } KnownInstanceType::Type => self.infer_subclass_of_type_expression(arguments_slice), KnownInstanceType::Tuple => self.infer_tuple_type_expression(arguments_slice), - } + }; + + ty.into() } fn infer_literal_parameter_type<'ast>( From c2c9069f32d1c26860148a16df478463a07cfc44 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 12:07:44 +0100 Subject: [PATCH 02/27] Return QualifiedType from declaration_ty --- crates/red_knot_python_semantic/src/types.rs | 5 +- .../src/types/infer.rs | 87 +++++++++++-------- 2 files changed, 52 insertions(+), 40 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index a5ff0358d88c96..7b77a32fc759ad 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -37,6 +37,7 @@ use crate::types::call::{ }; use crate::types::class_base::ClassBase; use crate::types::diagnostic::INVALID_TYPE_FORM; +use crate::types::infer::QualifiedType; use crate::types::mro::{Mro, MroError, MroIterator}; use crate::types::narrow::narrowing_constraint; use crate::{Db, FxOrderSet, Module, Program, PythonVersion}; @@ -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>) -> QualifiedType<'db> { let inference = infer_definition_types(db, definition); inference.declaration_ty(definition) } @@ -400,7 +401,7 @@ fn declarations_ty<'db>( if static_visibility.is_always_false() { None } else { - Some(declaration_ty(db, declaration)) + Some(declaration_ty(db, declaration).ignore_qualifiers()) } }, ); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 72ad4ab5fade91..a6e1d43363942a 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -114,7 +114,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()); @@ -243,7 +243,7 @@ pub(crate) struct TypeInference<'db> { bindings: FxHashMap, Type<'db>>, /// The types of every declaration in this region. - declarations: FxHashMap, Type<'db>>, + declarations: FxHashMap, QualifiedType<'db>>, /// The definitions that are deferred. deferred: FxHashSet>, @@ -282,7 +282,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>) -> QualifiedType<'db> { self.declarations[&definition] } @@ -319,7 +319,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: QualifiedType<'db>, inferred_ty: Type<'db>, }, } @@ -327,7 +327,7 @@ enum DeclaredAndInferredType<'db> { bitflags! { /// Type qualifiers that appear in an annotation expression. #[derive(Copy, Clone, Debug, Eq, PartialEq)] - struct TypeQualifiers: u8 { + pub(crate) struct TypeQualifiers: u8 { /// `typing.ClassVar` const CLASS_VAR = 1 << 0; /// `typing.Final` @@ -340,17 +340,18 @@ bitflags! { /// /// Example: `Final[Annotated[ClassVar[tuple[int]], "metadata"]]` would have the type `tuple[int]` /// and the qualifiers `CLASS_VAR | FINAL`. -struct QualifiedType<'db> { +#[derive(Clone, Debug, Copy, Eq, PartialEq)] +pub(crate) struct QualifiedType<'db> { inner: Type<'db>, qualifiers: TypeQualifiers, } impl<'db> QualifiedType<'db> { - fn ignore_qualifiers(self) -> Type<'db> { + pub(crate) fn ignore_qualifiers(&self) -> Type<'db> { self.inner } - fn add_qualifier(&mut self, qualifier: TypeQualifiers) { + pub(crate) fn add_qualifier(&mut self, qualifier: TypeQualifiers) { self.qualifiers |= qualifier; } } @@ -604,7 +605,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.ignore_qualifiers() + .into_class_literal() + .map(|ty| (ty.class, class.node())) } else { None } @@ -920,7 +923,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: QualifiedType<'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); @@ -928,7 +936,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.ignore_qualifiers()) { ty } else { self.context.report_lint( @@ -936,11 +944,11 @@ impl<'db> TypeInferenceBuilder<'db> { node, format_args!( "Cannot declare type `{}` for inferred type `{}`", - ty.display(self.db()), + ty.ignore_qualifiers().display(self.db()), inferred_ty.display(self.db()) ), ); - Type::unknown() + Type::unknown().into() }; self.types.declarations.insert(declaration, ty); } @@ -955,22 +963,27 @@ impl<'db> TypeInferenceBuilder<'db> { debug_assert!(definition.is_declaration(self.db())); let (declared_ty, inferred_ty) = match declared_and_inferred_ty { - DeclaredAndInferredType::AreTheSame(ty) => (ty, ty), - DeclaredAndInferredType::MightBeDifferent { + &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.ignore_qualifiers()) { (declared_ty, inferred_ty) } else { - report_invalid_assignment(&self.context, node, *declared_ty, *inferred_ty); + report_invalid_assignment( + &self.context, + node, + declared_ty.ignore_qualifiers(), + inferred_ty, + ); // if the assignment is invalid, fall back to assuming the annotation is correct - (declared_ty, declared_ty) + (declared_ty, declared_ty.ignore_qualifiers()) } } }; - 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( @@ -981,7 +994,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.add_declaration_with_binding( node, definition, - &DeclaredAndInferredType::AreTheSame(Type::unknown()), + &DeclaredAndInferredType::AreTheSame(Type::unknown().into()), ); } @@ -1166,7 +1179,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.add_declaration_with_binding( function.into(), definition, - &DeclaredAndInferredType::AreTheSame(function_ty), + &DeclaredAndInferredType::AreTheSame(function_ty.into()), ); } @@ -1261,7 +1274,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() @@ -1269,7 +1282,7 @@ impl<'db> TypeInferenceBuilder<'db> { .as_ref() .is_some_and(|d| d.is_ellipsis_literal_expr()) { - DeclaredAndInferredType::AreTheSame(declared_ty) + DeclaredAndInferredType::AreTheSame(declared_ty.into()) } else { self.context.report_lint( &INVALID_PARAMETER_DEFAULT, @@ -1781,7 +1794,7 @@ impl<'db> TypeInferenceBuilder<'db> { let tuple = TupleType::new( self.db(), elts.iter() - .map(|expr| self.infer_type_expression(expr).inner) + .map(|expr| self.infer_pure_type_expression(expr)) .collect::>(), ); let constraints = TypeVarBoundOrConstraints::Constraints(tuple); @@ -1790,13 +1803,13 @@ impl<'db> TypeInferenceBuilder<'db> { } } Some(expr) => Some(TypeVarBoundOrConstraints::UpperBound( - self.infer_type_expression(expr).inner, + self.infer_pure_type_expression(expr), )), None => None, }; let default_ty = self .infer_optional_type_expression(default.as_deref()) - .map(|qualified_type| qualified_type.inner); + .map(|qualified_type| qualified_type.ignore_qualifiers()); let ty = Type::KnownInstance(KnownInstanceType::TypeVar(TypeVarInstance::new( self.db(), name.id.clone(), @@ -2103,15 +2116,13 @@ impl<'db> TypeInferenceBuilder<'db> { simple: _, } = assignment; - let mut declared_ty = self - .infer_annotation_expression( - annotation, - DeferredExpressionState::from(self.are_all_types_deferred()), - ) - .inner; + let mut declared_ty = self.infer_annotation_expression( + annotation, + DeferredExpressionState::from(self.are_all_types_deferred()), + ); // Handle various singletons. - if let Type::Instance(InstanceType { class }) = declared_ty { + if let Type::Instance(InstanceType { class }) = declared_ty.ignore_qualifiers() { 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( @@ -2119,7 +2130,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.file(), &name_expr.id, ) { - declared_ty = Type::KnownInstance(known_instance); + declared_ty.inner = Type::KnownInstance(known_instance); } } } @@ -2128,7 +2139,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.ignore_qualifiers() } else { inferred_ty }; @@ -4800,7 +4811,7 @@ impl<'db> TypeInferenceBuilder<'db> { type_expr => self.infer_type_expression_no_store(type_expr), }; - self.store_expression_type(annotation, annotation_ty.inner); + self.store_expression_type(annotation, annotation_ty.ignore_qualifiers()); annotation_ty } @@ -4828,7 +4839,7 @@ impl<'db> TypeInferenceBuilder<'db> { /// Infer the type of a type expression, including all type qualifiers fn infer_type_expression(&mut self, expression: &ast::Expr) -> QualifiedType<'db> { let qualified_ty = self.infer_type_expression_no_store(expression); - self.store_expression_type(expression, qualified_ty.inner); + self.store_expression_type(expression, qualified_ty.ignore_qualifiers()); qualified_ty } From 93735ec78a572bfae922a3d23ad458f7b4b27198 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 13:32:56 +0100 Subject: [PATCH 03/27] Add diagnostic message --- .../resources/mdtest/attributes.md | 2 +- crates/red_knot_python_semantic/src/types.rs | 76 ++++++++++++------- .../src/types/diagnostic.rs | 20 +++++ .../src/types/infer.rs | 54 +++++++++++-- 4 files changed, 114 insertions(+), 38 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 24f0be14f3b9ea..6ec5c56bb42bd8 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -182,7 +182,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 pure class variable `pure_class_variable1` from an instance" c_instance.pure_class_variable1 = "value set on instance" C.pure_class_variable1 = "overwritten on class" diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 7b77a32fc759ad..ab251ad316ae22 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -37,7 +37,7 @@ use crate::types::call::{ }; use crate::types::class_base::ClassBase; use crate::types::diagnostic::INVALID_TYPE_FORM; -use crate::types::infer::QualifiedType; +use crate::types::infer::{QualifiedType, TypeQualifiers}; use crate::types::mro::{Mro, MroError, MroIterator}; use crate::types::narrow::narrowing_constraint; use crate::{Db, FxOrderSet, Module, Program, PythonVersion}; @@ -93,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(|(ty, _)| ty); match declared { // Symbol is declared, trust the declared type @@ -358,7 +358,7 @@ fn bindings_ty<'db>( } /// 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<(Symbol<'db>, TypeQualifiers), (Type<'db>, Box<[Type<'db>]>)>; /// Build a declared type from a [`DeclarationsIterator`]. /// @@ -401,7 +401,7 @@ fn declarations_ty<'db>( if static_visibility.is_always_false() { None } else { - Some(declaration_ty(db, declaration).ignore_qualifiers()) + Some(declaration_ty(db, declaration)) } }, ); @@ -409,16 +409,18 @@ 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.ignore_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.ignore_qualifiers(); + if !ty_first.is_equivalent_to(db, other_ty) { + conflicting.push(other_ty); } - builder = builder.add(other); + builder = builder.add(other_ty); } builder.build() } else { - first + first.ignore_qualifiers() }; if conflicting.is_empty() { let boundness = match undeclared_visibility { @@ -429,15 +431,18 @@ fn declarations_ty<'db>( Truthiness::Ambiguous => Boundness::PossiblyUnbound, }; - Ok(Symbol::Type(declared_ty, boundness)) + // TODO: deal with conflicting qualifiers? + Ok((Symbol::Type(declared_ty, boundness), first.qualifiers())) } else { Err(( declared_ty, - std::iter::once(first).chain(conflicting).collect(), + std::iter::once(first.ignore_qualifiers()) + .chain(conflicting) + .collect(), )) } } else { - Ok(Symbol::Unbound) + Ok((Symbol::Unbound, TypeQualifiers::empty())) } } @@ -1674,7 +1679,7 @@ impl<'db> Type<'db> { (Some(KnownClass::VersionInfo), "minor") => { Type::IntLiteral(Program::get(db).python_version(db).minor.into()).into() } - _ => class.instance_member(db, name), + _ => class.instance_member(db, name).0, }, Type::Union(union) => { @@ -3979,15 +3984,22 @@ 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, + ) -> (Symbol<'db>, TypeQualifiers) { for superclass in self.iter_mro(db) { match superclass { ClassBase::Dynamic(_) => { - return todo_type!("instance attribute on class with dynamic base").into(); + return ( + todo_type!("instance attribute on class with dynamic base").into(), + TypeQualifiers::empty(), + ); } ClassBase::Class(class) => { let member = class.own_instance_member(db, name); - if !member.is_unbound() { + if !member.0.is_unbound() { return member; } } @@ -3996,12 +4008,15 @@ impl<'db> Class<'db> { // TODO: The symbol is not present in any class body, but it could be implicitly // defined in `__init__` or other methods anywhere in the MRO. - todo_type!("implicit instance attribute").into() + ( + todo_type!("implicit instance attribute").into(), + TypeQualifiers::empty(), + ) } /// 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) -> (Symbol<'db>, TypeQualifiers) { // TODO: There are many things that are not yet implemented here: // - `typing.ClassVar` and `typing.Final` // - Proper diagnostics @@ -4017,39 +4032,42 @@ impl<'db> Class<'db> { let declarations = use_def.public_declarations(symbol); match declarations_ty(db, declarations) { - Ok(Symbol::Type(declared_ty, _)) => { + Ok((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 // and non-property methods. if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) { - todo_type!("@property").into() + (todo_type!("@property").into(), qualifiers) } else { - todo_type!("bound method").into() + (todo_type!("bound method").into(), qualifiers) } } else { - Symbol::Type(declared_ty, Boundness::Bound) + (Symbol::Type(declared_ty, Boundness::Bound), qualifiers) } } - Ok(Symbol::Unbound) => { + Ok((Symbol::Unbound, _)) => { 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, _) => ( + Symbol::Type( + UnionType::from_elements(db, [Type::unknown(), ty]), + Boundness::Bound, + ), + TypeQualifiers::empty(), ), - Symbol::Unbound => Symbol::Unbound, + Symbol::Unbound => (Symbol::Unbound, TypeQualifiers::empty()), } } Err((declared_ty, _)) => { // Ignore conflicting declarations - declared_ty.into() + (declared_ty.into(), TypeQualifiers::empty()) } } } else { - Symbol::Unbound + (Symbol::Unbound, TypeQualifiers::empty()) } } diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 80e80950252b23..97cd26edbc2e78 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -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 the argument of `static_assert` is statically known to be true. + /// + /// ## 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, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index a6e1d43363942a..01f21243a378c7 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -54,10 +54,11 @@ 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, - 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, + 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, }; use crate::types::mro::MroErrorKind; use crate::types::unpacker::{UnpackResult, Unpacker}; @@ -335,6 +336,12 @@ bitflags! { } } +impl TypeQualifiers { + pub(crate) fn is_class_var(self) -> bool { + self.contains(Self::CLASS_VAR) + } +} + /// When inferring the type of an annotation expression, we can also encounter type qualifiers /// such as `ClassVar` or `Final`. This struct holds the type and the corresponding qualifiers. /// @@ -354,6 +361,10 @@ impl<'db> QualifiedType<'db> { pub(crate) fn add_qualifier(&mut self, qualifier: TypeQualifiers) { self.qualifiers |= qualifier; } + + pub(crate) fn qualifiers(&self) -> TypeQualifiers { + self.qualifiers + } } impl<'db> From> for QualifiedType<'db> { @@ -899,6 +910,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(|(ty, _)| ty) .map(|s| s.ignore_possibly_unbound().unwrap_or(Type::unknown())) .unwrap_or_else(|(ty, conflicting)| { // TODO point out the conflicting declarations in the diagnostic? @@ -2035,6 +2047,32 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_standalone_expression(value); } } + ast::Expr::Attribute( + expr_attr @ ast::ExprAttribute { + value: attr_value, + attr, + ctx, + .. + }, + ) => { + self.infer_standalone_expression(value); + let attr_value_ty = self.infer_expression(attr_value); + match (ctx, attr_value_ty) { + (ast::ExprContext::Store, Type::Instance(instance)) => { + let qualifiers = instance.class.instance_member(self.db(), attr).1; + if qualifiers.is_class_var() { + self.context.report_lint( + &INVALID_ATTRIBUTE_ACCESS, + expr_attr.into(), + format_args!( + "Cannot assign to pure class variable `{attr}` from an instance", + ), + ); + } + } + _ => {} + } + } _ => { // TODO: Remove this once we handle all possible assignment targets. self.infer_standalone_expression(value); @@ -5262,7 +5300,7 @@ impl<'db> TypeInferenceBuilder<'db> { // However, we still treat `Annotated[T]` as `T` here for the purpose of // giving better diagnostics later on. // Pyright also does this. Mypy doesn't; it falls back to `Any` instead. - return self.infer_type_expression(arguments_slice).into(); + return self.infer_type_expression(arguments_slice); }; if arguments.len() < 2 { @@ -5278,9 +5316,9 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_expression(element); } - let ty = self.infer_pure_type_expression(type_expr); - self.store_expression_type(arguments_slice, ty); - ty + let qualified_ty = self.infer_type_expression(type_expr); + self.store_expression_type(arguments_slice, qualified_ty.ignore_qualifiers()); + return qualified_ty; } KnownInstanceType::Literal => { match self.infer_literal_parameter_type(arguments_slice) { From 672de4660fbf52d6980f0cb489aaa051e8ce1a78 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 14:14:00 +0100 Subject: [PATCH 04/27] Refactoring --- .../src/types/infer.rs | 151 +++++++++--------- 1 file changed, 77 insertions(+), 74 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 01f21243a378c7..6784ff6a28359d 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -974,9 +974,9 @@ 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.into(), ty), - &DeclaredAndInferredType::MightBeDifferent { + let (declared_ty, inferred_ty) = match *declared_and_inferred_ty { + DeclaredAndInferredType::AreTheSame(ty) => (ty.into(), ty), + DeclaredAndInferredType::MightBeDifferent { declared_ty, inferred_ty, } => { @@ -1006,7 +1006,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.add_declaration_with_binding( node, definition, - &DeclaredAndInferredType::AreTheSame(Type::unknown().into()), + &DeclaredAndInferredType::AreTheSame(Type::unknown()), ); } @@ -1191,7 +1191,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.add_declaration_with_binding( function.into(), definition, - &DeclaredAndInferredType::AreTheSame(function_ty.into()), + &DeclaredAndInferredType::AreTheSame(function_ty), ); } @@ -1294,7 +1294,7 @@ impl<'db> TypeInferenceBuilder<'db> { .as_ref() .is_some_and(|d| d.is_ellipsis_literal_expr()) { - DeclaredAndInferredType::AreTheSame(declared_ty.into()) + DeclaredAndInferredType::AreTheSame(declared_ty) } else { self.context.report_lint( &INVALID_PARAMETER_DEFAULT, @@ -1806,7 +1806,7 @@ impl<'db> TypeInferenceBuilder<'db> { let tuple = TupleType::new( self.db(), elts.iter() - .map(|expr| self.infer_pure_type_expression(expr)) + .map(|expr| self.infer_type_expression(expr)) .collect::>(), ); let constraints = TypeVarBoundOrConstraints::Constraints(tuple); @@ -1815,7 +1815,7 @@ impl<'db> TypeInferenceBuilder<'db> { } } Some(expr) => Some(TypeVarBoundOrConstraints::UpperBound( - self.infer_pure_type_expression(expr), + self.infer_type_expression(expr), )), None => None, }; @@ -2057,20 +2057,19 @@ impl<'db> TypeInferenceBuilder<'db> { ) => { self.infer_standalone_expression(value); let attr_value_ty = self.infer_expression(attr_value); - match (ctx, attr_value_ty) { - (ast::ExprContext::Store, Type::Instance(instance)) => { - let qualifiers = instance.class.instance_member(self.db(), attr).1; - if qualifiers.is_class_var() { - self.context.report_lint( - &INVALID_ATTRIBUTE_ACCESS, - expr_attr.into(), - format_args!( - "Cannot assign to pure class variable `{attr}` from an instance", - ), - ); - } + self.store_expression_type(expr_attr, attr_value_ty); + + if let (ast::ExprContext::Store, Type::Instance(instance)) = (ctx, attr_value_ty) { + let qualifiers = instance.class.instance_member(self.db(), attr).1; + if qualifiers.is_class_var() { + self.context.report_lint( + &INVALID_ATTRIBUTE_ACCESS, + expr_attr.into(), + format_args!( + "Cannot assign to pure class variable `{attr}` from an instance", + ), + ); } - _ => {} } } _ => { @@ -2703,7 +2702,7 @@ impl<'db> TypeInferenceBuilder<'db> { .enumerate() .map(|(index, arg_or_keyword)| { let infer_argument_type = match parameter_expectations.expectation_at_index(index) { - ParameterExpectation::TypeExpression => Self::infer_pure_type_expression, + ParameterExpectation::TypeExpression => Self::infer_type_expression, ParameterExpectation::ValueExpression => Self::infer_expression, }; @@ -4818,9 +4817,7 @@ impl<'db> TypeInferenceBuilder<'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).into() - } + 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).into(), @@ -4874,39 +4871,44 @@ impl<'db> TypeInferenceBuilder<'db> { /// Type expressions impl<'db> TypeInferenceBuilder<'db> { - /// Infer the type of a type expression, including all type qualifiers - fn infer_type_expression(&mut self, expression: &ast::Expr) -> QualifiedType<'db> { + /// Infer the type of a type expression, including all type qualifiers such + /// as `ClassVar` or `Final`. + fn infer_type_expression_with_qualifiers( + &mut self, + expression: &ast::Expr, + ) -> QualifiedType<'db> { let qualified_ty = self.infer_type_expression_no_store(expression); self.store_expression_type(expression, qualified_ty.ignore_qualifiers()); qualified_ty } - /// Infer the type of a type expression, ignoring all type qualifiers - fn infer_pure_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { - self.infer_type_expression(expression).ignore_qualifiers() + /// Infer the type of a type expression. + fn infer_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { + self.infer_type_expression_with_qualifiers(expression) + .ignore_qualifiers() } - /// Similar to [`infer_type_expression`], but accepts an optional type expression and returns - /// [`None`] if the expression is [`None`]. + /// Similar to [`infer_type_expression_with_qualifiers`], but accepts an optional + /// type expression and returns [`None`] if the expression is [`None`]. /// - /// [`infer_type_expression`]: TypeInferenceBuilder::infer_type_expression + /// [`infer_type_expression_with_qualifiers`]: TypeInferenceBuilder::infer_type_expression_with_qualifiers fn infer_optional_type_expression( &mut self, expression: Option<&ast::Expr>, ) -> Option> { - expression.map(|expr| self.infer_type_expression(expr)) + expression.map(|expr| self.infer_type_expression_with_qualifiers(expr)) } - /// Similar to [`infer_type_expression`], but accepts a [`DeferredExpressionState`]. + /// Similar to [`infer_type_expression_with_qualifiers`], but accepts a [`DeferredExpressionState`]. /// - /// [`infer_type_expression`]: TypeInferenceBuilder::infer_type_expression + /// [`infer_type_expression_with_qualifiers`]: TypeInferenceBuilder::infer_type_expression_with_qualifiers fn infer_type_expression_with_state( &mut self, expression: &ast::Expr, deferred_state: DeferredExpressionState, ) -> QualifiedType<'db> { let previous_deferred_state = std::mem::replace(&mut self.deferred_state, deferred_state); - let annotation_ty = self.infer_type_expression(expression); + let annotation_ty = self.infer_type_expression_with_qualifiers(expression); self.deferred_state = previous_deferred_state; annotation_ty } @@ -4982,8 +4984,8 @@ impl<'db> TypeInferenceBuilder<'db> { match binary.op { // PEP-604 unions are okay, e.g., `int | str` ast::Operator::BitOr => { - let left_ty = self.infer_pure_type_expression(&binary.left); - let right_ty = self.infer_pure_type_expression(&binary.right); + let left_ty = self.infer_type_expression(&binary.left); + let right_ty = self.infer_type_expression(&binary.right); UnionType::from_elements(self.db(), [left_ty, right_ty]) } // anything else is an invalid annotation: @@ -5141,7 +5143,7 @@ impl<'db> TypeInferenceBuilder<'db> { let mut return_todo = false; for element in elements { - let element_ty = self.infer_pure_type_expression(element); + let element_ty = self.infer_type_expression(element); return_todo |= element_could_alter_type_of_whole_tuple(element, element_ty); element_types.push(element_ty); } @@ -5160,7 +5162,7 @@ impl<'db> TypeInferenceBuilder<'db> { ty } single_element => { - let single_element_ty = self.infer_pure_type_expression(single_element); + let single_element_ty = self.infer_type_expression(single_element); if element_could_alter_type_of_whole_tuple(single_element, single_element_ty) { todo_type!("full tuple[...] support") } else { @@ -5201,7 +5203,7 @@ impl<'db> TypeInferenceBuilder<'db> { union_ty } ast::Expr::Tuple(_) => { - self.infer_type_expression(slice); + self.infer_type_expression_with_qualifiers(slice); self.context.report_lint( &INVALID_TYPE_FORM, slice.into(), @@ -5229,7 +5231,7 @@ impl<'db> TypeInferenceBuilder<'db> { _ => self.infer_subclass_of_type_expression(parameters), }, _ => { - self.infer_type_expression(parameters); + self.infer_type_expression_with_qualifiers(parameters); todo_type!("unsupported nested subscript in type[X]") } }; @@ -5238,7 +5240,7 @@ impl<'db> TypeInferenceBuilder<'db> { } // TODO: subscripts, etc. _ => { - self.infer_type_expression(slice); + self.infer_type_expression_with_qualifiers(slice); todo_type!("unsupported type[X] special form") } } @@ -5261,11 +5263,11 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_parameterized_known_instance_type_expression(subscript, known_instance) } Type::Dynamic(DynamicType::Todo(_)) => { - self.infer_type_expression(slice); + self.infer_type_expression_with_qualifiers(slice); value_ty.into() } _ => { - self.infer_type_expression(slice); + self.infer_type_expression_with_qualifiers(slice); todo_type!("generics").into() } } @@ -5300,7 +5302,7 @@ impl<'db> TypeInferenceBuilder<'db> { // However, we still treat `Annotated[T]` as `T` here for the purpose of // giving better diagnostics later on. // Pyright also does this. Mypy doesn't; it falls back to `Any` instead. - return self.infer_type_expression(arguments_slice); + return self.infer_type_expression_with_qualifiers(arguments_slice); }; if arguments.len() < 2 { @@ -5308,7 +5310,7 @@ impl<'db> TypeInferenceBuilder<'db> { } let [type_expr, metadata @ ..] = &arguments[..] else { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); return Type::unknown().into(); }; @@ -5316,7 +5318,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_expression(element); } - let qualified_ty = self.infer_type_expression(type_expr); + let qualified_ty = self.infer_type_expression_with_qualifiers(type_expr); self.store_expression_type(arguments_slice, qualified_ty.ignore_qualifiers()); return qualified_ty; } @@ -5339,30 +5341,30 @@ impl<'db> TypeInferenceBuilder<'db> { } } KnownInstanceType::Optional => { - let param_type = self.infer_pure_type_expression(arguments_slice); + let param_type = self.infer_type_expression(arguments_slice); UnionType::from_elements(self.db(), [param_type, Type::none(self.db())]) } KnownInstanceType::Union => match arguments_slice { ast::Expr::Tuple(t) => { let union_ty = UnionType::from_elements( self.db(), - t.iter().map(|elt| self.infer_pure_type_expression(elt)), + t.iter().map(|elt| self.infer_type_expression(elt)), ); self.store_expression_type(arguments_slice, union_ty); union_ty } - _ => self.infer_pure_type_expression(arguments_slice), + _ => self.infer_type_expression(arguments_slice), }, KnownInstanceType::TypeVar(_) => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("TypeVar annotations") } KnownInstanceType::TypeAliasType(_) => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("Generic PEP-695 type alias") } KnownInstanceType::Callable => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("Callable types") } @@ -5380,7 +5382,7 @@ impl<'db> TypeInferenceBuilder<'db> { Type::unknown() } _ => { - let argument_type = self.infer_pure_type_expression(arguments_slice); + let argument_type = self.infer_type_expression(arguments_slice); argument_type.negate(self.db()) } }, @@ -5392,7 +5394,7 @@ impl<'db> TypeInferenceBuilder<'db> { elements .fold(IntersectionBuilder::new(self.db()), |builder, element| { - builder.add_positive(self.infer_pure_type_expression(element)) + builder.add_positive(self.infer_type_expression(element)) }) .build() } @@ -5417,48 +5419,48 @@ impl<'db> TypeInferenceBuilder<'db> { // TODO: Generics KnownInstanceType::ChainMap => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::ChainMap.to_instance(self.db()) } KnownInstanceType::OrderedDict => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::OrderedDict.to_instance(self.db()) } KnownInstanceType::Dict => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::Dict.to_instance(self.db()) } KnownInstanceType::List => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::List.to_instance(self.db()) } KnownInstanceType::DefaultDict => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::DefaultDict.to_instance(self.db()) } KnownInstanceType::Counter => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::Counter.to_instance(self.db()) } KnownInstanceType::Set => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::Set.to_instance(self.db()) } KnownInstanceType::FrozenSet => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::FrozenSet.to_instance(self.db()) } KnownInstanceType::Deque => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); KnownClass::Deque.to_instance(self.db()) } KnownInstanceType::ReadOnly => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`ReadOnly[]` type qualifier") } KnownInstanceType::NotRequired => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`NotRequired[]` type qualifier") } KnownInstanceType::ClassVar | KnownInstanceType::Final => match arguments_slice { @@ -5474,7 +5476,8 @@ impl<'db> TypeInferenceBuilder<'db> { Type::unknown() } _ => { - let mut argument_type = self.infer_type_expression(arguments_slice); + let mut argument_type = + self.infer_type_expression_with_qualifiers(arguments_slice); match known_instance { KnownInstanceType::ClassVar => { argument_type.add_qualifier(TypeQualifiers::CLASS_VAR); @@ -5488,23 +5491,23 @@ impl<'db> TypeInferenceBuilder<'db> { } }, KnownInstanceType::Required => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`Required[]` type qualifier") } KnownInstanceType::TypeIs => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`TypeIs[]` special form") } KnownInstanceType::TypeGuard => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`TypeGuard[]` special form") } KnownInstanceType::Concatenate => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`Concatenate[]` special form") } KnownInstanceType::Unpack => { - self.infer_type_expression(arguments_slice); + self.infer_type_expression_with_qualifiers(arguments_slice); todo_type!("`Unpack[]` special form") } KnownInstanceType::NoReturn From 6b684ef9431a53f974a6da966fd66fa1ea56b258 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 14:27:37 +0100 Subject: [PATCH 05/27] More simplifications --- crates/red_knot_python_semantic/src/types.rs | 44 +++++++-------- .../src/types/diagnostic.rs | 2 +- .../src/types/infer.rs | 53 ++++++++++--------- 3 files changed, 53 insertions(+), 46 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index ab251ad316ae22..9d803836b1dc14 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -37,7 +37,7 @@ use crate::types::call::{ }; use crate::types::class_base::ClassBase; use crate::types::diagnostic::INVALID_TYPE_FORM; -use crate::types::infer::{QualifiedType, TypeQualifiers}; +use crate::types::infer::{TypeAndQualifiers, TypeQualifiers}; use crate::types::mro::{Mro, MroError, MroIterator}; use crate::types::narrow::narrowing_constraint; use crate::{Db, FxOrderSet, Module, Program, PythonVersion}; @@ -247,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>) -> QualifiedType<'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) } @@ -4026,10 +4026,10 @@ impl<'db> Class<'db> { let body_scope = self.body_scope(db); let table = symbol_table(db, body_scope); - if let Some(symbol) = table.symbol_id_by_name(name) { + let symbol = if let Some(symbol_id) = table.symbol_id_by_name(name) { let use_def = use_def_map(db, body_scope); - let declarations = use_def.public_declarations(symbol); + let declarations = use_def.public_declarations(symbol_id); match declarations_ty(db, declarations) { Ok((Symbol::Type(declared_ty, _), qualifiers)) => { @@ -4037,38 +4037,40 @@ impl<'db> Class<'db> { // TODO: Eventually, we are going to process all decorators correctly. This is // just a temporary heuristic to provide a broad categorization into properties // and non-property methods. - if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) { - (todo_type!("@property").into(), qualifiers) + let ty = if function + .has_decorator(db, KnownClass::Property.to_class_literal(db)) + { + todo_type!("@property") } else { - (todo_type!("bound method").into(), qualifiers) - } - } else { - (Symbol::Type(declared_ty, Boundness::Bound), qualifiers) + todo_type!("bound method") + }; + return (ty.into(), qualifiers); } + + return (Symbol::Type(declared_ty, Boundness::Bound), qualifiers); } Ok((Symbol::Unbound, _)) => { - let bindings = use_def.public_bindings(symbol); + let bindings = use_def.public_bindings(symbol_id); let inferred_ty = bindings_ty(db, bindings); match inferred_ty { - Symbol::Type(ty, _) => ( - Symbol::Type( - UnionType::from_elements(db, [Type::unknown(), ty]), - Boundness::Bound, - ), - TypeQualifiers::empty(), + Symbol::Type(ty, _) => Symbol::Type( + UnionType::from_elements(db, [Type::unknown(), ty]), + Boundness::Bound, ), - Symbol::Unbound => (Symbol::Unbound, TypeQualifiers::empty()), + Symbol::Unbound => Symbol::Unbound, } } Err((declared_ty, _)) => { // Ignore conflicting declarations - (declared_ty.into(), TypeQualifiers::empty()) + declared_ty.into() } } } else { - (Symbol::Unbound, TypeQualifiers::empty()) - } + Symbol::Unbound + }; + + (symbol, TypeQualifiers::empty()) } /// Return `true` if this class appears to be a cyclic definition, diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 97cd26edbc2e78..59eab889c13b63 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -753,7 +753,7 @@ declare_lint! { declare_lint! { /// ## What it does - /// Makes sure that the argument of `static_assert` is statically known to be true. + /// Makes sure that instance attribute accesses are valid. /// /// ## Examples /// ```python diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 6784ff6a28359d..a4b47253ee387d 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -243,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, QualifiedType<'db>>, + /// The types and type qualifiers of every declaration in this region. + declarations: FxHashMap, TypeAndQualifiers<'db>>, /// The definitions that are deferred. deferred: FxHashSet>, @@ -283,7 +283,7 @@ impl<'db> TypeInference<'db> { } #[track_caller] - pub(crate) fn declaration_ty(&self, definition: Definition<'db>) -> QualifiedType<'db> { + pub(crate) fn declaration_ty(&self, definition: Definition<'db>) -> TypeAndQualifiers<'db> { self.declarations[&definition] } @@ -320,7 +320,7 @@ enum DeclaredAndInferredType<'db> { AreTheSame(Type<'db>), /// Declared and inferred types might be different, we need to check assignability. MightBeDifferent { - declared_ty: QualifiedType<'db>, + declared_ty: TypeAndQualifiers<'db>, inferred_ty: Type<'db>, }, } @@ -330,9 +330,9 @@ bitflags! { #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub(crate) struct TypeQualifiers: u8 { /// `typing.ClassVar` - const CLASS_VAR = 1 << 0; + const CLASS_VAR = 1 << 0; /// `typing.Final` - const FINAL = 1 << 1; + const FINAL = 1 << 1; } } @@ -343,17 +343,19 @@ impl TypeQualifiers { } /// When inferring the type of an annotation expression, we can also encounter type qualifiers -/// such as `ClassVar` or `Final`. This struct holds the type and the corresponding 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: `Final[Annotated[ClassVar[tuple[int]], "metadata"]]` would have the type `tuple[int]` -/// and the qualifiers `CLASS_VAR | FINAL`. +/// Example: `Annotated[ClassVar[tuple[int]], "metadata"]` would have type `tuple[int]` and +/// qualifiers `CLASS_VAR | FINAL`. #[derive(Clone, Debug, Copy, Eq, PartialEq)] -pub(crate) struct QualifiedType<'db> { +pub(crate) struct TypeAndQualifiers<'db> { inner: Type<'db>, qualifiers: TypeQualifiers, } -impl<'db> QualifiedType<'db> { +impl<'db> TypeAndQualifiers<'db> { pub(crate) fn ignore_qualifiers(&self) -> Type<'db> { self.inner } @@ -367,7 +369,7 @@ impl<'db> QualifiedType<'db> { } } -impl<'db> From> for QualifiedType<'db> { +impl<'db> From> for TypeAndQualifiers<'db> { fn from(ty: Type<'db>) -> Self { Self { inner: ty, @@ -939,7 +941,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, node: AnyNodeRef, declaration: Definition<'db>, - ty: QualifiedType<'db>, + ty: TypeAndQualifiers<'db>, ) { debug_assert!(declaration.is_declaration(self.db())); let use_def = self.index.use_def_map(declaration.file_scope(self.db())); @@ -4791,7 +4793,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, annotation: &ast::Expr, deferred_state: DeferredExpressionState, - ) -> QualifiedType<'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; @@ -4806,14 +4808,17 @@ 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) -> QualifiedType<'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 @@ -4855,7 +4860,7 @@ impl<'db> TypeInferenceBuilder<'db> { fn infer_string_annotation_expression( &mut self, string: &ast::ExprStringLiteral, - ) -> QualifiedType<'db> { + ) -> TypeAndQualifiers<'db> { match parse_string_annotation(&self.context, string) { Some(parsed) => { // String annotations are always evaluated in the deferred context. @@ -4876,7 +4881,7 @@ impl<'db> TypeInferenceBuilder<'db> { fn infer_type_expression_with_qualifiers( &mut self, expression: &ast::Expr, - ) -> QualifiedType<'db> { + ) -> TypeAndQualifiers<'db> { let qualified_ty = self.infer_type_expression_no_store(expression); self.store_expression_type(expression, qualified_ty.ignore_qualifiers()); qualified_ty @@ -4895,7 +4900,7 @@ impl<'db> TypeInferenceBuilder<'db> { fn infer_optional_type_expression( &mut self, expression: Option<&ast::Expr>, - ) -> Option> { + ) -> Option> { expression.map(|expr| self.infer_type_expression_with_qualifiers(expr)) } @@ -4906,7 +4911,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, expression: &ast::Expr, deferred_state: DeferredExpressionState, - ) -> QualifiedType<'db> { + ) -> TypeAndQualifiers<'db> { let previous_deferred_state = std::mem::replace(&mut self.deferred_state, deferred_state); let annotation_ty = self.infer_type_expression_with_qualifiers(expression); self.deferred_state = previous_deferred_state; @@ -4914,7 +4919,7 @@ impl<'db> TypeInferenceBuilder<'db> { } /// Infer the type of a type expression without storing the result. - fn infer_type_expression_no_store(&mut self, expression: &ast::Expr) -> QualifiedType<'db> { + fn infer_type_expression_no_store(&mut self, expression: &ast::Expr) -> TypeAndQualifiers<'db> { // https://typing.readthedocs.io/en/latest/spec/annotations.html#grammar-token-expression-grammar-type_expression let ty = match expression { ast::Expr::Name(name) => match name.ctx { @@ -5099,7 +5104,7 @@ impl<'db> TypeInferenceBuilder<'db> { fn infer_string_type_expression( &mut self, string: &ast::ExprStringLiteral, - ) -> QualifiedType<'db> { + ) -> TypeAndQualifiers<'db> { match parse_string_annotation(&self.context, string) { Some(parsed) => { // String annotations are always evaluated in the deferred context. @@ -5250,7 +5255,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, subscript: &ast::ExprSubscript, value_ty: Type<'db>, - ) -> QualifiedType<'db> { + ) -> TypeAndQualifiers<'db> { let ast::ExprSubscript { range: _, value: _, @@ -5277,7 +5282,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, subscript: &ast::ExprSubscript, known_instance: KnownInstanceType, - ) -> QualifiedType<'db> { + ) -> TypeAndQualifiers<'db> { let arguments_slice = &*subscript.slice; let ty = match known_instance { KnownInstanceType::Annotated => { From 5fab161a7521dbfdd4c952fb8577723142c6cbb8 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 14:39:44 +0100 Subject: [PATCH 06/27] Support bare ClassVar --- .../mdtest/type_qualifiers/classvar.md | 26 +++++++++------- crates/red_knot_python_semantic/src/types.rs | 19 +++++++----- .../src/types/infer.rs | 31 ++++++++++++++----- 3 files changed, 49 insertions(+), 27 deletions(-) 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 index e54b42ff5a97f0..4fce47c0be3c0d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -1,7 +1,7 @@ # `typing.ClassVar` [`typing.ClassVar`] is a type qualifier that is used to indicate that a class variable may not be -set on instances. For more details, see the [typing spec]. +set on instances. This test mostly makes sure that we "see" the type qualifier in an annotation. For more details on the semantics, see the [test on attributes](../attributes.md) test. @@ -12,22 +12,27 @@ the semantics, see the [test on attributes](../attributes.md) test. from typing import ClassVar, Annotated class C: - x: ClassVar[int] = 1 - y: Annotated[ClassVar[int], "the annotation for y"] = 1 - z: "ClassVar[int]" = 1 + a: ClassVar[int] = 1 + b: Annotated[ClassVar[int], "the annotation for b"] = 1 + c: ClassVar = 1 + d: "ClassVar[int]" = 1 -reveal_type(C.x) # revealed: int -reveal_type(C.y) # revealed: int -reveal_type(C.z) # revealed: int +reveal_type(C.a) # revealed: int +reveal_type(C.b) # revealed: int +# TODO: should be Unknown | Literal[1] +reveal_type(C.c) # revealed: Unknown +reveal_type(C.d) # revealed: int c = C() # error: [invalid-attribute-access] -c.x = 2 +c.a = 2 # error: [invalid-attribute-access] -c.y = 2 +c.b = 2 # error: [invalid-attribute-access] -c.z = 2 +c.c = 2 +# error: [invalid-attribute-access] +c.d = 2 ``` ## Too many arguments @@ -45,5 +50,4 @@ class C: x: ClassVar[int] = 1 ``` -[typing spec]: https://typing.readthedocs.io/en/latest/spec/class-compat.html#classvar [`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 9d803836b1dc14..012bedddbbaa91 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -2223,11 +2223,11 @@ impl<'db> Type<'db> { /// `Type::ClassLiteral(builtins.int)`, that is, it is the `int` class itself. As a type /// expression, it names the type `Type::Instance(builtins.int)`, that is, all objects whose /// `__class__` is `int`. - pub fn in_type_expression( + pub(crate) fn in_type_expression( &self, db: &'db dyn Db, - ) -> Result, InvalidTypeExpressionError<'db>> { - match self { + ) -> Result, InvalidTypeExpressionError<'db>> { + let ty = match self { // In a type expression, a bare `type` is interpreted as "instance of `type`", which is // equivalent to `type[object]`. Type::ClassLiteral(_) | Type::SubclassOf(_) => Ok(self.to_instance(db)), @@ -2261,7 +2261,7 @@ impl<'db> Type<'db> { let mut invalid_expressions = smallvec::SmallVec::default(); for element in union.elements(db) { match element.in_type_expression(db) { - Ok(type_expr) => builder = builder.add(type_expr), + Ok(type_expr) => builder = builder.add(type_expr.ignore_qualifiers()), Err(InvalidTypeExpressionError { fallback_type, invalid_expressions: new_invalid_expressions, @@ -2295,9 +2295,10 @@ impl<'db> Type<'db> { 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()) + return Ok(TypeAndQualifiers::new( + Type::unknown(), + TypeQualifiers::CLASS_VAR, + )); } Type::KnownInstance(KnownInstanceType::Literal) => Err(InvalidTypeExpressionError { invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareLiteral], @@ -2309,7 +2310,9 @@ impl<'db> Type<'db> { _ => Ok(todo_type!( "Unsupported or invalid type in a type expression" )), - } + }; + + ty.map(Into::into) } /// The type `NoneType` / `None` diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index a4b47253ee387d..92fdd2d6c99e67 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -356,6 +356,13 @@ pub(crate) struct TypeAndQualifiers<'db> { } impl<'db> TypeAndQualifiers<'db> { + pub(crate) fn new(ty: Type<'db>, qualifiers: TypeQualifiers) -> Self { + Self { + inner: ty, + qualifiers, + } + } + pub(crate) fn ignore_qualifiers(&self) -> Type<'db> { self.inner } @@ -4923,19 +4930,27 @@ impl<'db> TypeInferenceBuilder<'db> { // https://typing.readthedocs.io/en/latest/spec/annotations.html#grammar-token-expression-grammar-type_expression let ty = match expression { ast::Expr::Name(name) => match name.ctx { - ast::ExprContext::Load => self - .infer_name_expression(name) - .in_type_expression(self.db()) - .unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)), + ast::ExprContext::Load => { + return self + .infer_name_expression(name) + .in_type_expression(self.db()) + .unwrap_or_else(|error| { + error.into_fallback_type(&self.context, expression).into() + }); + } ast::ExprContext::Invalid => Type::unknown(), ast::ExprContext::Store | ast::ExprContext::Del => todo_type!(), }, ast::Expr::Attribute(attribute_expression) => match attribute_expression.ctx { - ast::ExprContext::Load => self - .infer_attribute_expression(attribute_expression) - .in_type_expression(self.db()) - .unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)), + ast::ExprContext::Load => { + return self + .infer_attribute_expression(attribute_expression) + .in_type_expression(self.db()) + .unwrap_or_else(|error| { + error.into_fallback_type(&self.context, expression).into() + }); + } ast::ExprContext::Invalid => Type::unknown(), ast::ExprContext::Store | ast::ExprContext::Del => todo_type!(), }, From e25f38cf878977f0c8aa77aaaebf44e75d83791f Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 14:46:14 +0100 Subject: [PATCH 07/27] Better error message --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 2 +- crates/red_knot_python_semantic/src/types/infer.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 6ec5c56bb42bd8..1d0908e9161b20 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -182,7 +182,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 -# error: [invalid-attribute-access] "Cannot assign to pure class variable `pure_class_variable1` from an instance" +# error: [invalid-attribute-access] "Cannot assign to pure class variable `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/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 92fdd2d6c99e67..09b034ccce50d9 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2075,7 +2075,8 @@ impl<'db> TypeInferenceBuilder<'db> { &INVALID_ATTRIBUTE_ACCESS, expr_attr.into(), format_args!( - "Cannot assign to pure class variable `{attr}` from an instance", + "Cannot assign to pure class variable `{attr}` from an instance of type `{ty}`", + ty = attr_value_ty.display(self.db()), ), ); } From 308c33e20e61f70200ce26a12fe4ab33339e85e9 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 14:47:30 +0100 Subject: [PATCH 08/27] =?UTF-8?q?Test=20for=20ClassVar[Annotated[T,=20?= =?UTF-8?q?=E2=80=A6]]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../resources/mdtest/type_qualifiers/classvar.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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 index 4fce47c0be3c0d..05aedab134ffbc 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -14,14 +14,16 @@ from typing import ClassVar, Annotated class C: a: ClassVar[int] = 1 b: Annotated[ClassVar[int], "the annotation for b"] = 1 - c: ClassVar = 1 - d: "ClassVar[int]" = 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.c) # revealed: Unknown -reveal_type(C.d) # revealed: int +reveal_type(C.d) # revealed: Unknown +reveal_type(C.e) # revealed: int c = C() @@ -33,6 +35,8 @@ c.b = 2 c.c = 2 # error: [invalid-attribute-access] c.d = 2 +# error: [invalid-attribute-access] +c.e = 2 ``` ## Too many arguments From 3f2eacebb0687de91f80dc451af50148b5a2b524 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 14:53:03 +0100 Subject: [PATCH 09/27] Wording --- .../resources/mdtest/type_qualifiers/classvar.md | 6 +++--- crates/red_knot_python_semantic/src/types/infer.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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 index 05aedab134ffbc..e131fcdf0dad55 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -1,10 +1,10 @@ # `typing.ClassVar` [`typing.ClassVar`] is a type qualifier that is used to indicate that a class variable may not be -set on instances. +written to from instances of that class. -This test mostly makes sure that we "see" the type qualifier in an annotation. For more details on -the semantics, see the [test on attributes](../attributes.md) test. +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 diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 09b034ccce50d9..efd487d23966d3 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -4884,7 +4884,7 @@ impl<'db> TypeInferenceBuilder<'db> { /// Type expressions impl<'db> TypeInferenceBuilder<'db> { - /// Infer the type of a type expression, including all type qualifiers such + /// Infer the type of a type expression, and collect all type qualifiers such /// as `ClassVar` or `Final`. fn infer_type_expression_with_qualifiers( &mut self, From 5ace6c65e022bccc8ba12c353ba9b1f08bb332c6 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 15:10:18 +0100 Subject: [PATCH 10/27] Move TypeAndQualifiers --- crates/red_knot_python_semantic/src/types.rs | 133 +++++++++++++----- .../src/types/infer.rs | 66 +-------- 2 files changed, 100 insertions(+), 99 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 012bedddbbaa91..743df60ae2148d 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; @@ -37,7 +38,6 @@ use crate::types::call::{ }; use crate::types::class_base::ClassBase; use crate::types::diagnostic::INVALID_TYPE_FORM; -use crate::types::infer::{TypeAndQualifiers, TypeQualifiers}; use crate::types::mro::{Mro, MroError, MroIterator}; use crate::types::narrow::narrowing_constraint; use crate::{Db, FxOrderSet, Module, Program, PythonVersion}; @@ -124,10 +124,10 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> bindings_ty(db, bindings) } // Symbol is possibly undeclared - Err((declared_ty, _)) => { + Err((declared_ty, _conflicting_types)) => { // 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.ignore_qualifiers().into() } } @@ -357,22 +357,19 @@ fn bindings_ty<'db>( } } +type SymbolAndQualifiers<'db> = (Symbol<'db>, TypeQualifiers); + /// The result of looking up a declared type from declarations; see [`declarations_ty`]. -type DeclaredTypeResult<'db> = Result<(Symbol<'db>, TypeQualifiers), (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 +/// `Ok((declared_type, type_qualifiers))`. If there are conflicting declarations, returns /// `Err((union_of_declared_types, conflicting_declared_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`. +/// Also returns a set of [`TypeQualifiers`] that have been specified on the declaration(s). fn declarations_ty<'db>( db: &'db dyn Db, declarations: DeclarationsIterator<'_, 'db>, @@ -435,7 +432,7 @@ fn declarations_ty<'db>( Ok((Symbol::Type(declared_ty, boundness), first.qualifiers())) } else { Err(( - declared_ty, + TypeAndQualifiers::new(declared_ty, first.qualifiers()), std::iter::once(first.ignore_qualifiers()) .chain(conflicting) .collect(), @@ -2478,6 +2475,69 @@ 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; + } +} + +impl TypeQualifiers { + pub(crate) fn is_class_var(self) -> bool { + self.contains(Self::CLASS_VAR) + } +} + +/// 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 +/// qualifiers `CLASS_VAR | FINAL`. +#[derive(Clone, Debug, Copy, Eq, PartialEq)] +pub(crate) struct TypeAndQualifiers<'db> { + inner: Type<'db>, + qualifiers: TypeQualifiers, +} + +impl<'db> TypeAndQualifiers<'db> { + pub(crate) fn new(ty: Type<'db>, qualifiers: TypeQualifiers) -> Self { + Self { + inner: ty, + qualifiers, + } + } + + /// Forget about type qualifiers and only return the inner type. + pub(crate) fn ignore_qualifiers(&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(ty: Type<'db>) -> Self { + Self { + inner: ty, + 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. @@ -4029,10 +4089,10 @@ impl<'db> Class<'db> { let body_scope = self.body_scope(db); let table = symbol_table(db, body_scope); - let symbol = if let Some(symbol_id) = table.symbol_id_by_name(name) { + if let Some(symbol) = table.symbol_id_by_name(name) { let use_def = use_def_map(db, body_scope); - let declarations = use_def.public_declarations(symbol_id); + let declarations = use_def.public_declarations(symbol); match declarations_ty(db, declarations) { Ok((Symbol::Type(declared_ty, _), qualifiers)) => { @@ -4040,40 +4100,41 @@ impl<'db> Class<'db> { // TODO: Eventually, we are going to process all decorators correctly. This is // just a temporary heuristic to provide a broad categorization into properties // and non-property methods. - let ty = if function - .has_decorator(db, KnownClass::Property.to_class_literal(db)) - { - todo_type!("@property") + if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) { + (todo_type!("@property").into(), qualifiers) } else { - todo_type!("bound method") - }; - return (ty.into(), qualifiers); + (todo_type!("bound method").into(), qualifiers) + } + } else { + (Symbol::Type(declared_ty, Boundness::Bound), qualifiers) } - - return (Symbol::Type(declared_ty, Boundness::Bound), qualifiers); } - Ok((Symbol::Unbound, _)) => { - let bindings = use_def.public_bindings(symbol_id); + Ok((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, _) => ( + Symbol::Type( + UnionType::from_elements(db, [Type::unknown(), ty]), + Boundness::Bound, + ), + qualifiers, ), - Symbol::Unbound => Symbol::Unbound, + Symbol::Unbound => (Symbol::Unbound, qualifiers), } } - Err((declared_ty, _)) => { + Err((declared_ty, _conflicting_declarations)) => { // Ignore conflicting declarations - declared_ty.into() + ( + declared_ty.ignore_qualifiers().into(), + declared_ty.qualifiers(), + ) } } } else { - Symbol::Unbound - }; - - (symbol, TypeQualifiers::empty()) + (Symbol::Unbound, TypeQualifiers::empty()) + } } /// Return `true` if this class appears to be a cyclic definition, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index efd487d23966d3..06bcebd6d000ee 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -28,7 +28,6 @@ //! definitions once the rest of the types in the scope have been inferred. use std::num::NonZeroU32; -use bitflags::bitflags; use itertools::{Either, Itertools}; use ruff_db::files::File; use ruff_db::parsed::parsed_module; @@ -68,7 +67,8 @@ use crate::types::{ FunctionType, InstanceType, IntersectionBuilder, IntersectionType, IterationOutcome, KnownClass, KnownFunction, KnownInstanceType, MetaclassCandidate, MetaclassErrorKind, SliceLiteralType, SubclassOfType, Symbol, Truthiness, TupleType, Type, TypeAliasType, - TypeArrayDisplay, TypeVarBoundOrConstraints, TypeVarInstance, UnionBuilder, UnionType, + TypeAndQualifiers, TypeArrayDisplay, TypeQualifiers, TypeVarBoundOrConstraints, + TypeVarInstance, UnionBuilder, UnionType, }; use crate::unpack::Unpack; use crate::util::subscript::{PyIndex, PySlice}; @@ -325,66 +325,6 @@ enum DeclaredAndInferredType<'db> { }, } -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; - } -} - -impl TypeQualifiers { - pub(crate) fn is_class_var(self) -> bool { - self.contains(Self::CLASS_VAR) - } -} - -/// 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 -/// qualifiers `CLASS_VAR | FINAL`. -#[derive(Clone, Debug, Copy, Eq, PartialEq)] -pub(crate) struct TypeAndQualifiers<'db> { - inner: Type<'db>, - qualifiers: TypeQualifiers, -} - -impl<'db> TypeAndQualifiers<'db> { - pub(crate) fn new(ty: Type<'db>, qualifiers: TypeQualifiers) -> Self { - Self { - inner: ty, - qualifiers, - } - } - - pub(crate) fn ignore_qualifiers(&self) -> Type<'db> { - self.inner - } - - pub(crate) fn add_qualifier(&mut self, qualifier: TypeQualifiers) { - self.qualifiers |= qualifier; - } - - pub(crate) fn qualifiers(&self) -> TypeQualifiers { - self.qualifiers - } -} - -impl<'db> From> for TypeAndQualifiers<'db> { - fn from(ty: Type<'db>) -> Self { - Self { - inner: ty, - qualifiers: TypeQualifiers::empty(), - } - } -} - /// Builder to infer all types in a region. /// /// A builder is used by creating it with [`new()`](TypeInferenceBuilder::new), and then calling @@ -933,7 +873,7 @@ impl<'db> TypeInferenceBuilder<'db> { conflicting.display(self.db()) ), ); - ty + ty.ignore_qualifiers() }); if !bound_ty.is_assignable_to(self.db(), declared_ty) { report_invalid_assignment(&self.context, node, declared_ty, bound_ty); From 7bf9ebb5d54143a5a2c10b917ab49739ef6704f8 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 15:19:35 +0100 Subject: [PATCH 11/27] Tuple struct --- crates/red_knot_python_semantic/src/types.rs | 18 ++++++++++++------ .../src/types/infer.rs | 7 +++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 743df60ae2148d..fd63ccdefbd068 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -93,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).map(|(ty, _)| ty); + let declared = declarations_ty(db, declarations).map(|SymbolAndQualifiers(ty, _)| ty); match declared { // Symbol is declared, trust the declared type @@ -357,7 +357,7 @@ fn bindings_ty<'db>( } } -type SymbolAndQualifiers<'db> = (Symbol<'db>, TypeQualifiers); +struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers); /// The result of looking up a declared type from declarations; see [`declarations_ty`]. type DeclaredTypeResult<'db> = @@ -429,7 +429,10 @@ fn declarations_ty<'db>( }; // TODO: deal with conflicting qualifiers? - Ok((Symbol::Type(declared_ty, boundness), first.qualifiers())) + Ok(SymbolAndQualifiers( + Symbol::Type(declared_ty, boundness), + first.qualifiers(), + )) } else { Err(( TypeAndQualifiers::new(declared_ty, first.qualifiers()), @@ -439,7 +442,10 @@ fn declarations_ty<'db>( )) } } else { - Ok((Symbol::Unbound, TypeQualifiers::empty())) + Ok(SymbolAndQualifiers( + Symbol::Unbound, + TypeQualifiers::empty(), + )) } } @@ -4095,7 +4101,7 @@ impl<'db> Class<'db> { let declarations = use_def.public_declarations(symbol); match declarations_ty(db, declarations) { - Ok((Symbol::Type(declared_ty, _), qualifiers)) => { + 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 @@ -4109,7 +4115,7 @@ impl<'db> Class<'db> { (Symbol::Type(declared_ty, Boundness::Bound), qualifiers) } } - Ok((Symbol::Unbound, qualifiers)) => { + Ok(SymbolAndQualifiers(Symbol::Unbound, qualifiers)) => { let bindings = use_def.public_bindings(symbol); let inferred_ty = bindings_ty(db, bindings); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 06bcebd6d000ee..b8376fe451fa52 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -66,8 +66,8 @@ 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, - TypeAndQualifiers, TypeArrayDisplay, TypeQualifiers, TypeVarBoundOrConstraints, + SliceLiteralType, SubclassOfType, Symbol, SymbolAndQualifiers, Truthiness, TupleType, Type, + TypeAliasType, TypeAndQualifiers, TypeArrayDisplay, TypeQualifiers, TypeVarBoundOrConstraints, TypeVarInstance, UnionBuilder, UnionType, }; use crate::unpack::Unpack; @@ -859,8 +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(|(ty, _)| ty) - .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())); From 8acb264688cd94cc3376a7f988828e909ed39546 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 15:34:09 +0100 Subject: [PATCH 12/27] Conflicting type qualifiers --- .../mdtest/type_qualifiers/classvar.md | 25 +++++++++++++++++++ crates/red_knot_python_semantic/src/types.rs | 21 +++++++++------- 2 files changed, 37 insertions(+), 9 deletions(-) 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 index e131fcdf0dad55..e79dd0bd0db1af 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -39,6 +39,31 @@ c.d = 2 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 diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index fd63ccdefbd068..5c9aff833944ae 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -366,10 +366,11 @@ type DeclaredTypeResult<'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, type_qualifiers))`. 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. /// -/// Also returns a set of [`TypeQualifiers`] that have been specified on the declaration(s). +/// 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>, @@ -407,6 +408,8 @@ fn declarations_ty<'db>( let mut conflicting: Vec> = vec![]; let declared_ty = if let Some(second) = types.next() { let ty_first = first.ignore_qualifiers(); + let mut qualifiers = first.qualifiers(); + let mut builder = UnionBuilder::new(db).add(ty_first); for other in std::iter::once(second).chain(types) { let other_ty = other.ignore_qualifiers(); @@ -414,10 +417,11 @@ fn declarations_ty<'db>( conflicting.push(other_ty); } builder = builder.add(other_ty); + qualifiers = qualifiers.union(other.qualifiers()); } - builder.build() + TypeAndQualifiers::new(builder.build(), qualifiers) } else { - first.ignore_qualifiers() + first }; if conflicting.is_empty() { let boundness = match undeclared_visibility { @@ -428,14 +432,13 @@ fn declarations_ty<'db>( Truthiness::Ambiguous => Boundness::PossiblyUnbound, }; - // TODO: deal with conflicting qualifiers? Ok(SymbolAndQualifiers( - Symbol::Type(declared_ty, boundness), - first.qualifiers(), + Symbol::Type(declared_ty.ignore_qualifiers(), boundness), + declared_ty.qualifiers(), )) } else { Err(( - TypeAndQualifiers::new(declared_ty, first.qualifiers()), + declared_ty, std::iter::once(first.ignore_qualifiers()) .chain(conflicting) .collect(), From 202cd4385b21bd523933367dffb25608b3005551 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 15:40:09 +0100 Subject: [PATCH 13/27] Add TODO --- .../red_knot_python_semantic/resources/mdtest/attributes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 1d0908e9161b20..9ee1ce5610704b 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]`. From 712fdaffc6921d6409c4afa6583d6ad74066ad80 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 15:58:37 +0100 Subject: [PATCH 14/27] Move diagnostic emitting code to proper place --- crates/red_knot_python_semantic/src/types.rs | 19 ++--- .../src/types/infer.rs | 77 +++++++++---------- 2 files changed, 43 insertions(+), 53 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 5c9aff833944ae..7b3350b481a7c5 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -127,7 +127,7 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> Err((declared_ty, _conflicting_types)) => { // Intentionally ignore conflicting declared types; that's not our problem, // it's the problem of the module we are importing from. - declared_ty.ignore_qualifiers().into() + declared_ty.inner_type().into() } } @@ -407,12 +407,12 @@ 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 ty_first = first.ignore_qualifiers(); + let ty_first = first.inner_type(); let mut qualifiers = first.qualifiers(); let mut builder = UnionBuilder::new(db).add(ty_first); for other in std::iter::once(second).chain(types) { - let other_ty = other.ignore_qualifiers(); + let other_ty = other.inner_type(); if !ty_first.is_equivalent_to(db, other_ty) { conflicting.push(other_ty); } @@ -433,13 +433,13 @@ fn declarations_ty<'db>( }; Ok(SymbolAndQualifiers( - Symbol::Type(declared_ty.ignore_qualifiers(), boundness), + Symbol::Type(declared_ty.inner_type(), boundness), declared_ty.qualifiers(), )) } else { Err(( declared_ty, - std::iter::once(first.ignore_qualifiers()) + std::iter::once(first.inner_type()) .chain(conflicting) .collect(), )) @@ -2267,7 +2267,7 @@ impl<'db> Type<'db> { let mut invalid_expressions = smallvec::SmallVec::default(); for element in union.elements(db) { match element.in_type_expression(db) { - Ok(type_expr) => builder = builder.add(type_expr.ignore_qualifiers()), + Ok(type_expr) => builder = builder.add(type_expr.inner_type()), Err(InvalidTypeExpressionError { fallback_type, invalid_expressions: new_invalid_expressions, @@ -2523,7 +2523,7 @@ impl<'db> TypeAndQualifiers<'db> { } /// Forget about type qualifiers and only return the inner type. - pub(crate) fn ignore_qualifiers(&self) -> Type<'db> { + pub(crate) fn inner_type(&self) -> Type<'db> { self.inner } @@ -4135,10 +4135,7 @@ impl<'db> Class<'db> { } Err((declared_ty, _conflicting_declarations)) => { // Ignore conflicting declarations - ( - declared_ty.ignore_qualifiers().into(), - declared_ty.qualifiers(), - ) + (declared_ty.inner_type().into(), declared_ty.qualifiers()) } } } else { diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index b8376fe451fa52..6ad9e5a1ee09fd 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -565,7 +565,7 @@ 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.ignore_qualifiers() + ty.inner_type() .into_class_literal() .map(|ty| (ty.class, class.node())) } else { @@ -872,7 +872,7 @@ impl<'db> TypeInferenceBuilder<'db> { conflicting.display(self.db()) ), ); - ty.ignore_qualifiers() + ty.inner_type() }); if !bound_ty.is_assignable_to(self.db(), declared_ty) { report_invalid_assignment(&self.context, node, declared_ty, bound_ty); @@ -896,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.ignore_qualifiers()) { + let ty = if inferred_ty.is_assignable_to(self.db(), ty.inner_type()) { ty } else { self.context.report_lint( @@ -904,7 +904,7 @@ impl<'db> TypeInferenceBuilder<'db> { node, format_args!( "Cannot declare type `{}` for inferred type `{}`", - ty.ignore_qualifiers().display(self.db()), + ty.inner_type().display(self.db()), inferred_ty.display(self.db()) ), ); @@ -928,17 +928,17 @@ impl<'db> TypeInferenceBuilder<'db> { declared_ty, inferred_ty, } => { - if inferred_ty.is_assignable_to(self.db(), declared_ty.ignore_qualifiers()) { + if inferred_ty.is_assignable_to(self.db(), declared_ty.inner_type()) { (declared_ty, inferred_ty) } else { report_invalid_assignment( &self.context, node, - declared_ty.ignore_qualifiers(), + declared_ty.inner_type(), inferred_ty, ); // if the assignment is invalid, fall back to assuming the annotation is correct - (declared_ty, declared_ty.ignore_qualifiers()) + (declared_ty, declared_ty.inner_type()) } } }; @@ -1769,7 +1769,7 @@ impl<'db> TypeInferenceBuilder<'db> { }; let default_ty = self .infer_optional_type_expression(default.as_deref()) - .map(|qualified_type| qualified_type.ignore_qualifiers()); + .map(|qualified_type| qualified_type.inner_type()); let ty = Type::KnownInstance(KnownInstanceType::TypeVar(TypeVarInstance::new( self.db(), name.id.clone(), @@ -1995,32 +1995,6 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_standalone_expression(value); } } - ast::Expr::Attribute( - expr_attr @ ast::ExprAttribute { - value: attr_value, - attr, - ctx, - .. - }, - ) => { - self.infer_standalone_expression(value); - let attr_value_ty = self.infer_expression(attr_value); - self.store_expression_type(expr_attr, attr_value_ty); - - if let (ast::ExprContext::Store, Type::Instance(instance)) = (ctx, attr_value_ty) { - let qualifiers = instance.class.instance_member(self.db(), attr).1; - if qualifiers.is_class_var() { - self.context.report_lint( - &INVALID_ATTRIBUTE_ACCESS, - expr_attr.into(), - format_args!( - "Cannot assign to pure class variable `{attr}` from an instance of type `{ty}`", - ty = attr_value_ty.display(self.db()), - ), - ); - } - } - } _ => { // TODO: Remove this once we handle all possible assignment targets. self.infer_standalone_expression(value); @@ -2108,7 +2082,7 @@ impl<'db> TypeInferenceBuilder<'db> { ); // Handle various singletons. - if let Type::Instance(InstanceType { class }) = declared_ty.ignore_qualifiers() { + if let Type::Instance(InstanceType { class }) = declared_ty.inner_type() { 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( @@ -2125,7 +2099,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.ignore_qualifiers() + declared_ty.inner_type() } else { inferred_ty }; @@ -3433,14 +3407,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 (ast::ExprContext::Store, Type::Instance(instance)) = (ctx, value_ty) { + let qualifiers = instance.class.instance_member(self.db(), attr).1; + if qualifiers.is_class_var() { + self.context.report_lint( + &INVALID_ATTRIBUTE_ACCESS, + attribute.into(), + format_args!( + "Cannot assign to pure class variable `{attr}` from an instance of type `{ty}`", + ty = value_ty.display(self.db()), + ), + ); + } + } + + Type::Never + } + ExprContext::Del => { self.infer_expression(value); Type::Never } @@ -4798,7 +4791,7 @@ impl<'db> TypeInferenceBuilder<'db> { type_expr => self.infer_type_expression_no_store(type_expr), }; - self.store_expression_type(annotation, annotation_ty.ignore_qualifiers()); + self.store_expression_type(annotation, annotation_ty.inner_type()); annotation_ty } @@ -4830,14 +4823,14 @@ impl<'db> TypeInferenceBuilder<'db> { expression: &ast::Expr, ) -> TypeAndQualifiers<'db> { let qualified_ty = self.infer_type_expression_no_store(expression); - self.store_expression_type(expression, qualified_ty.ignore_qualifiers()); + self.store_expression_type(expression, qualified_ty.inner_type()); qualified_ty } /// Infer the type of a type expression. fn infer_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { self.infer_type_expression_with_qualifiers(expression) - .ignore_qualifiers() + .inner_type() } /// Similar to [`infer_type_expression_with_qualifiers`], but accepts an optional @@ -5279,7 +5272,7 @@ impl<'db> TypeInferenceBuilder<'db> { } let qualified_ty = self.infer_type_expression_with_qualifiers(type_expr); - self.store_expression_type(arguments_slice, qualified_ty.ignore_qualifiers()); + self.store_expression_type(arguments_slice, qualified_ty.inner_type()); return qualified_ty; } KnownInstanceType::Literal => { From f05627927a53aa344e7df3cf04dba6c49d4f987e Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 19:27:34 +0100 Subject: [PATCH 15/27] Use SymbolAndQualifiers in more places --- crates/red_knot_python_semantic/src/types.rs | 38 ++++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 7b3350b481a7c5..6e83601da35fad 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -357,7 +357,7 @@ fn bindings_ty<'db>( } } -struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers); +pub(crate) struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers); /// The result of looking up a declared type from declarations; see [`declarations_ty`]. type DeclaredTypeResult<'db> = @@ -1685,7 +1685,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).0, + _ => { + let SymbolAndQualifiers(symbol, _) = class.instance_member(db, name); + symbol + } }, Type::Union(union) => { @@ -4056,22 +4059,19 @@ 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>, TypeQualifiers) { + 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 ( + return SymbolAndQualifiers( todo_type!("instance attribute on class with dynamic base").into(), TypeQualifiers::empty(), ); } ClassBase::Class(class) => { - let member = class.own_instance_member(db, name); - if !member.0.is_unbound() { + if let member @ SymbolAndQualifiers(Symbol::Type(_, _), _) = + class.own_instance_member(db, name) + { return member; } } @@ -4080,7 +4080,7 @@ impl<'db> Class<'db> { // TODO: The symbol is not present in any class body, but it could be implicitly // defined in `__init__` or other methods anywhere in the MRO. - ( + SymbolAndQualifiers( todo_type!("implicit instance attribute").into(), TypeQualifiers::empty(), ) @@ -4088,7 +4088,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>, TypeQualifiers) { + 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 @@ -4110,12 +4110,12 @@ impl<'db> Class<'db> { // just a temporary heuristic to provide a broad categorization into properties // and non-property methods. if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) { - (todo_type!("@property").into(), qualifiers) + SymbolAndQualifiers(todo_type!("@property").into(), qualifiers) } else { - (todo_type!("bound method").into(), qualifiers) + SymbolAndQualifiers(todo_type!("bound method").into(), qualifiers) } } else { - (Symbol::Type(declared_ty, Boundness::Bound), qualifiers) + SymbolAndQualifiers(Symbol::Type(declared_ty, Boundness::Bound), qualifiers) } } Ok(SymbolAndQualifiers(Symbol::Unbound, qualifiers)) => { @@ -4123,23 +4123,23 @@ impl<'db> Class<'db> { let inferred_ty = bindings_ty(db, bindings); match inferred_ty { - Symbol::Type(ty, _) => ( + Symbol::Type(ty, _) => SymbolAndQualifiers( Symbol::Type( UnionType::from_elements(db, [Type::unknown(), ty]), Boundness::Bound, ), qualifiers, ), - Symbol::Unbound => (Symbol::Unbound, qualifiers), + Symbol::Unbound => SymbolAndQualifiers(Symbol::Unbound, qualifiers), } } Err((declared_ty, _conflicting_declarations)) => { // Ignore conflicting declarations - (declared_ty.inner_type().into(), declared_ty.qualifiers()) + SymbolAndQualifiers(declared_ty.inner_type().into(), declared_ty.qualifiers()) } } } else { - (Symbol::Unbound, TypeQualifiers::empty()) + SymbolAndQualifiers(Symbol::Unbound, TypeQualifiers::empty()) } } From 22715aa3bc06d6d028c49e4236cca9000efe88fa Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 19:37:15 +0100 Subject: [PATCH 16/27] Some simplifications around SymbolAndQualifiers<'db> --- crates/red_knot_python_semantic/src/types.rs | 45 ++++++++++---------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 6e83601da35fad..14cbaa31a846eb 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -359,6 +359,19 @@ fn bindings_ty<'db>( pub(crate) struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers); +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 { + let symbol: Symbol<'db> = ty.into(); + symbol.into() + } +} + /// The result of looking up a declared type from declarations; see [`declarations_ty`]. type DeclaredTypeResult<'db> = Result, (TypeAndQualifiers<'db>, Box<[Type<'db>]>)>; @@ -445,10 +458,7 @@ fn declarations_ty<'db>( )) } } else { - Ok(SymbolAndQualifiers( - Symbol::Unbound, - TypeQualifiers::empty(), - )) + Ok(Symbol::Unbound.into()) } } @@ -2518,11 +2528,8 @@ pub(crate) struct TypeAndQualifiers<'db> { } impl<'db> TypeAndQualifiers<'db> { - pub(crate) fn new(ty: Type<'db>, qualifiers: TypeQualifiers) -> Self { - Self { - inner: ty, - qualifiers, - } + pub(crate) fn new(inner: Type<'db>, qualifiers: TypeQualifiers) -> Self { + Self { inner, qualifiers } } /// Forget about type qualifiers and only return the inner type. @@ -2542,9 +2549,9 @@ impl<'db> TypeAndQualifiers<'db> { } impl<'db> From> for TypeAndQualifiers<'db> { - fn from(ty: Type<'db>) -> Self { + fn from(inner: Type<'db>) -> Self { Self { - inner: ty, + inner, qualifiers: TypeQualifiers::empty(), } } @@ -4063,10 +4070,7 @@ impl<'db> Class<'db> { for superclass in self.iter_mro(db) { match superclass { ClassBase::Dynamic(_) => { - return SymbolAndQualifiers( - todo_type!("instance attribute on class with dynamic base").into(), - TypeQualifiers::empty(), - ); + return todo_type!("instance attribute on class with dynamic base").into(); } ClassBase::Class(class) => { if let member @ SymbolAndQualifiers(Symbol::Type(_, _), _) = @@ -4080,10 +4084,7 @@ impl<'db> Class<'db> { // TODO: The symbol is not present in any class body, but it could be implicitly // defined in `__init__` or other methods anywhere in the MRO. - SymbolAndQualifiers( - todo_type!("implicit instance attribute").into(), - TypeQualifiers::empty(), - ) + todo_type!("implicit instance attribute").into() } /// A helper function for `instance_member` that looks up the `name` attribute only on @@ -4110,9 +4111,9 @@ impl<'db> Class<'db> { // just a temporary heuristic to provide a broad categorization into properties // and non-property methods. if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) { - SymbolAndQualifiers(todo_type!("@property").into(), qualifiers) + todo_type!("@property").into() } else { - SymbolAndQualifiers(todo_type!("bound method").into(), qualifiers) + todo_type!("bound method").into() } } else { SymbolAndQualifiers(Symbol::Type(declared_ty, Boundness::Bound), qualifiers) @@ -4139,7 +4140,7 @@ impl<'db> Class<'db> { } } } else { - SymbolAndQualifiers(Symbol::Unbound, TypeQualifiers::empty()) + Symbol::Unbound.into() } } From 3c1a6702f451a56a16ae0792f057259655a93936 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 19:43:49 +0100 Subject: [PATCH 17/27] Add doc comment --- crates/red_knot_python_semantic/src/types.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 14cbaa31a846eb..4e2076653ca7bc 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -357,6 +357,18 @@ 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<'db> From> for SymbolAndQualifiers<'db> { @@ -367,8 +379,7 @@ impl<'db> From> for SymbolAndQualifiers<'db> { impl<'db> From> for SymbolAndQualifiers<'db> { fn from(ty: Type<'db>) -> Self { - let symbol: Symbol<'db> = ty.into(); - symbol.into() + SymbolAndQualifiers(ty.into(), TypeQualifiers::empty()) } } From cc2f5f8dcfb6878ed596527447f3bb8f1415aa2c Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 19:54:25 +0100 Subject: [PATCH 18/27] Special form => Type qualifier --- .../resources/mdtest/type_qualifiers/classvar.md | 2 +- crates/red_knot_python_semantic/src/types/infer.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index e79dd0bd0db1af..dc52227cecccce 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -68,7 +68,7 @@ c.a = 2 ```py class C: - # error: [invalid-type-form] "Special form `typing.ClassVar` expected exactly one type parameter" + # error: [invalid-type-form] "Type qualifier `typing.ClassVar` expects exactly one type parameter" x: ClassVar[int, str] = 1 ``` diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 6ad9e5a1ee09fd..2e7f2bccad5a4e 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -5422,7 +5422,7 @@ impl<'db> TypeInferenceBuilder<'db> { &INVALID_TYPE_FORM, subscript.into(), format_args!( - "Special form `{}` expected exactly one type parameter", + "Type qualifier `{}` expects exactly one type parameter", known_instance.repr(self.db()) ), ); From b8a8590c349cd07ee500321bac19fc8e41b312c3 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 19:58:59 +0100 Subject: [PATCH 19/27] Move is_class_var method --- crates/red_knot_python_semantic/src/types.rs | 12 ++++++------ crates/red_knot_python_semantic/src/types/infer.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 4e2076653ca7bc..6e641ec0f43942 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -371,6 +371,12 @@ fn bindings_ty<'db>( /// 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()) @@ -2519,12 +2525,6 @@ bitflags! { } } -impl TypeQualifiers { - pub(crate) fn is_class_var(self) -> bool { - self.contains(Self::CLASS_VAR) - } -} - /// 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 diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 2e7f2bccad5a4e..e3a098b69fda63 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -3418,8 +3418,8 @@ impl<'db> TypeInferenceBuilder<'db> { let value_ty = self.infer_expression(value); if let (ast::ExprContext::Store, Type::Instance(instance)) = (ctx, value_ty) { - let qualifiers = instance.class.instance_member(self.db(), attr).1; - if qualifiers.is_class_var() { + 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(), From d99015ba22cb6af1d77a382987ad03ea3932dbbc Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 22:53:25 +0100 Subject: [PATCH 20/27] Only return qualifiers from infer_annotation_expression --- crates/red_knot_python_semantic/src/types.rs | 25 +- .../src/types/infer.rs | 325 ++++++++++-------- 2 files changed, 191 insertions(+), 159 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 6e641ec0f43942..2cd658eafb5724 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -2262,8 +2262,8 @@ impl<'db> Type<'db> { pub(crate) fn in_type_expression( &self, db: &'db dyn Db, - ) -> Result, InvalidTypeExpressionError<'db>> { - let ty = match self { + ) -> Result, InvalidTypeExpressionError<'db>> { + match self { // In a type expression, a bare `type` is interpreted as "instance of `type`", which is // equivalent to `type[object]`. Type::ClassLiteral(_) | Type::SubclassOf(_) => Ok(self.to_instance(db)), @@ -2297,7 +2297,7 @@ impl<'db> Type<'db> { let mut invalid_expressions = smallvec::SmallVec::default(); for element in union.elements(db) { match element.in_type_expression(db) { - Ok(type_expr) => builder = builder.add(type_expr.inner_type()), + Ok(type_expr) => builder = builder.add(type_expr), Err(InvalidTypeExpressionError { fallback_type, invalid_expressions: new_invalid_expressions, @@ -2330,12 +2330,12 @@ impl<'db> Type<'db> { invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareAnnotated], fallback_type: Type::unknown(), }), - Type::KnownInstance(KnownInstanceType::ClassVar) => { - return Ok(TypeAndQualifiers::new( - Type::unknown(), - TypeQualifiers::CLASS_VAR, - )); - } + Type::KnownInstance(KnownInstanceType::ClassVar) => Err(InvalidTypeExpressionError { + invalid_expressions: smallvec::smallvec![ + InvalidTypeExpression::ClassVarInTypeExpression + ], + fallback_type: Type::unknown(), + }), Type::KnownInstance(KnownInstanceType::Literal) => Err(InvalidTypeExpressionError { invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareLiteral], fallback_type: Type::unknown(), @@ -2346,9 +2346,7 @@ impl<'db> Type<'db> { _ => Ok(todo_type!( "Unsupported or invalid type in a type expression" )), - }; - - ty.map(Into::into) + } } /// The type `NoneType` / `None` @@ -2601,6 +2599,8 @@ enum InvalidTypeExpression { BareAnnotated, /// `x: Literal` is invalid as an annotation BareLiteral, + /// The `ClassVar` type qualifier was used in a type expression (but can only be used in an annotation expression) + ClassVarInTypeExpression, } impl InvalidTypeExpression { @@ -2608,6 +2608,7 @@ 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 `ClassVar` is not allowed in type expressions (only in annotation expressions)", } } } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index e3a098b69fda63..9e90ff4b9ab746 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1767,9 +1767,7 @@ impl<'db> TypeInferenceBuilder<'db> { )), None => None, }; - let default_ty = self - .infer_optional_type_expression(default.as_deref()) - .map(|qualified_type| qualified_type.inner_type()); + let default_ty = self.infer_optional_type_expression(default.as_deref()); let ty = Type::KnownInstance(KnownInstanceType::TypeVar(TypeVarInstance::new( self.db(), name.id.clone(), @@ -4786,9 +4784,90 @@ impl<'db> TypeInferenceBuilder<'db> { 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) + } + _ => 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 { + self.report_invalid_arguments_to_annotated(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_type()); + inner_annotation_ty + } else { + self.infer_type_expression(slice); + Type::unknown().into() + } + } else { + self.report_invalid_arguments_to_annotated(subscript); + self.infer_annotation_expression_impl(slice) + } + } + Type::KnownInstance(KnownInstanceType::ClassVar) => 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 = KnownInstanceType::ClassVar.repr(self.db()), + ), + ); + Type::unknown().into() + } + _ => { + let mut type_and_qualifiers = + self.infer_annotation_expression_impl(slice); + type_and_qualifiers.add_qualifier(TypeQualifiers::CLASS_VAR); + 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.inner_type()); @@ -4816,74 +4895,56 @@ impl<'db> TypeInferenceBuilder<'db> { /// Type expressions impl<'db> TypeInferenceBuilder<'db> { - /// Infer the type of a type expression, and collect all type qualifiers such - /// as `ClassVar` or `Final`. - fn infer_type_expression_with_qualifiers( - &mut self, - expression: &ast::Expr, - ) -> TypeAndQualifiers<'db> { - let qualified_ty = self.infer_type_expression_no_store(expression); - self.store_expression_type(expression, qualified_ty.inner_type()); - qualified_ty - } - /// Infer the type of a type expression. fn infer_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { - self.infer_type_expression_with_qualifiers(expression) - .inner_type() + let ty = self.infer_type_expression_no_store(expression); + self.store_expression_type(expression, ty); + ty } - /// Similar to [`infer_type_expression_with_qualifiers`], but accepts an optional + /// Similar to [`infer_type_expression`], but accepts an optional /// type expression and returns [`None`] if the expression is [`None`]. /// - /// [`infer_type_expression_with_qualifiers`]: TypeInferenceBuilder::infer_type_expression_with_qualifiers + /// [`infer_type_expression`]: TypeInferenceBuilder::infer_type_expression fn infer_optional_type_expression( &mut self, expression: Option<&ast::Expr>, - ) -> Option> { - expression.map(|expr| self.infer_type_expression_with_qualifiers(expr)) + ) -> Option> { + expression.map(|expr| self.infer_type_expression(expr)) } - /// Similar to [`infer_type_expression_with_qualifiers`], but accepts a [`DeferredExpressionState`]. + /// Similar to [`infer_type_expression`], but accepts a [`DeferredExpressionState`]. /// - /// [`infer_type_expression_with_qualifiers`]: TypeInferenceBuilder::infer_type_expression_with_qualifiers + /// [`infer_type_expression`]: TypeInferenceBuilder::infer_type_expression fn infer_type_expression_with_state( &mut self, expression: &ast::Expr, deferred_state: DeferredExpressionState, - ) -> TypeAndQualifiers<'db> { + ) -> Type<'db> { let previous_deferred_state = std::mem::replace(&mut self.deferred_state, deferred_state); - let annotation_ty = self.infer_type_expression_with_qualifiers(expression); + let type_expression_ty = self.infer_type_expression(expression); self.deferred_state = previous_deferred_state; - annotation_ty + type_expression_ty } /// Infer the type of a type expression without storing the result. - fn infer_type_expression_no_store(&mut self, expression: &ast::Expr) -> TypeAndQualifiers<'db> { + fn infer_type_expression_no_store(&mut self, expression: &ast::Expr) -> Type<'db> { // https://typing.readthedocs.io/en/latest/spec/annotations.html#grammar-token-expression-grammar-type_expression - let ty = match expression { + match expression { ast::Expr::Name(name) => match name.ctx { - ast::ExprContext::Load => { - return self - .infer_name_expression(name) - .in_type_expression(self.db()) - .unwrap_or_else(|error| { - error.into_fallback_type(&self.context, expression).into() - }); - } + ast::ExprContext::Load => self + .infer_name_expression(name) + .in_type_expression(self.db()) + .unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)), ast::ExprContext::Invalid => Type::unknown(), ast::ExprContext::Store | ast::ExprContext::Del => todo_type!(), }, ast::Expr::Attribute(attribute_expression) => match attribute_expression.ctx { - ast::ExprContext::Load => { - return self - .infer_attribute_expression(attribute_expression) - .in_type_expression(self.db()) - .unwrap_or_else(|error| { - error.into_fallback_type(&self.context, expression).into() - }); - } + ast::ExprContext::Load => self + .infer_attribute_expression(attribute_expression) + .in_type_expression(self.db()) + .unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)), ast::ExprContext::Invalid => Type::unknown(), ast::ExprContext::Store | ast::ExprContext::Del => todo_type!(), }, @@ -4891,9 +4952,7 @@ impl<'db> TypeInferenceBuilder<'db> { ast::Expr::NoneLiteral(_literal) => Type::none(self.db()), // https://typing.readthedocs.io/en/latest/spec/annotations.html#string-annotations - ast::Expr::StringLiteral(string) => { - return self.infer_string_type_expression(string); - } + ast::Expr::StringLiteral(string) => self.infer_string_type_expression(string), // TODO: an Ellipsis literal *on its own* does not have any meaning in annotation // expressions, but is meaningful in the context of a number of special forms. @@ -4916,21 +4975,7 @@ impl<'db> TypeInferenceBuilder<'db> { let value_ty = self.infer_expression(value); - let qualified_ty = 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).into() - } - Some(KnownClass::Type) => { - self.infer_subclass_of_type_expression(slice).into() - } - _ => self.infer_subscript_type_expression(subscript, value_ty), - } - } - _ => self.infer_subscript_type_expression(subscript, value_ty), - }; - return qualified_ty; + self.infer_subscript_type_expression_no_store(subscript, slice, value_ty) } ast::Expr::BinOp(binary) => { @@ -5043,16 +5088,27 @@ impl<'db> TypeInferenceBuilder<'db> { Type::unknown() } ast::Expr::IpyEscapeCommand(_) => todo!("Implement Ipy escape command support"), - }; + } + } - ty.into() + 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, - ) -> TypeAndQualifiers<'db> { + fn infer_string_type_expression(&mut self, string: &ast::ExprStringLiteral) -> Type<'db> { match parse_string_annotation(&self.context, string) { Some(parsed) => { // String annotations are always evaluated in the deferred context. @@ -5061,7 +5117,7 @@ impl<'db> TypeInferenceBuilder<'db> { DeferredExpressionState::InStringAnnotation, ) } - None => Type::unknown().into(), + None => Type::unknown(), } } @@ -5156,7 +5212,7 @@ impl<'db> TypeInferenceBuilder<'db> { union_ty } ast::Expr::Tuple(_) => { - self.infer_type_expression_with_qualifiers(slice); + self.infer_type_expression(slice); self.context.report_lint( &INVALID_TYPE_FORM, slice.into(), @@ -5184,7 +5240,7 @@ impl<'db> TypeInferenceBuilder<'db> { _ => self.infer_subclass_of_type_expression(parameters), }, _ => { - self.infer_type_expression_with_qualifiers(parameters); + self.infer_type_expression(parameters); todo_type!("unsupported nested subscript in type[X]") } }; @@ -5193,7 +5249,7 @@ impl<'db> TypeInferenceBuilder<'db> { } // TODO: subscripts, etc. _ => { - self.infer_type_expression_with_qualifiers(slice); + self.infer_type_expression(slice); todo_type!("unsupported type[X] special form") } } @@ -5203,7 +5259,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, subscript: &ast::ExprSubscript, value_ty: Type<'db>, - ) -> TypeAndQualifiers<'db> { + ) -> Type<'db> { let ast::ExprSubscript { range: _, value: _, @@ -5216,64 +5272,64 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_parameterized_known_instance_type_expression(subscript, known_instance) } Type::Dynamic(DynamicType::Todo(_)) => { - self.infer_type_expression_with_qualifiers(slice); - value_ty.into() + self.infer_type_expression(slice); + value_ty } _ => { - self.infer_type_expression_with_qualifiers(slice); - todo_type!("generics").into() + self.infer_type_expression(slice); + todo_type!("generics") } } } + fn report_invalid_arguments_to_annotated(&self, subscript: &ast::ExprSubscript) { + 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)", + KnownInstanceType::Annotated.repr(self.db()) + ), + ); + } + fn infer_parameterized_known_instance_type_expression( &mut self, subscript: &ast::ExprSubscript, known_instance: KnownInstanceType, - ) -> TypeAndQualifiers<'db> { + ) -> Type<'db> { let arguments_slice = &*subscript.slice; - let ty = match known_instance { + 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(); + self.report_invalid_arguments_to_annotated(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 // giving better diagnostics later on. // Pyright also does this. Mypy doesn't; it falls back to `Any` instead. - return self.infer_type_expression_with_qualifiers(arguments_slice); + return self.infer_type_expression(arguments_slice); }; if arguments.len() < 2 { - report_invalid_arguments(); + self.report_invalid_arguments_to_annotated(subscript); } let [type_expr, metadata @ ..] = &arguments[..] else { - self.infer_type_expression_with_qualifiers(arguments_slice); - return Type::unknown().into(); + self.infer_type_expression(arguments_slice); + return Type::unknown(); }; for element in metadata { self.infer_expression(element); } - let qualified_ty = self.infer_type_expression_with_qualifiers(type_expr); - self.store_expression_type(arguments_slice, qualified_ty.inner_type()); - return qualified_ty; + let ty = self.infer_type_expression(type_expr); + self.store_expression_type(arguments_slice, ty); + ty } KnownInstanceType::Literal => { match self.infer_literal_parameter_type(arguments_slice) { @@ -5309,15 +5365,15 @@ impl<'db> TypeInferenceBuilder<'db> { _ => self.infer_type_expression(arguments_slice), }, KnownInstanceType::TypeVar(_) => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("TypeVar annotations") } KnownInstanceType::TypeAliasType(_) => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("Generic PEP-695 type alias") } KnownInstanceType::Callable => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("Callable types") } @@ -5372,95 +5428,72 @@ impl<'db> TypeInferenceBuilder<'db> { // TODO: Generics KnownInstanceType::ChainMap => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::ChainMap.to_instance(self.db()) } KnownInstanceType::OrderedDict => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::OrderedDict.to_instance(self.db()) } KnownInstanceType::Dict => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::Dict.to_instance(self.db()) } KnownInstanceType::List => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::List.to_instance(self.db()) } KnownInstanceType::DefaultDict => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::DefaultDict.to_instance(self.db()) } KnownInstanceType::Counter => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::Counter.to_instance(self.db()) } KnownInstanceType::Set => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::Set.to_instance(self.db()) } KnownInstanceType::FrozenSet => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::FrozenSet.to_instance(self.db()) } KnownInstanceType::Deque => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); KnownClass::Deque.to_instance(self.db()) } KnownInstanceType::ReadOnly => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`ReadOnly[]` type qualifier") } KnownInstanceType::NotRequired => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`NotRequired[]` type qualifier") } - KnownInstanceType::ClassVar | KnownInstanceType::Final => match arguments_slice { - ast::Expr::Tuple(_) => { - self.context.report_lint( - &INVALID_TYPE_FORM, - subscript.into(), - format_args!( - "Type qualifier `{}` expects exactly one type parameter", - known_instance.repr(self.db()) - ), - ); - Type::unknown() - } - _ => { - let mut argument_type = - self.infer_type_expression_with_qualifiers(arguments_slice); - match known_instance { - KnownInstanceType::ClassVar => { - argument_type.add_qualifier(TypeQualifiers::CLASS_VAR); - } - KnownInstanceType::Final => { - argument_type.add_qualifier(TypeQualifiers::FINAL); - } - _ => unreachable!(), - } - return argument_type; - } - }, + KnownInstanceType::ClassVar | KnownInstanceType::Final => { + // TODO: emit diagnostic: `ClassVar` and `Final` are not valid in type expressions + self.infer_type_expression(arguments_slice) + } KnownInstanceType::Required => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`Required[]` type qualifier") } KnownInstanceType::TypeIs => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`TypeIs[]` special form") } KnownInstanceType::TypeGuard => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`TypeGuard[]` special form") } KnownInstanceType::Concatenate => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`Concatenate[]` special form") } KnownInstanceType::Unpack => { - self.infer_type_expression_with_qualifiers(arguments_slice); + self.infer_type_expression(arguments_slice); todo_type!("`Unpack[]` special form") } KnownInstanceType::NoReturn @@ -5504,9 +5537,7 @@ impl<'db> TypeInferenceBuilder<'db> { } KnownInstanceType::Type => self.infer_subclass_of_type_expression(arguments_slice), KnownInstanceType::Tuple => self.infer_tuple_type_expression(arguments_slice), - }; - - ty.into() + } } fn infer_literal_parameter_type<'ast>( From 37e9eaf11545780db02cfff7b6ec473b8794f640 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 22:58:30 +0100 Subject: [PATCH 21/27] Minor changes --- crates/red_knot_python_semantic/src/types.rs | 4 ++-- crates/red_knot_python_semantic/src/types/infer.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 2cd658eafb5724..bb107e3a2f6047 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -124,7 +124,7 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> bindings_ty(db, bindings) } // Symbol is possibly undeclared - Err((declared_ty, _conflicting_types)) => { + 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.inner_type().into() @@ -2259,7 +2259,7 @@ impl<'db> Type<'db> { /// `Type::ClassLiteral(builtins.int)`, that is, it is the `int` class itself. As a type /// expression, it names the type `Type::Instance(builtins.int)`, that is, all objects whose /// `__class__` is `int`. - pub(crate) fn in_type_expression( + pub fn in_type_expression( &self, db: &'db dyn Db, ) -> Result, InvalidTypeExpressionError<'db>> { diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 9e90ff4b9ab746..7f1b42584f6d85 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -4902,8 +4902,8 @@ impl<'db> TypeInferenceBuilder<'db> { ty } - /// Similar to [`infer_type_expression`], but accepts an optional - /// type expression and returns [`None`] if the expression is [`None`]. + /// Similar to [`infer_type_expression`], but accepts an optional type expression and returns + /// [`None`] if the expression is [`None`]. /// /// [`infer_type_expression`]: TypeInferenceBuilder::infer_type_expression fn infer_optional_type_expression( @@ -4922,9 +4922,9 @@ impl<'db> TypeInferenceBuilder<'db> { deferred_state: DeferredExpressionState, ) -> Type<'db> { let previous_deferred_state = std::mem::replace(&mut self.deferred_state, deferred_state); - let type_expression_ty = self.infer_type_expression(expression); + let annotation_ty = self.infer_type_expression(expression); self.deferred_state = previous_deferred_state; - type_expression_ty + annotation_ty } /// Infer the type of a type expression without storing the result. From cbf384fbe3ff9698a197019b69ded1326b02df12 Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 17 Jan 2025 23:07:58 +0100 Subject: [PATCH 22/27] Add partial support for Final --- .../mdtest/type_qualifiers/classvar.md | 11 ++++++++ crates/red_knot_python_semantic/src/types.rs | 13 +++++++-- .../src/types/infer.rs | 28 ++++++++++++++++--- 3 files changed, 46 insertions(+), 6 deletions(-) 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 index dc52227cecccce..26060e685d3bbf 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_qualifiers/classvar.md @@ -72,6 +72,17 @@ class C: 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 diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index bb107e3a2f6047..2aa5410ee66956 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -2336,6 +2336,12 @@ impl<'db> Type<'db> { ], 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(), @@ -2599,8 +2605,10 @@ enum InvalidTypeExpression { BareAnnotated, /// `x: Literal` is invalid as an annotation BareLiteral, - /// The `ClassVar` type qualifier was used in a type expression (but can only be used in an annotation expression) + /// The `ClassVar` type qualifier was used in a type expression ClassVarInTypeExpression, + /// The `Final` type qualifier was used in a type expression + FinalInTypeExpression, } impl InvalidTypeExpression { @@ -2608,7 +2616,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 `ClassVar` is not allowed in type expressions (only in annotation expressions)", + 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)", } } } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 7f1b42584f6d85..80bd34adf4e37a 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -4791,6 +4791,9 @@ impl<'db> TypeInferenceBuilder<'db> { 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| { @@ -4840,14 +4843,16 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_annotation_expression_impl(slice) } } - Type::KnownInstance(KnownInstanceType::ClassVar) => match 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 = KnownInstanceType::ClassVar.repr(self.db()), + type_qualifier = known_instance.repr(self.db()), ), ); Type::unknown().into() @@ -4855,7 +4860,15 @@ impl<'db> TypeInferenceBuilder<'db> { _ => { let mut type_and_qualifiers = self.infer_annotation_expression_impl(slice); - type_and_qualifiers.add_qualifier(TypeQualifiers::CLASS_VAR); + 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 } }, @@ -5473,7 +5486,14 @@ impl<'db> TypeInferenceBuilder<'db> { todo_type!("`NotRequired[]` type qualifier") } KnownInstanceType::ClassVar | KnownInstanceType::Final => { - // TODO: emit diagnostic: `ClassVar` and `Final` are not valid in type expressions + 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 => { From 827abf6ed082ddd3834a044f62f73e26e58b6362 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 18 Jan 2025 13:21:44 +0100 Subject: [PATCH 23/27] Update doc comment --- crates/red_knot_python_semantic/src/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 2aa5410ee66956..0a4f470df9a94c 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -2534,8 +2534,8 @@ bitflags! { /// 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 -/// qualifiers `CLASS_VAR | FINAL`. +/// 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>, From e15ab5e18d2f68d7860b902d98cb3f2ec32024b9 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 18 Jan 2025 13:25:41 +0100 Subject: [PATCH 24/27] report_invalid_arguments_to_annotated as a free function --- .../src/types/diagnostic.rs | 19 ++++++++- .../src/types/infer.rs | 41 +++++++++---------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 59eab889c13b63..9699197527da84 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}; @@ -1080,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 80bd34adf4e37a..f565e05e8ab0b5 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -51,13 +51,13 @@ 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_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, + 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, }; use crate::types::mro::MroErrorKind; use crate::types::unpacker::{UnpackResult, Unpacker}; @@ -4821,7 +4821,11 @@ impl<'db> TypeInferenceBuilder<'db> { }) = slice { if arguments.len() < 2 { - self.report_invalid_arguments_to_annotated(subscript); + report_invalid_arguments_to_annotated( + self.db(), + &self.context, + subscript, + ); } if let [inner_annotation, metadata @ ..] = &arguments[..] { @@ -4839,7 +4843,11 @@ impl<'db> TypeInferenceBuilder<'db> { Type::unknown().into() } } else { - self.report_invalid_arguments_to_annotated(subscript); + report_invalid_arguments_to_annotated( + self.db(), + &self.context, + subscript, + ); self.infer_annotation_expression_impl(slice) } } @@ -5295,17 +5303,6 @@ impl<'db> TypeInferenceBuilder<'db> { } } - fn report_invalid_arguments_to_annotated(&self, subscript: &ast::ExprSubscript) { - 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)", - KnownInstanceType::Annotated.repr(self.db()) - ), - ); - } - fn infer_parameterized_known_instance_type_expression( &mut self, subscript: &ast::ExprSubscript, @@ -5318,7 +5315,7 @@ impl<'db> TypeInferenceBuilder<'db> { elts: arguments, .. }) = arguments_slice else { - self.report_invalid_arguments_to_annotated(subscript); + 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 @@ -5328,7 +5325,7 @@ impl<'db> TypeInferenceBuilder<'db> { }; if arguments.len() < 2 { - self.report_invalid_arguments_to_annotated(subscript); + report_invalid_arguments_to_annotated(self.db(), &self.context, subscript); } let [type_expr, metadata @ ..] = &arguments[..] else { From 02f4959d9885fc9113f61f1075569a3912f62cad Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 18 Jan 2025 13:27:02 +0100 Subject: [PATCH 25/27] pure class variable => ClassVar --- crates/red_knot_python_semantic/resources/mdtest/attributes.md | 2 +- crates/red_knot_python_semantic/src/types/infer.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/attributes.md b/crates/red_knot_python_semantic/resources/mdtest/attributes.md index 9ee1ce5610704b..2e352ed2642f12 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/attributes.md +++ b/crates/red_knot_python_semantic/resources/mdtest/attributes.md @@ -186,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 -# error: [invalid-attribute-access] "Cannot assign to pure class variable `pure_class_variable1` from an instance of type `C`" +# 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/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index f565e05e8ab0b5..766a979abcff46 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -3422,7 +3422,7 @@ impl<'db> TypeInferenceBuilder<'db> { &INVALID_ATTRIBUTE_ACCESS, attribute.into(), format_args!( - "Cannot assign to pure class variable `{attr}` from an instance of type `{ty}`", + "Cannot assign to ClassVar `{attr}` from an instance of type `{ty}`", ty = value_ty.display(self.db()), ), ); From cf49085f487299d2fd71436c24eceb27b7c5d56e Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 18 Jan 2025 13:33:32 +0100 Subject: [PATCH 26/27] inner_type => inner_ty --- crates/red_knot_python_semantic/src/types.rs | 14 ++++++------ .../src/types/infer.rs | 22 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 0a4f470df9a94c..cd52cc46e759f1 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -127,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.inner_type().into() + declared_ty.inner_ty().into() } } @@ -437,12 +437,12 @@ 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 ty_first = first.inner_type(); + 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) { - let other_ty = other.inner_type(); + let other_ty = other.inner_ty(); if !ty_first.is_equivalent_to(db, other_ty) { conflicting.push(other_ty); } @@ -463,13 +463,13 @@ fn declarations_ty<'db>( }; Ok(SymbolAndQualifiers( - Symbol::Type(declared_ty.inner_type(), boundness), + Symbol::Type(declared_ty.inner_ty(), boundness), declared_ty.qualifiers(), )) } else { Err(( declared_ty, - std::iter::once(first.inner_type()) + std::iter::once(first.inner_ty()) .chain(conflicting) .collect(), )) @@ -2548,7 +2548,7 @@ impl<'db> TypeAndQualifiers<'db> { } /// Forget about type qualifiers and only return the inner type. - pub(crate) fn inner_type(&self) -> Type<'db> { + pub(crate) fn inner_ty(&self) -> Type<'db> { self.inner } @@ -4157,7 +4157,7 @@ impl<'db> Class<'db> { } Err((declared_ty, _conflicting_declarations)) => { // Ignore conflicting declarations - SymbolAndQualifiers(declared_ty.inner_type().into(), declared_ty.qualifiers()) + SymbolAndQualifiers(declared_ty.inner_ty().into(), declared_ty.qualifiers()) } } } else { diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 766a979abcff46..1518dbb9e05df9 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -565,7 +565,7 @@ 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.inner_type() + ty.inner_ty() .into_class_literal() .map(|ty| (ty.class, class.node())) } else { @@ -872,7 +872,7 @@ impl<'db> TypeInferenceBuilder<'db> { conflicting.display(self.db()) ), ); - ty.inner_type() + ty.inner_ty() }); if !bound_ty.is_assignable_to(self.db(), declared_ty) { report_invalid_assignment(&self.context, node, declared_ty, bound_ty); @@ -896,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.inner_type()) { + let ty = if inferred_ty.is_assignable_to(self.db(), ty.inner_ty()) { ty } else { self.context.report_lint( @@ -904,7 +904,7 @@ impl<'db> TypeInferenceBuilder<'db> { node, format_args!( "Cannot declare type `{}` for inferred type `{}`", - ty.inner_type().display(self.db()), + ty.inner_ty().display(self.db()), inferred_ty.display(self.db()) ), ); @@ -928,17 +928,17 @@ impl<'db> TypeInferenceBuilder<'db> { declared_ty, inferred_ty, } => { - if inferred_ty.is_assignable_to(self.db(), declared_ty.inner_type()) { + 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.inner_type(), + declared_ty.inner_ty(), inferred_ty, ); // if the assignment is invalid, fall back to assuming the annotation is correct - (declared_ty, declared_ty.inner_type()) + (declared_ty, declared_ty.inner_ty()) } } }; @@ -2080,7 +2080,7 @@ impl<'db> TypeInferenceBuilder<'db> { ); // Handle various singletons. - if let Type::Instance(InstanceType { class }) = declared_ty.inner_type() { + 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( @@ -2097,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.inner_type() + declared_ty.inner_ty() } else { inferred_ty }; @@ -4836,7 +4836,7 @@ impl<'db> TypeInferenceBuilder<'db> { let inner_annotation_ty = self.infer_annotation_expression_impl(inner_annotation); - self.store_expression_type(slice, inner_annotation_ty.inner_type()); + self.store_expression_type(slice, inner_annotation_ty.inner_ty()); inner_annotation_ty } else { self.infer_type_expression(slice); @@ -4891,7 +4891,7 @@ impl<'db> TypeInferenceBuilder<'db> { type_expr => self.infer_type_expression_no_store(type_expr).into(), }; - self.store_expression_type(annotation, annotation_ty.inner_type()); + self.store_expression_type(annotation, annotation_ty.inner_ty()); annotation_ty } From c915773872ca0e32d3a5341d4b4a6a2676e45f0e Mon Sep 17 00:00:00 2001 From: David Peter Date: Sat, 18 Jan 2025 13:36:18 +0100 Subject: [PATCH 27/27] Remove superfluous match on ExprContext --- crates/red_knot_python_semantic/src/types/infer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 1518dbb9e05df9..e34790f1113b3b 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -3415,7 +3415,7 @@ impl<'db> TypeInferenceBuilder<'db> { ExprContext::Store => { let value_ty = self.infer_expression(value); - if let (ast::ExprContext::Store, Type::Instance(instance)) = (ctx, value_ty) { + 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(