[ty] Handle explicit variance in legacy typevars (#17897)

We now track the variance of each typevar, and obey the `covariant` and
`contravariant` parameters to the legacy `TypeVar` constructor. We still
don't yet infer variance for PEP-695 typevars or for the
`infer_variance` legacy constructor parameter.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Carl Meyer <carl@astral.sh>
This commit is contained in:
Douglas Creager 2025-05-07 08:44:51 -04:00 committed by GitHub
parent c5e299e796
commit 0d9b6a0975
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 368 additions and 25 deletions

View file

@ -932,6 +932,7 @@ impl<'db> Type<'db> {
typevar.name(db).clone(),
typevar.definition(db),
Some(TypeVarBoundOrConstraints::UpperBound(bound.normalized(db))),
typevar.variance(db),
typevar.default_ty(db),
typevar.kind(db),
))
@ -942,6 +943,7 @@ impl<'db> Type<'db> {
typevar.name(db).clone(),
typevar.definition(db),
Some(TypeVarBoundOrConstraints::Constraints(union.normalized(db))),
typevar.variance(db),
typevar.default_ty(db),
typevar.kind(db),
))
@ -5618,6 +5620,9 @@ pub struct TypeVarInstance<'db> {
/// The upper bound or constraint on the type of this TypeVar
bound_or_constraints: Option<TypeVarBoundOrConstraints<'db>>,
/// The variance of the TypeVar
variance: TypeVarVariance,
/// The default type for this TypeVar
default_ty: Option<Type<'db>>,
@ -5646,7 +5651,15 @@ impl<'db> TypeVarInstance<'db> {
}
}
#[derive(Clone, Debug, Hash, PartialEq, Eq, salsa::Update)]
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, salsa::Update)]
pub enum TypeVarVariance {
Invariant,
Covariant,
Contravariant,
Bivariant,
}
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, salsa::Update)]
pub enum TypeVarBoundOrConstraints<'db> {
UpperBound(Type<'db>),
Constraints(UnionType<'db>),

View file

@ -5,7 +5,7 @@ use crate::semantic_index::SemanticIndex;
use crate::types::signatures::{Parameter, Parameters, Signature};
use crate::types::{
declaration_type, KnownInstanceType, Type, TypeVarBoundOrConstraints, TypeVarInstance,
UnionType,
TypeVarVariance, UnionType,
};
use crate::{Db, FxOrderSet};
@ -260,7 +260,7 @@ impl<'db> Specialization<'db> {
return false;
}
for ((_typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
for ((typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
.zip(self.types(db))
.zip(other.types(db))
{
@ -268,13 +268,19 @@ impl<'db> Specialization<'db> {
return false;
}
// TODO: We currently treat all typevars as invariant. Once we track the actual
// variance of each typevar, these checks should change:
// Subtyping of each type in the specialization depends on the variance of the
// corresponding typevar:
// - covariant: verify that self_type <: other_type
// - contravariant: verify that other_type <: self_type
// - invariant: verify that self_type == other_type
// - bivariant: skip, can't make subtyping false
if !self_type.is_equivalent_to(db, *other_type) {
let compatible = match typevar.variance(db) {
TypeVarVariance::Invariant => self_type.is_equivalent_to(db, *other_type),
TypeVarVariance::Covariant => self_type.is_subtype_of(db, *other_type),
TypeVarVariance::Contravariant => other_type.is_subtype_of(db, *self_type),
TypeVarVariance::Bivariant => true,
};
if !compatible {
return false;
}
}
@ -288,7 +294,7 @@ impl<'db> Specialization<'db> {
return false;
}
for ((_typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
for ((typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
.zip(self.types(db))
.zip(other.types(db))
{
@ -296,13 +302,19 @@ impl<'db> Specialization<'db> {
return false;
}
// TODO: We currently treat all typevars as invariant. Once we track the actual
// variance of each typevar, these checks should change:
// Equivalence of each type in the specialization depends on the variance of the
// corresponding typevar:
// - covariant: verify that self_type == other_type
// - contravariant: verify that other_type == self_type
// - invariant: verify that self_type == other_type
// - bivariant: skip, can't make equivalence false
if !self_type.is_equivalent_to(db, *other_type) {
let compatible = match typevar.variance(db) {
TypeVarVariance::Invariant
| TypeVarVariance::Covariant
| TypeVarVariance::Contravariant => self_type.is_equivalent_to(db, *other_type),
TypeVarVariance::Bivariant => true,
};
if !compatible {
return false;
}
}
@ -316,7 +328,7 @@ impl<'db> Specialization<'db> {
return false;
}
for ((_typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
for ((typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
.zip(self.types(db))
.zip(other.types(db))
{
@ -324,13 +336,19 @@ impl<'db> Specialization<'db> {
continue;
}
// TODO: We currently treat all typevars as invariant. Once we track the actual
// variance of each typevar, these checks should change:
// Assignability of each type in the specialization depends on the variance of the
// corresponding typevar:
// - covariant: verify that self_type <: other_type
// - contravariant: verify that other_type <: self_type
// - invariant: verify that self_type == other_type
// - bivariant: skip, can't make assignability false
if !self_type.is_gradual_equivalent_to(db, *other_type) {
let compatible = match typevar.variance(db) {
TypeVarVariance::Invariant => self_type.is_gradual_equivalent_to(db, *other_type),
TypeVarVariance::Covariant => self_type.is_assignable_to(db, *other_type),
TypeVarVariance::Contravariant => other_type.is_assignable_to(db, *self_type),
TypeVarVariance::Bivariant => true,
};
if !compatible {
return false;
}
}
@ -348,17 +366,25 @@ impl<'db> Specialization<'db> {
return false;
}
for ((_typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
for ((typevar, self_type), other_type) in (generic_context.variables(db).into_iter())
.zip(self.types(db))
.zip(other.types(db))
{
// TODO: We currently treat all typevars as invariant. Once we track the actual
// variance of each typevar, these checks should change:
// Equivalence of each type in the specialization depends on the variance of the
// corresponding typevar:
// - covariant: verify that self_type == other_type
// - contravariant: verify that other_type == self_type
// - invariant: verify that self_type == other_type
// - bivariant: skip, can't make equivalence false
if !self_type.is_gradual_equivalent_to(db, *other_type) {
let compatible = match typevar.variance(db) {
TypeVarVariance::Invariant
| TypeVarVariance::Covariant
| TypeVarVariance::Contravariant => {
self_type.is_gradual_equivalent_to(db, *other_type)
}
TypeVarVariance::Bivariant => true,
};
if !compatible {
return false;
}
}

View file

@ -89,8 +89,8 @@ use crate::types::{
MemberLookupPolicy, MetaclassCandidate, Parameter, ParameterForm, Parameters, Signature,
Signatures, SliceLiteralType, StringLiteralType, SubclassOfType, Symbol, SymbolAndQualifiers,
Truthiness, TupleType, Type, TypeAliasType, TypeAndQualifiers, TypeArrayDisplay,
TypeQualifiers, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, UnionBuilder,
UnionType,
TypeQualifiers, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, TypeVarVariance,
UnionBuilder, UnionType,
};
use crate::unpack::{Unpack, UnpackPosition};
use crate::util::subscript::{PyIndex, PySlice};
@ -2504,6 +2504,7 @@ impl<'db> TypeInferenceBuilder<'db> {
name.id.clone(),
definition,
bound_or_constraint,
TypeVarVariance::Invariant, // TODO: infer this
default_ty,
TypeVarKind::Pep695,
)));
@ -4990,12 +4991,72 @@ impl<'db> TypeInferenceBuilder<'db> {
continue;
};
let [Some(name_param), constraints, bound, default, _contravariant, _covariant, _infer_variance] =
let [Some(name_param), constraints, bound, default, contravariant, covariant, _infer_variance] =
overload.parameter_types()
else {
continue;
};
let covariant = match covariant {
Some(ty) => ty.bool(self.db()),
None => Truthiness::AlwaysFalse,
};
let contravariant = match contravariant {
Some(ty) => ty.bool(self.db()),
None => Truthiness::AlwaysFalse,
};
let variance = match (contravariant, covariant) {
(Truthiness::Ambiguous, _) => {
if let Some(builder) = self.context.report_lint(
&INVALID_LEGACY_TYPE_VARIABLE,
call_expression,
) {
builder.into_diagnostic(format_args!(
"The `contravariant` parameter of \
a legacy `typing.TypeVar` cannot have \
an ambiguous value",
));
}
continue;
}
(_, Truthiness::Ambiguous) => {
if let Some(builder) = self.context.report_lint(
&INVALID_LEGACY_TYPE_VARIABLE,
call_expression,
) {
builder.into_diagnostic(format_args!(
"The `covariant` parameter of \
a legacy `typing.TypeVar` cannot have \
an ambiguous value",
));
}
continue;
}
(Truthiness::AlwaysTrue, Truthiness::AlwaysTrue) => {
if let Some(builder) = self.context.report_lint(
&INVALID_LEGACY_TYPE_VARIABLE,
call_expression,
) {
builder.into_diagnostic(format_args!(
"A legacy `typing.TypeVar` cannot be \
both covariant and contravariant",
));
}
continue;
}
(Truthiness::AlwaysTrue, Truthiness::AlwaysFalse) => {
TypeVarVariance::Contravariant
}
(Truthiness::AlwaysFalse, Truthiness::AlwaysTrue) => {
TypeVarVariance::Covariant
}
(Truthiness::AlwaysFalse, Truthiness::AlwaysFalse) => {
TypeVarVariance::Invariant
}
};
let name_param = name_param
.into_string_literal()
.map(|name| name.value(self.db()).as_ref());
@ -5062,6 +5123,7 @@ impl<'db> TypeInferenceBuilder<'db> {
target.id.clone(),
containing_assignment,
bound_or_constraint,
variance,
*default,
TypeVarKind::Legacy,
)),