Skip to content

Commit 6bef521

Browse files
Adist319meta-codesync[bot]
authored andcommitted
Fix TypeVar resolution order in union type aliases (#2056)
Summary: Union type aliases like `Result = Ok[_T] | Error[_TE]` were collecting type parameters in the wrong order because union members get sorted alphabetically during simplification. This caused `Result[T, TE]` to incorrectly match `T` to `_TE` instead of `_T`. The fix sorts collected type parameters by their source location after traversal, restoring the user's intended order. Fixes #1759. Pull Request resolved: #2056 Test Plan: - Added `test_union_type_alias_typevar_order` - original issue reproduction - Added `test_union_type_alias_typevar_order_multiple` - 3 TypeVars with reversed alphabetical order - Added `test_union_type_alias_duplicate_typevar` - same TypeVar appearing multiple times Reviewed By: grievejia Differential Revision: D90423141 Pulled By: stroxler fbshipit-source-id: 07b5e9024f9373a1ffbf769d6bf14560314cc222
1 parent d7ffc4b commit 6bef521

2 files changed

Lines changed: 123 additions & 14 deletions

File tree

pyrefly/lib/alt/solve.rs

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
967967
seen_type_vars: &mut SmallMap<TypeVar, Quantified>,
968968
seen_type_var_tuples: &mut SmallMap<TypeVarTuple, Quantified>,
969969
seen_param_specs: &mut SmallMap<ParamSpec, Quantified>,
970-
tparams: &mut Vec<TParam>,
970+
tparams: &mut Vec<(TextRange, TParam)>,
971971
) {
972972
match ty {
973973
Type::Union(box Union { members: ts, .. }) => {
@@ -1049,10 +1049,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
10491049
ty_var.restriction().clone(),
10501050
);
10511051
e.insert(q.clone());
1052-
tparams.push(TParam {
1053-
quantified: q.clone(),
1054-
variance: ty_var.variance(),
1055-
});
1052+
tparams.push((
1053+
ty_var.qname().range(),
1054+
TParam {
1055+
quantified: q.clone(),
1056+
variance: ty_var.variance(),
1057+
},
1058+
));
10561059
q
10571060
}
10581061
};
@@ -1068,10 +1071,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
10681071
ty_var_tuple.default().cloned(),
10691072
);
10701073
e.insert(q.clone());
1071-
tparams.push(TParam {
1072-
quantified: q.clone(),
1073-
variance: PreInferenceVariance::Invariant,
1074-
});
1074+
tparams.push((
1075+
ty_var_tuple.qname().range(),
1076+
TParam {
1077+
quantified: q.clone(),
1078+
variance: PreInferenceVariance::Invariant,
1079+
},
1080+
));
10751081
q
10761082
}
10771083
};
@@ -1087,10 +1093,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
10871093
param_spec.default().cloned(),
10881094
);
10891095
e.insert(q.clone());
1090-
tparams.push(TParam {
1091-
quantified: q.clone(),
1092-
variance: PreInferenceVariance::Invariant,
1093-
});
1096+
tparams.push((
1097+
param_spec.qname().range(),
1098+
TParam {
1099+
quantified: q.clone(),
1100+
variance: PreInferenceVariance::Invariant,
1101+
},
1102+
));
10941103
q
10951104
}
10961105
};
@@ -1175,13 +1184,19 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
11751184
.filter_map(|key| self.get_idx(*key).deref().parameter().cloned())
11761185
.collect();
11771186
} else {
1187+
let mut tparams_with_ranges = Vec::new();
11781188
self.tvars_to_tparams_for_type_alias(
11791189
&mut ty,
11801190
&mut seen_type_vars,
11811191
&mut seen_type_var_tuples,
11821192
&mut seen_param_specs,
1183-
&mut tparams,
1193+
&mut tparams_with_ranges,
11841194
);
1195+
// Sort by source location to restore the user's intended type parameter order.
1196+
// This is needed because union members get sorted alphabetically during
1197+
// simplification, which can change the traversal order.
1198+
tparams_with_ranges.sort_by_key(|(range, _)| range.start());
1199+
tparams.extend(tparams_with_ranges.into_iter().map(|(_, tp)| tp));
11851200
}
11861201
if let Some(n) = tparams_for_type_alias_type {
11871202
for extra_tparam in tparams.iter().skip(n) {

pyrefly/lib/test/type_alias.rs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,3 +885,97 @@ type U = (a := 1) # E: Named expression cannot be used within a type alias # E:
885885
type V = int | (b := str) # E: Named expression cannot be used within a type alias
886886
"#,
887887
);
888+
889+
testcase!(
890+
test_union_type_alias_typevar_order,
891+
r#"
892+
import dataclasses as dc
893+
from typing import TypeVar, Iterable
894+
895+
@dc.dataclass
896+
class Ok[T]:
897+
value: T
898+
899+
@dc.dataclass
900+
class Error[T: Exception]:
901+
error: T
902+
903+
_T = TypeVar("_T")
904+
_TE = TypeVar("_TE", bound=Exception)
905+
Result = Ok[_T] | Error[_TE]
906+
907+
def func[T, TE: Exception](
908+
results: Iterable[Result[T, TE]],
909+
) -> tuple[Iterable[Ok[T]], Iterable[Error[TE]]]: ...
910+
911+
# Verify instantiation works correctly
912+
def test(r: Result[int, ValueError]) -> None:
913+
pass
914+
915+
test(Ok(42))
916+
test(Error(ValueError("error")))
917+
"#,
918+
);
919+
920+
testcase!(
921+
test_union_type_alias_typevar_order_multiple,
922+
r#"
923+
from typing import TypeVar, assert_type
924+
import dataclasses as dc
925+
926+
@dc.dataclass
927+
class Zebra[T]:
928+
value: T
929+
930+
@dc.dataclass
931+
class Bee[T]:
932+
value: T
933+
934+
@dc.dataclass
935+
class Aardvark[T]:
936+
value: T
937+
938+
_T1 = TypeVar("_T1")
939+
_T2 = TypeVar("_T2")
940+
_T3 = TypeVar("_T3")
941+
942+
# Source order: _T1, _T2, _T3 (from Zebra, Bee, Aardvark)
943+
# Alphabetical order would be: Aardvark[_T3], Bee[_T2], Zebra[_T1] -> _T3, _T2, _T1
944+
Combined = Zebra[_T1] | Bee[_T2] | Aardvark[_T3]
945+
946+
# This should work: int->_T1, str->_T2, float->_T3
947+
x: Combined[int, str, float] = Zebra(42)
948+
assert_type(x, Zebra[int] | Bee[str] | Aardvark[float])
949+
950+
y: Combined[int, str, float] = Bee("hello")
951+
z: Combined[int, str, float] = Aardvark(3.14)
952+
"#,
953+
);
954+
955+
// Test that duplicate TypeVars are handled correctly (only first occurrence counts)
956+
testcase!(
957+
test_union_type_alias_duplicate_typevar,
958+
r#"
959+
from typing import TypeVar, assert_type
960+
import dataclasses as dc
961+
962+
_T = TypeVar("_T")
963+
964+
@dc.dataclass
965+
class First[T]:
966+
value: T
967+
968+
@dc.dataclass
969+
class Second[T]:
970+
value: T
971+
972+
# _T appears in both, but should only be one type parameter
973+
Alias = First[_T] | Second[_T]
974+
975+
x: Alias[int] = First(42)
976+
assert_type(x, First[int] | Second[int])
977+
978+
y: Alias[str] = Second("hello")
979+
assert_type(y, First[str] | Second[str])
980+
"#,
981+
);

0 commit comments

Comments
 (0)