Skip to content

Merge multiple possible legacy type parameters in same scope #4134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
defr0std wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Merge multiple possible legacy type parameters in same scope #4134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbd21a3
Merge multiple possible legacy type parameters in same scope
Jul 13, 2026
3f8e003
Add more test coverage, add asserts instead of silently degrading
Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions pyrefly/lib/alt/jaxtyping.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
Binding::Import(import) => {
import.module.as_str() == "jaxtyping" && import.name.as_str() == "jaxtyping"
}
Binding::PossibleLegacyTParam(key, _) => self.legacy_tparam_is_jaxtyping_module(*key),
Binding::PossibleLegacyTParam(keys, _) => keys
.iter()
.any(|key| self.legacy_tparam_is_jaxtyping_module(*key)),
_ => false,
}
}
Expand All @@ -204,9 +206,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
| Binding::ForwardToFirstUse(idx) => {
self.binding_is_jaxtyping_wrapper_origin(self.binding_following_forwards(*idx))
}
Binding::PossibleLegacyTParam(key, _) => {
self.legacy_tparam_is_jaxtyping_wrapper_origin(*key)
}
Binding::PossibleLegacyTParam(keys, _) => keys
.iter()
.any(|key| self.legacy_tparam_is_jaxtyping_wrapper_origin(*key)),
_ => false,
}
}
Expand Down
88 changes: 54 additions & 34 deletions pyrefly/lib/alt/solve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4909,44 +4909,64 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
#[inline(never)]
fn binding_to_type_info_possible_legacy_tparam(
&self,
key: Idx<KeyLegacyTypeParam>,
keys: &[Idx<KeyLegacyTypeParam>],
range_if_scoped_params_exist: &Option<TextRange>,
errors: &ErrorCollector,
) -> TypeInfo {
let ty = match &*self.get_idx(key) {
LegacyTypeParameterLookup::Parameter(p) => {
// This class or function has scoped (PEP 695) type parameters. Mixing legacy-style parameters is an error.
if let Some(r) = range_if_scoped_params_exist {
self.error(
errors,
*r,
ErrorKind::InvalidTypeVar,
format!(
"Type parameter {} is not included in the type parameter list",
self.module().display(&self.bindings().idx_to_key(key).0)
),
);
}
p.clone().to_value()
}
LegacyTypeParameterLookup::NotParameter(ty) => ty.clone(),
};
match self.bindings().get(key) {
BindingLegacyTypeParam::ModuleKeyed(idx, attrs) => {
// `idx` points to a module whose attr chain may end in a legacy type
// variable that needs to be replaced with a QuantifiedValue. Since the
// binding is for the module itself, we use the mechanism for attribute
// ("facet") type narrowing to change the type produced when the final
// attr is accessed.
let module = (*self.get_idx(*idx)).clone();
if matches!(ty, Type::QuantifiedValue(_)) {
let facets = attrs.mapped_ref(|a| FacetKind::Attribute(a.clone()));
module.with_narrow(&facets, ty)
} else {
module
let first_key = *keys
.first()
.expect("PossibleLegacyTParam bindings must always have at least one key");
match self.bindings().get(first_key) {
BindingLegacyTypeParam::ParamKeyed(_) => {
let ty = match &*self.get_idx(first_key) {
LegacyTypeParameterLookup::Parameter(p) => {
if let Some(r) = range_if_scoped_params_exist {
self.error(
errors,
*r,
ErrorKind::InvalidTypeVar,
format!(
"Type parameter {} is not included in the type parameter list",
self.module()
.display(&self.bindings().idx_to_key(first_key).0)
),
);
}
p.clone().to_value()
}
LegacyTypeParameterLookup::NotParameter(ty) => ty.clone(),
};
TypeInfo::of_ty(ty)
}
BindingLegacyTypeParam::ModuleKeyed(idx, _) => {
let mut module_info = (*self.get_idx(*idx)).clone();
for &key in keys {
let ty = match &*self.get_idx(key) {
LegacyTypeParameterLookup::Parameter(p) => {
if let Some(r) = range_if_scoped_params_exist {
self.error(
errors,
*r,
ErrorKind::InvalidTypeVar,
format!(
"Type parameter {} is not included in the type parameter list",
self.module().display(&self.bindings().idx_to_key(key).0)
),
);
}
p.clone().to_value()
}
LegacyTypeParameterLookup::NotParameter(ty) => ty.clone(),
};
if let BindingLegacyTypeParam::ModuleKeyed(_, attrs) = self.bindings().get(key)
&& matches!(ty, Type::QuantifiedValue(_))
{
let facets = attrs.mapped_ref(|a| FacetKind::Attribute(a.clone()));
module_info = module_info.with_narrow(&facets, ty);
}
}
module_info
}
BindingLegacyTypeParam::ParamKeyed(_) => TypeInfo::of_ty(ty),
}
}

Expand Down Expand Up @@ -5015,7 +5035,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
}
Binding::PossibleLegacyTParam(key, range_if_scoped_params_exist) => self
.binding_to_type_info_possible_legacy_tparam(
*key,
key,
range_if_scoped_params_exist,
errors,
),
Expand Down
27 changes: 20 additions & 7 deletions pyrefly/lib/binding/binding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use ruff_python_ast::name::Name;
use ruff_text_size::Ranged;
use ruff_text_size::TextRange;
use starlark_map::small_set::SmallSet;
use thin_vec::ThinVec;
use vec1::Vec1;

use crate::alt::class::class_field::ClassField;
Expand Down Expand Up @@ -926,7 +927,7 @@ pub enum Key {
///
/// The resulting type may not actually involve a legacy type param, since it may turn out I am
/// some other kind of type.
PossibleLegacyTParam(TextRange),
PossibleLegacyTParam(Box<(TextRange, TextRange)>),
/// A `del` statement. It is a `Binding` associated with a the type `Any` because `del` defines a name in scope,
/// so we need to provide a `Key` for any reads of that name in the edge case where there is no other definition
///
Expand Down Expand Up @@ -965,7 +966,7 @@ impl Ranged for Key {
Self::UsageLink(r) => *r,
Self::YieldLink(r) => *r,
Self::Delete(r) => *r,
Self::PossibleLegacyTParam(r) => *r,
Self::PossibleLegacyTParam(x) => x.0,
Self::PatternNarrow(r) => *r,
Self::Exhaustive(_, r) => *r,
}
Expand Down Expand Up @@ -1005,8 +1006,13 @@ impl DisplayWith<ModuleInfo> for Key {
Self::UsageLink(r) => write!(f, "Key::UsageLink({})", ctx.display(r)),
Self::YieldLink(r) => write!(f, "Key::YieldLink({})", ctx.display(r)),
Self::Delete(r) => write!(f, "Key::Delete({})", ctx.display(r)),
Self::PossibleLegacyTParam(r) => {
write!(f, "Key::PossibleLegacyTParam({})", ctx.display(r))
Self::PossibleLegacyTParam(x) => {
write!(
f,
"Key::PossibleLegacyTParam({}, scope={})",
ctx.display(&x.0),
ctx.display(&x.1)
)
}
Self::PatternNarrow(r) => write!(f, "Key::PatternNarrow({})", ctx.display(r)),
Self::Exhaustive(kind, r) => {
Expand Down Expand Up @@ -2337,7 +2343,7 @@ pub enum Binding {
/// The TextRange is optional and controls whether to produce an error
/// saying there are scoped type parameters for this function / class, and
/// therefore the use of legacy type parameters is invalid.
PossibleLegacyTParam(Idx<KeyLegacyTypeParam>, Option<TextRange>),
PossibleLegacyTParam(ThinVec<Idx<KeyLegacyTypeParam>>, Option<TextRange>),
/// An assignment to a name.
NameAssign(Box<NameAssign>),
/// A type alias (legacy, scoped, or `TypeAliasType` call).
Expand Down Expand Up @@ -2525,8 +2531,15 @@ impl DisplayWith<Bindings> for Binding {
Self::TypeParameter(tp) => {
write!(f, "TypeParameter({}, {}, ..)", tp.identity, tp.kind)
}
Self::PossibleLegacyTParam(k, _) => {
write!(f, "PossibleLegacyTParam({})", ctx.display(*k))
Self::PossibleLegacyTParam(keys, _) => {
write!(f, "PossibleLegacyTParam([")?;
for (i, &k) in keys.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "{}", ctx.display(k))?;
}
write!(f, "])")
}
Self::AnnotatedType(k1, k2) => {
write!(
Expand Down
112 changes: 80 additions & 32 deletions pyrefly/lib/binding/bindings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1799,10 +1799,11 @@ impl<'a> BindingsBuilder<'a> {
// intercept_lookup which wraps them in PossibleLegacyTParam.
// By finalize time we can follow through to the original
// binding to check whether it's actually a type alias.
Binding::PossibleLegacyTParam(tparam_idx, _)
if let Some(legacy_binding) = self.idx_to_binding(*tparam_idx) =>
{
self.follow_to_type_alias(legacy_binding.idx())
Binding::PossibleLegacyTParam(tparam_indices, _) => {
tparam_indices.iter().find_map(|tparam_idx| {
let legacy_binding = self.idx_to_binding(*tparam_idx)?;
self.follow_to_type_alias(legacy_binding.idx())
})
}
_ => None,
}
Expand Down Expand Up @@ -2166,8 +2167,8 @@ impl LegacyTParamId {
/// Note that the range here is not the range of the full `LegacyTParamId`, but
/// just of the name being bound (which in the `Attr` case is just the base
/// rather than the entire identifier).
fn as_possible_legacy_tparam_key(&self) -> Key {
Key::PossibleLegacyTParam(self.as_identifier().range)
fn as_possible_legacy_tparam_key(&self, scope_range: TextRange) -> Key {
Key::PossibleLegacyTParam(Box::new((self.as_identifier().range, scope_range)))
}

/// Get the key used to track this potential legacy tparam in the `legacy_tparams` map.
Expand Down Expand Up @@ -2363,18 +2364,21 @@ impl<'a> BindingsBuilder<'a> {
// Short circuit if there are too many forwards - it may mean there's a cycle.
let mut original_binding = self.idx_to_binding(original_idx);
let mut gas = Gas::new(100);
while let Some(
Binding::Forward(fwd_idx)
| Binding::PromoteForward(fwd_idx)
| Binding::ForwardToFirstUse(fwd_idx)
| Binding::Phi(JoinStyle::NarrowOf(fwd_idx), _),
) = original_binding
{
loop {
if gas.stop() {
return None;
} else {
original_idx = *fwd_idx;
original_binding = self.idx_to_binding(original_idx);
}
match original_binding {
Some(
Binding::Forward(fwd_idx)
| Binding::PromoteForward(fwd_idx)
| Binding::ForwardToFirstUse(fwd_idx)
| Binding::Phi(JoinStyle::NarrowOf(fwd_idx), _),
) => {
original_idx = *fwd_idx;
original_binding = self.idx_to_binding(original_idx);
}
_ => break,
}
}
Some((original_idx, original_binding))
Expand All @@ -2392,22 +2396,62 @@ impl<'a> BindingsBuilder<'a> {
has_scoped_type_params: bool,
) -> Option<PossibleTParam> {
let (original_idx, original_binding) = self.get_original_binding(original_idx)?;
// If we found a potential legacy type variable, first insert the key / binding pair
// for the raw lookup, then insert another key / binding pair for the
// `CheckLegacyTypeParam`, and return the `Idx<Key>`.
let tparam_idx = Self::make_legacy_tparam(&id, original_binding, original_idx)
.map(|(k, v)| self.insert_binding(k, v))?;
let idx = self.insert_binding(
id.as_possible_legacy_tparam_key(),
Binding::PossibleLegacyTParam(
tparam_idx,
if has_scoped_type_params {
Some(id.range())
} else {
None
},
),
);

let scope_range = self.scopes.current_scope_range();
let key = match &id {
LegacyTParamId::Name(_) => id.as_possible_legacy_tparam_key(scope_range),
LegacyTParamId::Attr(..) => {
let original_range = self.table.get::<Key>().0.idx_to_key(original_idx).range();
Key::PossibleLegacyTParam(Box::new((original_range, scope_range)))
}
};
let existing_idx = self.table.get::<Key>().0.key_to_idx(&key);
let idx = if let Some(existing_idx) = existing_idx {
let entry = self.table.get_mut::<Key>();
if let Some(binding) = entry.1.get_mut(existing_idx) {
match binding {
Binding::PossibleLegacyTParam(tparams, _) => {
if !tparams.contains(&tparam_idx) {
tparams.push(tparam_idx);
}
}
_ => unreachable!("Expected Binding::PossibleLegacyTParam"),
}
existing_idx
} else {
let mut tparams = ThinVec::new();
tparams.push(tparam_idx);
entry.1.insert(
existing_idx,
Binding::PossibleLegacyTParam(
tparams,
if has_scoped_type_params {
Some(id.range())
} else {
None
},
),
);
existing_idx
}
} else {
let mut tparams = ThinVec::new();
tparams.push(tparam_idx);
self.insert_binding(
key,
Binding::PossibleLegacyTParam(
tparams,
if has_scoped_type_params {
Some(id.range())
} else {
None
},
),
)
};

Some(PossibleTParam {
id,
idx,
Expand Down Expand Up @@ -2472,8 +2516,12 @@ impl<'a> BindingsBuilder<'a> {
for entry in legacy_tparams.legacy_tparams.values() {
match entry {
TParamLookupResult::MaybeTParam(possible_tparam) => {
self.scopes
.add_possible_legacy_tparam(possible_tparam.id.as_identifier());
let key = self.table.get::<Key>().0.idx_to_key(possible_tparam.idx);
let range = key.range();
self.scopes.add_possible_legacy_tparam(
possible_tparam.id.as_identifier().id.clone(),
range,
);
}
_ => {}
}
Expand Down
Loading
Loading