[ty] Fix mixed tuple subtyping (#18852)
Some checks failed
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Has been cancelled

## Summary

The code in the `Variable` branch of
`VariableLengthTupleSpec::has_relation_to` made the incorrect assumption
that if you zip two possibly-different-length iterators together and
iterate over the resulting zip iterator, the original two iterators will
only have their common elements consumed. But in fact, the zip iterator
detects that it is done when it receives a `None` from one iterator and
`Some()` element from the other iterator, which means that it consumes
one additional element from the longer iterator. This meant that we
failed to detect mismatched types on this extra consumed element,
because we never compared it to the variable type of the other tuple.

Use `zip_longest` from itertools as an alternative, which allows us to
combine all the handling into just two `zip_longest`, one for prefixes
and one for suffixes.

Marking this PR internal since it fixes a bug in a commit that wasn't
released yet.

## Test Plan

Added mdtests that failed before this fix and pass after it.
This commit is contained in:
Carl Meyer 2025-06-21 13:09:23 -07:00 committed by GitHub
parent f24e650dfd
commit 089f5152f6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 48 additions and 37 deletions

View file

@ -245,6 +245,18 @@ static_assert(
)
)
static_assert(
not is_subtype_of(
tuple[Literal["foo"], *tuple[int, ...]],
tuple[int, ...],
)
)
static_assert(
not is_subtype_of(
tuple[*tuple[int, ...], Literal["foo"]],
tuple[int, ...],
)
)
static_assert(
not is_subtype_of(
tuple[Literal[1], Literal[2], *tuple[int, ...], Literal[10]],

View file

@ -16,7 +16,7 @@
//! that adds that "collapse `Never`" behavior, whereas [`TupleSpec`] allows you to add any element
//! types, including `Never`.)
use itertools::Either;
use itertools::{Either, EitherOrBoth, Itertools};
use crate::types::class::{ClassType, KnownClass};
use crate::types::{Type, TypeMapping, TypeRelation, TypeVarInstance, TypeVarVariance, UnionType};
@ -544,45 +544,44 @@ impl<'db> VariableLengthTupleSpec<'db> {
TupleSpec::Variable(other) => {
// The overlapping parts of the prefixes and suffixes must satisfy the relation.
let mut self_prefix = self.prefix.iter();
let mut other_prefix = other.prefix.iter();
let prefixes_match = (&mut self_prefix)
.zip(&mut other_prefix)
.all(|(self_ty, other_ty)| self_ty.has_relation_to(db, *other_ty, relation));
if !prefixes_match {
// Any remaining parts must satisfy the relation with the other tuple's
// variable-length part.
if !self
.prefix
.iter()
.zip_longest(&other.prefix)
.all(|pair| match pair {
EitherOrBoth::Both(self_ty, other_ty) => {
self_ty.has_relation_to(db, *other_ty, relation)
}
EitherOrBoth::Left(self_ty) => {
self_ty.has_relation_to(db, other.variable, relation)
}
EitherOrBoth::Right(other_ty) => {
self.variable.has_relation_to(db, *other_ty, relation)
}
})
{
return false;
}
let mut self_suffix = self.suffix.iter().rev();
let mut other_suffix = other.suffix.iter().rev();
let suffixes_match = (&mut self_suffix)
.zip(&mut other_suffix)
.all(|(self_ty, other_ty)| self_ty.has_relation_to(db, *other_ty, relation));
if !suffixes_match {
return false;
}
// Any remaining parts of either prefix or suffix must satisfy the relation with
// the other tuple's variable-length portion.
let prefix_matches_variable = self_prefix
.all(|self_ty| self_ty.has_relation_to(db, other.variable, relation));
if !prefix_matches_variable {
return false;
}
let prefix_matches_variable = other_prefix
.all(|other_ty| self.variable.has_relation_to(db, *other_ty, relation));
if !prefix_matches_variable {
return false;
}
let suffix_matches_variable = self_suffix
.all(|self_ty| self_ty.has_relation_to(db, other.variable, relation));
if !suffix_matches_variable {
return false;
}
let suffix_matches_variable = other_suffix
.all(|other_ty| self.variable.has_relation_to(db, *other_ty, relation));
if !suffix_matches_variable {
if !self
.suffix
.iter()
.rev()
.zip_longest(other.suffix.iter().rev())
.all(|pair| match pair {
EitherOrBoth::Both(self_ty, other_ty) => {
self_ty.has_relation_to(db, *other_ty, relation)
}
EitherOrBoth::Left(self_ty) => {
self_ty.has_relation_to(db, other.variable, relation)
}
EitherOrBoth::Right(other_ty) => {
self.variable.has_relation_to(db, *other_ty, relation)
}
})
{
return false;
}