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 6601893b3929c..f1ce8d5eb277d 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md @@ -282,6 +282,9 @@ class WithDefault(Generic[T, WithDefaultU]): ... reveal_type(WithDefault[str, str]()) # revealed: WithDefault[str, str] reveal_type(WithDefault[str]()) # revealed: WithDefault[str, int] + +# error: [invalid-type-arguments] "Too many type arguments to class `WithDefault`: expected between 1 and 2, got 3" +reveal_type(WithDefault[str, str, str]()) # revealed: WithDefault[Unknown, Unknown] ``` Type variable defaults can reference earlier type variables, but not later ones: diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/aliases.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/aliases.md index 5a3140926d70d..b8fb4c04962b6 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/aliases.md @@ -246,6 +246,45 @@ def _(g: G): reveal_type(g) # revealed: list[int] ``` +Self-referential defaults should not crash type inference: + +```py +# error: [cyclic-type-alias-definition] "Cyclic definition of `A`" +type A[T = A] = A[int] +``` + +A self-referential default that does not reference itself in the alias body should also not crash, +even when the default is evaluated (e.g., by omitting the type argument): + +```py +type B[T = B] = list[T] + +def _(x: B) -> None: + pass +``` + +Mutually-referential defaults (where two type aliases reference each other via their typevar +defaults) should also not crash: + +```py +type X[T = Y] = list[T] +type Y[U = X] = list[U] + +def _(x: X, y: Y) -> None: + pass +``` + +Indirect self-references through a chain of type aliases should also not crash: + +```py +type P[T = R] = list[T] +type Q[T = P] = list[T] +type R[T = Q] = list[T] + +def _(p: P) -> None: + pass +``` + ## Snapshots of verbose diagnostics 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 c09f91e804dd1..2eb4f6665994d 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md @@ -210,6 +210,9 @@ class WithDefault[T, U = int]: ... reveal_type(WithDefault[str, str]()) # revealed: WithDefault[str, str] reveal_type(WithDefault[str]()) # revealed: WithDefault[str, int] + +# error: [invalid-type-arguments] "Too many type arguments to class `WithDefault`: expected between 1 and 2, got 3" +reveal_type(WithDefault[str, str, str]()) # revealed: WithDefault[Unknown, Unknown] ``` ## Diagnostics for bad specializations diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/classes.md_-_Generic_classes___Leg\342\200\246_-_Specializing_generic\342\200\246_(5a066394f338af48).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/classes.md_-_Generic_classes___Leg\342\200\246_-_Specializing_generic\342\200\246_(5a066394f338af48).snap" index e92d1159c34d2..eb8400acf3b26 100644 --- "a/crates/ty_python_semantic/resources/mdtest/snapshots/classes.md_-_Generic_classes___Leg\342\200\246_-_Specializing_generic\342\200\246_(5a066394f338af48).snap" +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/classes.md_-_Generic_classes___Leg\342\200\246_-_Specializing_generic\342\200\246_(5a066394f338af48).snap" @@ -70,31 +70,34 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/generics/legacy/classes. 55 | 56 | reveal_type(WithDefault[str, str]()) # revealed: WithDefault[str, str] 57 | reveal_type(WithDefault[str]()) # revealed: WithDefault[str, int] -58 | from typing_extensions import TypeVar, Generic -59 | -60 | WithDefaultT1 = TypeVar("WithDefaultT1", default=int) -61 | WithDefaultT2 = TypeVar("WithDefaultT2", default=WithDefaultT1) +58 | +59 | # error: [invalid-type-arguments] "Too many type arguments to class `WithDefault`: expected between 1 and 2, got 3" +60 | reveal_type(WithDefault[str, str, str]()) # revealed: WithDefault[Unknown, Unknown] +61 | from typing_extensions import TypeVar, Generic 62 | -63 | # This is fine: WithDefaultT2's default references WithDefaultT1, which comes before it -64 | class GoodOrder(Generic[WithDefaultT1, WithDefaultT2]): ... +63 | WithDefaultT1 = TypeVar("WithDefaultT1", default=int) +64 | WithDefaultT2 = TypeVar("WithDefaultT2", default=WithDefaultT1) 65 | -66 | # error: [invalid-generic-class] "Default of `WithDefaultT2` cannot reference later type parameter `WithDefaultT1`" -67 | class BadOrder(Generic[WithDefaultT2, WithDefaultT1]): ... +66 | # This is fine: WithDefaultT2's default references WithDefaultT1, which comes before it +67 | class GoodOrder(Generic[WithDefaultT1, WithDefaultT2]): ... 68 | -69 | WithDefaultU = TypeVar("WithDefaultU", default=int) -70 | -71 | # error: [invalid-generic-class] -72 | class AlsoBadOrder(Generic[WithDefaultT2, WithDefaultT1, WithDefaultU]): ... -73 | from typing_extensions import TypeVar, Generic -74 | -75 | StartT = TypeVar("StartT", default=int) -76 | StopT = TypeVar("StopT", default=StartT) -77 | StepT = TypeVar("StepT", default=int | None) -78 | Start2T = TypeVar("Start2T", default="StopT") -79 | Stop2T = TypeVar("Stop2T", default=int) -80 | -81 | # error: [invalid-generic-class] "Default of `Start2T` cannot reference out-of-scope type variable `StopT`" -82 | class Bad(Generic[Start2T, Stop2T, StepT]): ... +69 | # error: [invalid-generic-class] "Default of `WithDefaultT2` cannot reference later type parameter `WithDefaultT1`" +70 | class BadOrder(Generic[WithDefaultT2, WithDefaultT1]): ... +71 | +72 | WithDefaultU = TypeVar("WithDefaultU", default=int) +73 | +74 | # error: [invalid-generic-class] +75 | class AlsoBadOrder(Generic[WithDefaultT2, WithDefaultT1, WithDefaultU]): ... +76 | from typing_extensions import TypeVar, Generic +77 | +78 | StartT = TypeVar("StartT", default=int) +79 | StopT = TypeVar("StopT", default=StartT) +80 | StepT = TypeVar("StepT", default=int | None) +81 | Start2T = TypeVar("Start2T", default="StopT") +82 | Stop2T = TypeVar("Stop2T", default=int) +83 | +84 | # error: [invalid-generic-class] "Default of `Start2T` cannot reference out-of-scope type variable `StopT`" +85 | class Bad(Generic[Start2T, Stop2T, StepT]): ... ``` # Diagnostics @@ -179,26 +182,39 @@ info: rule `invalid-type-arguments` is enabled by default ``` +``` +error[invalid-type-arguments]: Too many type arguments to class `WithDefault`: expected between 1 and 2, got 3 + --> src/mdtest_snippet.py:60:35 + | +59 | # error: [invalid-type-arguments] "Too many type arguments to class `WithDefault`: expected between 1 and 2, got 3" +60 | reveal_type(WithDefault[str, str, str]()) # revealed: WithDefault[Unknown, Unknown] + | ^^^ +61 | from typing_extensions import TypeVar, Generic + | +info: rule `invalid-type-arguments` is enabled by default + +``` + ``` error[invalid-generic-class]: Default of `WithDefaultT2` cannot reference later type parameter `WithDefaultT1` - --> src/mdtest_snippet.py:67:7 + --> src/mdtest_snippet.py:70:7 | -66 | # error: [invalid-generic-class] "Default of `WithDefaultT2` cannot reference later type parameter `WithDefaultT1`" -67 | class BadOrder(Generic[WithDefaultT2, WithDefaultT1]): ... +69 | # error: [invalid-generic-class] "Default of `WithDefaultT2` cannot reference later type parameter `WithDefaultT1`" +70 | class BadOrder(Generic[WithDefaultT2, WithDefaultT1]): ... | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -68 | -69 | WithDefaultU = TypeVar("WithDefaultU", default=int) +71 | +72 | WithDefaultU = TypeVar("WithDefaultU", default=int) | - ::: src/mdtest_snippet.py:60:1 + ::: src/mdtest_snippet.py:63:1 | -58 | from typing_extensions import TypeVar, Generic -59 | -60 | WithDefaultT1 = TypeVar("WithDefaultT1", default=int) +61 | from typing_extensions import TypeVar, Generic +62 | +63 | WithDefaultT1 = TypeVar("WithDefaultT1", default=int) | ----------------------------------------------------- `WithDefaultT1` defined here -61 | WithDefaultT2 = TypeVar("WithDefaultT2", default=WithDefaultT1) +64 | WithDefaultT2 = TypeVar("WithDefaultT2", default=WithDefaultT1) | --------------------------------------------------------------- `WithDefaultT2` defined here -62 | -63 | # This is fine: WithDefaultT2's default references WithDefaultT1, which comes before it +65 | +66 | # This is fine: WithDefaultT2's default references WithDefaultT1, which comes before it | info: rule `invalid-generic-class` is enabled by default @@ -206,23 +222,23 @@ info: rule `invalid-generic-class` is enabled by default ``` error[invalid-generic-class]: Default of `WithDefaultT2` cannot reference later type parameter `WithDefaultT1` - --> src/mdtest_snippet.py:72:7 + --> src/mdtest_snippet.py:75:7 | -71 | # error: [invalid-generic-class] -72 | class AlsoBadOrder(Generic[WithDefaultT2, WithDefaultT1, WithDefaultU]): ... +74 | # error: [invalid-generic-class] +75 | class AlsoBadOrder(Generic[WithDefaultT2, WithDefaultT1, WithDefaultU]): ... | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -73 | from typing_extensions import TypeVar, Generic +76 | from typing_extensions import TypeVar, Generic | - ::: src/mdtest_snippet.py:60:1 + ::: src/mdtest_snippet.py:63:1 | -58 | from typing_extensions import TypeVar, Generic -59 | -60 | WithDefaultT1 = TypeVar("WithDefaultT1", default=int) +61 | from typing_extensions import TypeVar, Generic +62 | +63 | WithDefaultT1 = TypeVar("WithDefaultT1", default=int) | ----------------------------------------------------- `WithDefaultT1` defined here -61 | WithDefaultT2 = TypeVar("WithDefaultT2", default=WithDefaultT1) +64 | WithDefaultT2 = TypeVar("WithDefaultT2", default=WithDefaultT1) | --------------------------------------------------------------- `WithDefaultT2` defined here -62 | -63 | # This is fine: WithDefaultT2's default references WithDefaultT1, which comes before it +65 | +66 | # This is fine: WithDefaultT2's default references WithDefaultT1, which comes before it | info: rule `invalid-generic-class` is enabled by default @@ -230,19 +246,19 @@ info: rule `invalid-generic-class` is enabled by default ``` error[invalid-generic-class]: Default of `Start2T` cannot reference out-of-scope type variable `StopT` - --> src/mdtest_snippet.py:82:7 + --> src/mdtest_snippet.py:85:7 | -81 | # error: [invalid-generic-class] "Default of `Start2T` cannot reference out-of-scope type variable `StopT`" -82 | class Bad(Generic[Start2T, Stop2T, StepT]): ... +84 | # error: [invalid-generic-class] "Default of `Start2T` cannot reference out-of-scope type variable `StopT`" +85 | class Bad(Generic[Start2T, Stop2T, StepT]): ... | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - ::: src/mdtest_snippet.py:78:1 + ::: src/mdtest_snippet.py:81:1 | -76 | StopT = TypeVar("StopT", default=StartT) -77 | StepT = TypeVar("StepT", default=int | None) -78 | Start2T = TypeVar("Start2T", default="StopT") +79 | StopT = TypeVar("StopT", default=StartT) +80 | StepT = TypeVar("StepT", default=int | None) +81 | Start2T = TypeVar("Start2T", default="StopT") | --------------------------------------------- `Start2T` defined here -79 | Stop2T = TypeVar("Stop2T", default=int) +82 | Stop2T = TypeVar("Stop2T", default=int) | info: rule `invalid-generic-class` is enabled by default diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index f5c5aae538294..6a9a3aedc950d 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -1,7 +1,7 @@ use compact_str::ToCompactString; use itertools::Itertools; use ruff_diagnostics::{Edit, Fix}; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use std::borrow::Cow; use std::cell::RefCell; @@ -238,6 +238,11 @@ pub(crate) struct FindLegacyTypeVars; pub(crate) type SpecializationVisitor<'db> = CycleDetector, ()>; pub(crate) struct VisitSpecialization; +/// A [`CycleDetector`] that is used in `TypeVarInstance::default_type`. +pub(crate) type TypeVarDefaultVisitor<'db> = + CycleDetector, Option>>; +pub(crate) struct VisitTypeVarDefault; + /// How a generic type has been specialized. /// /// This matters only if there is at least one invariant type parameter. @@ -7086,9 +7091,20 @@ impl<'db> TypeVarInstance<'db> { } pub(crate) fn default_type(self, db: &'db dyn Db) -> Option> { - self._default(db).and_then(|d| match d { - TypeVarDefaultEvaluation::Eager(ty) => Some(ty), - TypeVarDefaultEvaluation::Lazy => self.lazy_default(db), + let visitor = TypeVarDefaultVisitor::new(None); + self.default_type_impl(db, &visitor) + } + + fn default_type_impl( + self, + db: &'db dyn Db, + visitor: &TypeVarDefaultVisitor<'db>, + ) -> Option> { + visitor.visit(self, || { + self._default(db).and_then(|default| match default { + TypeVarDefaultEvaluation::Eager(ty) => Some(ty), + TypeVarDefaultEvaluation::Lazy => self.lazy_default_impl(db, visitor), + }) }) } @@ -7159,15 +7175,87 @@ impl<'db> TypeVarInstance<'db> { )) } - fn type_is_self_referential(self, db: &'db dyn Db, ty: Type<'db>) -> bool { - let identity = self.identity(db); - any_over_type(db, ty, false, |ty| match ty { - Type::TypeVar(bound_typevar) => identity == bound_typevar.typevar(db).identity(db), - Type::KnownInstance(KnownInstanceType::TypeVar(typevar)) => { - identity == typevar.identity(db) + fn type_is_self_referential( + self, + db: &'db dyn Db, + ty: Type<'db>, + visitor: &TypeVarDefaultVisitor<'db>, + ) -> bool { + #[derive(Copy, Clone)] + struct State<'db, 'a> { + db: &'db dyn Db, + visitor: &'a TypeVarDefaultVisitor<'db>, + seen_typevars: &'a RefCell>>, + seen_type_aliases: &'a RefCell>>, + } + + fn typevar_default_is_self_referential<'db>( + state: State<'db, '_>, + typevar: TypeVarInstance<'db>, + self_identity: TypeVarIdentity<'db>, + ) -> bool { + if typevar.identity(state.db) == self_identity { + return true; } - _ => false, - }) + + if !state.seen_typevars.borrow_mut().insert(typevar) { + return false; + } + + typevar + .default_type_impl(state.db, state.visitor) + .is_some_and(|default_ty| { + type_is_self_referential_impl(state, default_ty, self_identity) + }) + } + + fn type_alias_is_self_referential<'db>( + state: State<'db, '_>, + type_alias: TypeAliasType<'db>, + self_identity: TypeVarIdentity<'db>, + ) -> bool { + if !state.seen_type_aliases.borrow_mut().insert(type_alias) { + return false; + } + + type_is_self_referential_impl(state, type_alias.raw_value_type(state.db), self_identity) + } + + fn type_is_self_referential_impl<'db>( + state: State<'db, '_>, + ty: Type<'db>, + self_identity: TypeVarIdentity<'db>, + ) -> bool { + any_over_type(state.db, ty, false, |inner_ty| match inner_ty { + Type::TypeVar(bound_typevar) => typevar_default_is_self_referential( + state, + bound_typevar.typevar(state.db), + self_identity, + ), + Type::KnownInstance(KnownInstanceType::TypeVar(typevar)) => { + typevar_default_is_self_referential(state, typevar, self_identity) + } + Type::TypeAlias(alias) => { + type_alias_is_self_referential(state, alias, self_identity) + } + Type::KnownInstance(KnownInstanceType::TypeAliasType(alias)) => { + type_alias_is_self_referential(state, alias, self_identity) + } + _ => false, + }) + } + + let seen_typevars = RefCell::new(FxHashSet::default()); + let seen_type_aliases = RefCell::new(FxHashSet::default()); + + let state = State { + db, + visitor, + seen_typevars: &seen_typevars, + seen_type_aliases: &seen_type_aliases, + }; + + type_is_self_referential_impl(state, ty, self.identity(db)) } /// Returns the "unchecked" upper bound of a type variable instance. @@ -7348,12 +7436,21 @@ impl<'db> TypeVarInstance<'db> { } fn lazy_default(self, db: &'db dyn Db) -> Option> { + let visitor = TypeVarDefaultVisitor::new(None); + self.lazy_default_impl(db, &visitor) + } + + fn lazy_default_impl( + self, + db: &'db dyn Db, + visitor: &TypeVarDefaultVisitor<'db>, + ) -> Option> { let default = self.lazy_default_unchecked(db)?; // Unlike bounds/constraints, default types are allowed to be generic (https://peps.python.org/pep-0696/#using-another-type-parameter-as-default). // Here we simply check for non-self-referential. // TODO: We should also check for non-forward references. - if self.type_is_self_referential(db, default) { + if self.type_is_self_referential(db, default, visitor) { return None; } @@ -7777,14 +7874,7 @@ impl<'db> BoundTypeVarInstance<'db> { /// `BoundTypeVarInstance`. As part of binding `U` we must also bind its default value /// (resulting in `T@C`). pub(crate) fn default_type(self, db: &'db dyn Db) -> Option> { - let binding_context = self.binding_context(db); - self.typevar(db).default_type(db).map(|ty| { - ty.apply_type_mapping( - db, - &TypeMapping::BindLegacyTypevars(binding_context), - TypeContext::default(), - ) - }) + bound_typevar_default_type(db, self) } fn materialize_impl( @@ -7812,6 +7902,36 @@ impl<'db> BoundTypeVarInstance<'db> { } } +#[salsa::tracked( + cycle_initial=|_, _, _| None, + cycle_fn=bound_typevar_default_type_cycle_recover, + heap_size=ruff_memory_usage::heap_size +)] +fn bound_typevar_default_type<'db>( + db: &'db dyn Db, + bound_typevar: BoundTypeVarInstance<'db>, +) -> Option> { + let binding_context = bound_typevar.binding_context(db); + bound_typevar.typevar(db).default_type(db).map(|ty| { + ty.apply_type_mapping( + db, + &TypeMapping::BindLegacyTypevars(binding_context), + TypeContext::default(), + ) + }) +} + +#[expect(clippy::ref_option)] +fn bound_typevar_default_type_cycle_recover<'db>( + _db: &'db dyn Db, + _cycle: &salsa::Cycle, + _previous_default: &Option>, + _default: Option>, + _bound_typevar: BoundTypeVarInstance<'db>, +) -> Option> { + None +} + /// Whether a typevar default is eagerly specified or lazily evaluated. #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, salsa::Update, get_size2::GetSize)] pub enum TypeVarDefaultEvaluation<'db> {