diff --git a/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md b/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md index d952ed9e86..ad1fe65958 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md @@ -37,7 +37,7 @@ class RepeatedTypevar(Generic[T, T]): ... You can only specialize `typing.Generic` with typevars (TODO: or param specs or typevar tuples). ```py -# error: [invalid-argument-type] "`` is not a valid argument to `typing.Generic`" +# error: [invalid-argument-type] "`` is not a valid argument to `Generic`" class GenericOfType(Generic[int]): ... ``` diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md index c205d46617..1d3fee229b 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md @@ -67,6 +67,41 @@ T = TypeVar("T") # error: [invalid-generic-class] "Cannot both inherit from `typing.Generic` and use PEP 695 type variables" class BothGenericSyntaxes[U](Generic[T]): ... + +reveal_type(BothGenericSyntaxes.__mro__) # revealed: tuple[, Unknown, ] + +# error: [invalid-generic-class] "Cannot both inherit from `typing.Generic` and use PEP 695 type variables" +# error: [invalid-base] "Cannot inherit from plain `Generic`" +class DoublyInvalid[T](Generic): ... + +reveal_type(DoublyInvalid.__mro__) # revealed: tuple[, Unknown, ] +``` + +Generic classes implicitly inherit from `Generic`: + +```py +class Foo[T]: ... + +# revealed: tuple[, typing.Generic, ] +reveal_type(Foo.__mro__) +# revealed: tuple[, typing.Generic, ] +reveal_type(Foo[int].__mro__) + +class A: ... +class Bar[T](A): ... + +# revealed: tuple[, , typing.Generic, ] +reveal_type(Bar.__mro__) +# revealed: tuple[, , typing.Generic, ] +reveal_type(Bar[int].__mro__) + +class B: ... +class Baz[T](A, B): ... + +# revealed: tuple[, , , typing.Generic, ] +reveal_type(Baz.__mro__) +# revealed: tuple[, , , typing.Generic, ] +reveal_type(Baz[int].__mro__) ``` ## Specializing generic classes explicitly diff --git a/crates/ty_python_semantic/resources/mdtest/mro.md b/crates/ty_python_semantic/resources/mdtest/mro.md index 47d607fcda..1a73deb594 100644 --- a/crates/ty_python_semantic/resources/mdtest/mro.md +++ b/crates/ty_python_semantic/resources/mdtest/mro.md @@ -644,14 +644,14 @@ reveal_type(C.__mro__) # revealed: tuple[, Unknown, class D(D.a): a: D -#reveal_type(D.__class__) # revealed: +reveal_type(D.__class__) # revealed: reveal_type(D.__mro__) # revealed: tuple[, Unknown, ] class E[T](E.a): ... -#reveal_type(E.__class__) # revealed: -reveal_type(E.__mro__) # revealed: tuple[, Unknown, ] +reveal_type(E.__class__) # revealed: +reveal_type(E.__mro__) # revealed: tuple[, Unknown, typing.Generic, ] class F[T](F(), F): ... # error: [cyclic-class-definition] -#reveal_type(F.__class__) # revealed: +reveal_type(F.__class__) # revealed: type[Unknown] reveal_type(F.__mro__) # revealed: tuple[, Unknown, ] ``` diff --git a/crates/ty_python_semantic/resources/mdtest/protocols.md b/crates/ty_python_semantic/resources/mdtest/protocols.md index a2116c8f97..763694dacb 100644 --- a/crates/ty_python_semantic/resources/mdtest/protocols.md +++ b/crates/ty_python_semantic/resources/mdtest/protocols.md @@ -58,9 +58,13 @@ class Bar1(Protocol[T], Generic[T]): class Bar2[T](Protocol): x: T -# error: [invalid-generic-class] "Cannot both inherit from subscripted `typing.Protocol` and use PEP 695 type variables" +# error: [invalid-generic-class] "Cannot both inherit from subscripted `Protocol` and use PEP 695 type variables" class Bar3[T](Protocol[T]): x: T + +# Note that this class definition *will* actually succeed at runtime, +# unlike classes that combine PEP-695 type parameters with inheritance from `Generic[]` +reveal_type(Bar3.__mro__) # revealed: tuple[, typing.Protocol, typing.Generic, ] ``` It's an error to include both bare `Protocol` and subscripted `Protocol[]` in the bases list diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 83870384ee..0aa8e785fe 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -223,8 +223,11 @@ impl<'db> ClassType<'db> { } } - pub(super) const fn is_generic(self) -> bool { - matches!(self, Self::Generic(_)) + pub(super) fn has_pep_695_type_params(self, db: &'db dyn Db) -> bool { + match self { + Self::NonGeneric(class) => class.has_pep_695_type_params(db), + Self::Generic(generic) => generic.origin(db).has_pep_695_type_params(db), + } } /// Returns the class literal and specialization for this class. For a non-generic class, this @@ -573,6 +576,10 @@ impl<'db> ClassLiteral<'db> { .or_else(|| self.inherited_legacy_generic_context(db)) } + pub(crate) fn has_pep_695_type_params(self, db: &'db dyn Db) -> bool { + self.pep695_generic_context(db).is_some() + } + #[salsa::tracked(cycle_fn=pep695_generic_context_cycle_recover, cycle_initial=pep695_generic_context_cycle_initial)] pub(crate) fn pep695_generic_context(self, db: &'db dyn Db) -> Option> { let scope = self.body_scope(db); diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index c3d5a61fb7..4866b73efd 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -27,7 +27,6 @@ use crate::{Db, FxOrderSet}; pub struct GenericContext<'db> { #[returns(ref)] pub(crate) variables: FxOrderSet>, - pub(crate) origin: GenericContextOrigin, } impl<'db> GenericContext<'db> { @@ -41,7 +40,7 @@ impl<'db> GenericContext<'db> { .iter() .filter_map(|type_param| Self::variable_from_type_param(db, index, type_param)) .collect(); - Self::new(db, variables, GenericContextOrigin::TypeParameterList) + Self::new(db, variables) } fn variable_from_type_param( @@ -87,11 +86,7 @@ impl<'db> GenericContext<'db> { if variables.is_empty() { return None; } - Some(Self::new( - db, - variables, - GenericContextOrigin::LegacyGenericFunction, - )) + Some(Self::new(db, variables)) } /// Creates a generic context from the legacy `TypeVar`s that appear in class's base class @@ -107,7 +102,7 @@ impl<'db> GenericContext<'db> { if variables.is_empty() { return None; } - Some(Self::new(db, variables, GenericContextOrigin::Inherited)) + Some(Self::new(db, variables)) } pub(crate) fn len(self, db: &'db dyn Db) -> usize { @@ -244,46 +239,21 @@ impl<'db> GenericContext<'db> { .iter() .map(|ty| ty.normalized(db)) .collect(); - Self::new(db, variables, self.origin(db)) + Self::new(db, variables) } } -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -pub enum GenericContextOrigin { - LegacyBase(LegacyGenericBase), - Inherited, - LegacyGenericFunction, - TypeParameterList, -} - -impl GenericContextOrigin { - pub(crate) const fn as_str(self) -> &'static str { - match self { - Self::LegacyBase(base) => base.as_str(), - Self::Inherited => "inherited", - Self::LegacyGenericFunction => "legacy generic function", - Self::TypeParameterList => "type parameter list", - } - } -} - -impl std::fmt::Display for GenericContextOrigin { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.as_str()) - } -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -pub enum LegacyGenericBase { +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(super) enum LegacyGenericBase { Generic, Protocol, } impl LegacyGenericBase { - pub(crate) const fn as_str(self) -> &'static str { + const fn as_str(self) -> &'static str { match self { - Self::Generic => "`typing.Generic`", - Self::Protocol => "subscripted `typing.Protocol`", + Self::Generic => "Generic", + Self::Protocol => "Protocol", } } } @@ -294,12 +264,6 @@ impl std::fmt::Display for LegacyGenericBase { } } -impl From for GenericContextOrigin { - fn from(base: LegacyGenericBase) -> Self { - Self::LegacyBase(base) - } -} - /// An assignment of a specific type to each type variable in a generic scope. /// /// TODO: Handle nested specializations better, with actual parent links to the specialization of diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index b60ac4c9ba..d3b93fcab0 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -108,7 +108,7 @@ use super::diagnostic::{ report_runtime_check_against_non_runtime_checkable_protocol, report_slice_step_size_zero, report_unresolved_reference, }; -use super::generics::{GenericContextOrigin, LegacyGenericBase}; +use super::generics::LegacyGenericBase; use super::slots::check_class_slots; use super::string_annotation::{ BYTE_STRING_TYPE_ANNOTATION, FSTRING_TYPE_ANNOTATION, parse_string_annotation, @@ -856,6 +856,25 @@ impl<'db> TypeInferenceBuilder<'db> { } continue; } + // Note that unlike several of the other errors caught in this function, + // this does not lead to the class creation failing at runtime, + // but it is semantically invalid. + Type::KnownInstance(KnownInstanceType::Protocol(Some(_))) => { + if class_node.type_params.is_none() { + continue; + } + let Some(builder) = self + .context + .report_lint(&INVALID_GENERIC_CLASS, &class_node.bases()[i]) + else { + continue; + }; + builder.into_diagnostic( + "Cannot both inherit from subscripted `Protocol` \ + and use PEP 695 type variables", + ); + continue; + } Type::ClassLiteral(class) => class, // dynamic/unknown bases are never `@final` _ => continue, @@ -917,7 +936,7 @@ impl<'db> TypeInferenceBuilder<'db> { { builder.into_diagnostic(format_args!( "Cannot create a consistent method resolution order (MRO) \ - for class `{}` with bases list `[{}]`", + for class `{}` with bases list `[{}]`", class.name(self.db()), bases_list .iter() @@ -926,6 +945,16 @@ impl<'db> TypeInferenceBuilder<'db> { )); } } + MroErrorKind::Pep695ClassWithGenericInheritance => { + if let Some(builder) = + self.context.report_lint(&INVALID_GENERIC_CLASS, class_node) + { + builder.into_diagnostic( + "Cannot both inherit from `typing.Generic` \ + and use PEP 695 type variables", + ); + } + } MroErrorKind::InheritanceCycle => { if let Some(builder) = self .context @@ -1022,21 +1051,6 @@ impl<'db> TypeInferenceBuilder<'db> { } } - // (5) Check that a generic class does not have invalid or conflicting generic - // contexts. - if class.pep695_generic_context(self.db()).is_some() { - if let Some(legacy_context) = class.legacy_generic_context(self.db()) { - if let Some(builder) = - self.context.report_lint(&INVALID_GENERIC_CLASS, class_node) - { - builder.into_diagnostic(format_args!( - "Cannot both inherit from {} and use PEP 695 type variables", - legacy_context.origin(self.db()) - )); - } - } - } - if let (Some(legacy), Some(inherited)) = ( class.legacy_generic_context(self.db()), class.inherited_legacy_generic_context(self.db()), @@ -7628,7 +7642,7 @@ impl<'db> TypeInferenceBuilder<'db> { self.context.report_lint(&INVALID_ARGUMENT_TYPE, value_node) { builder.into_diagnostic(format_args!( - "`{}` is not a valid argument to {origin}", + "`{}` is not a valid argument to `{origin}`", typevar.display(self.db()), )); } @@ -7636,9 +7650,7 @@ impl<'db> TypeInferenceBuilder<'db> { } }) .collect(); - typevars.map(|typevars| { - GenericContext::new(self.db(), typevars, GenericContextOrigin::from(origin)) - }) + typevars.map(|typevars| GenericContext::new(self.db(), typevars)) } fn infer_slice_expression(&mut self, slice: &ast::ExprSlice) -> Type<'db> { diff --git a/crates/ty_python_semantic/src/types/mro.rs b/crates/ty_python_semantic/src/types/mro.rs index 72f3eceafa..44957fb929 100644 --- a/crates/ty_python_semantic/src/types/mro.rs +++ b/crates/ty_python_semantic/src/types/mro.rs @@ -67,10 +67,41 @@ impl<'db> Mro<'db> { fn of_class_impl( db: &'db dyn Db, class: ClassType<'db>, - bases: &[Type<'db>], + original_bases: &[Type<'db>], specialization: Option>, ) -> Result> { - match bases { + /// Possibly add `Generic` to the resolved bases list. + /// + /// This function is called in two cases: + /// - If we encounter a subscripted `Generic` in the original bases list + /// (`Generic[T]` or similar) + /// - If the class has PEP-695 type parameters, + /// `Generic` is [implicitly appended] to the bases list at runtime + /// + /// Whether or not `Generic` is added to the bases list depends on: + /// - Whether `Protocol` is present in the original bases list + /// - Whether any of the bases yet to be visited in the original bases list + /// is a generic alias (which would therefore have `Generic` in its MRO) + /// + /// This function emulates the behavior of `typing._GenericAlias.__mro_entries__` at + /// . + /// + /// [implicitly inherits]: https://docs.python.org/3/reference/compound_stmts.html#generic-classes + fn maybe_add_generic<'db>( + resolved_bases: &mut Vec>, + original_bases: &[Type<'db>], + remaining_bases: &[Type<'db>], + ) { + if original_bases.contains(&Type::KnownInstance(KnownInstanceType::Protocol(None))) { + return; + } + if remaining_bases.iter().any(Type::is_generic_alias) { + return; + } + resolved_bases.push(ClassBase::Generic); + } + + match original_bases { // `builtins.object` is the special case: // the only class in Python that has an MRO with length <2 [] if class.is_object(db) => Ok(Self::from([ @@ -93,7 +124,7 @@ impl<'db> Mro<'db> { // ``` [] => { // e.g. `class Foo[T]: ...` implicitly has `Generic` inserted into its bases - if class.is_generic() { + if class.has_pep_695_type_params(db) { Ok(Self::from([ ClassBase::Class(class), ClassBase::Generic, @@ -110,13 +141,14 @@ impl<'db> Mro<'db> { // but it's a common case (i.e., worth optimizing for), // and the `c3_merge` function requires lots of allocations. [single_base] - if !matches!( - single_base, - Type::GenericAlias(_) - | Type::KnownInstance( - KnownInstanceType::Generic(_) | KnownInstanceType::Protocol(_) - ) - ) => + if !class.has_pep_695_type_params(db) + && !matches!( + single_base, + Type::GenericAlias(_) + | Type::KnownInstance( + KnownInstanceType::Generic(_) | KnownInstanceType::Protocol(_) + ) + ) => { ClassBase::try_from_type(db, *single_base).map_or_else( || Err(MroErrorKind::InvalidBases(Box::from([(0, *single_base)]))), @@ -137,31 +169,21 @@ impl<'db> Mro<'db> { // We'll fallback to a full implementation of the C3-merge algorithm to determine // what MRO Python will give this class at runtime // (if an MRO is indeed resolvable at all!) - original_bases => { + _ => { let mut resolved_bases = vec![]; let mut invalid_bases = vec![]; for (i, base) in original_bases.iter().enumerate() { - // This emulates the behavior of `typing._GenericAlias.__mro_entries__` at - // . - // - // Note that emit a diagnostic for inheriting from bare (unsubscripted) `Generic` elsewhere + // Note that we emit a diagnostic for inheriting from bare (unsubscripted) `Generic` elsewhere // (see `infer::TypeInferenceBuilder::check_class_definitions`), // which is why we only care about `KnownInstanceType::Generic(Some(_))`, // not `KnownInstanceType::Generic(None)`. if let Type::KnownInstance(KnownInstanceType::Generic(Some(_))) = base { - if original_bases - .contains(&Type::KnownInstance(KnownInstanceType::Protocol(None))) - { - continue; - } - if original_bases[i + 1..] - .iter() - .any(|b| b.is_generic_alias() && b != base) - { - continue; - } - resolved_bases.push(ClassBase::Generic); + maybe_add_generic( + &mut resolved_bases, + original_bases, + &original_bases[i + 1..], + ); } else { match ClassBase::try_from_type(db, *base) { Some(valid_base) => resolved_bases.push(valid_base), @@ -174,6 +196,12 @@ impl<'db> Mro<'db> { return Err(MroErrorKind::InvalidBases(invalid_bases.into_boxed_slice())); } + // `Generic` is implicitly added to the bases list of a class that has PEP-695 type parameters + // (documented at https://docs.python.org/3/reference/compound_stmts.html#generic-classes) + if class.has_pep_695_type_params(db) { + maybe_add_generic(&mut resolved_bases, original_bases, &[]); + } + let mut seqs = vec![VecDeque::from([ClassBase::Class(class)])]; for base in &resolved_bases { if base.has_cyclic_mro(db) { @@ -192,6 +220,18 @@ impl<'db> Mro<'db> { return Ok(mro); } + // We now know that the MRO is unresolvable through the C3-merge algorithm. + // The rest of this function is dedicated to figuring out the best error message + // to report to the user. + + if class.has_pep_695_type_params(db) + && original_bases.iter().any(|base| { + matches!(base, Type::KnownInstance(KnownInstanceType::Generic(_))) + }) + { + return Err(MroErrorKind::Pep695ClassWithGenericInheritance); + } + let mut duplicate_dynamic_bases = false; let duplicate_bases: Vec> = { @@ -416,6 +456,9 @@ pub(super) enum MroErrorKind<'db> { /// See [`DuplicateBaseError`] for more details. DuplicateBases(Box<[DuplicateBaseError<'db>]>), + /// The class uses PEP-695 parameters and also inherits from `Generic[]`. + Pep695ClassWithGenericInheritance, + /// A cycle was encountered resolving the class' bases. InheritanceCycle,