Skip to content

Commit 0aebd10

Browse files
fangyi-zhoumeta-codesync[bot]
authored andcommitted
Handle type[T] calls where T is bounded by a class (#2381)
Summary: When types of `type[T]` are called, we currently infer a callable that takes any arguments and returns the type variable T. This is a conformance gap, where we should treat such calls as on `T`. However, the spec made clear that this is not free of problems: From https://typing.python.org/en/latest/spec/constructors.html#constructor-calls-for-type-t > It should be noted that such code could be unsafe because the type `type[T]` may represent subclasses of `T`, and those subclasses could redefine the `__new__` and `__init__` methods in a way that is incompatible with the base class. Likewise, the metaclass of `T` could redefine the `__call__` method in a way that is incompatible with the base metaclass. If `T` is unrestricted, we assume an implicit bound of `object`, and use the callable type `() -> T`. If `T` is bounded, we use the bound class type to infer the callable signature, but use the type `T` as the return type. We cannot handle constrained type variables, since we don't have any intersection type support at the call target level, so I'm leaving the current behaviour as is. Let's assess the impact on open source codebases, since I expect this change will bring some false positives. #2369 Pull Request resolved: #2381 Test Plan: Ran conformance test suite and fixed 4 failing tests. Reviewed By: stroxler Differential Revision: D92900019 Pulled By: migeed-z fbshipit-source-id: a50b259d559a938e75801f0fd44aff33e83051d7
1 parent 123e68b commit 0aebd10

File tree

6 files changed

+148
-31
lines changed

6 files changed

+148
-31
lines changed

conformance/third_party/conformance.exp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3308,6 +3308,61 @@
33083308
"severity": "error",
33093309
"stop_column": 10,
33103310
"stop_line": 59
3311+
},
3312+
{
3313+
"code": -2,
3314+
"column": 9,
3315+
"concise_description": "Expected 0 positional arguments, got 1",
3316+
"description": "Expected 0 positional arguments, got 1",
3317+
"line": 64,
3318+
"name": "bad-argument-count",
3319+
"severity": "error",
3320+
"stop_column": 10,
3321+
"stop_line": 64
3322+
},
3323+
{
3324+
"code": -2,
3325+
"column": 8,
3326+
"concise_description": "Missing argument `x` in function `Meta1.__call__`",
3327+
"description": "Missing argument `x` in function `Meta1.__call__`",
3328+
"line": 72,
3329+
"name": "missing-argument",
3330+
"severity": "error",
3331+
"stop_column": 10,
3332+
"stop_line": 72
3333+
},
3334+
{
3335+
"code": -2,
3336+
"column": 8,
3337+
"concise_description": "Missing argument `y` in function `Meta1.__call__`",
3338+
"description": "Missing argument `y` in function `Meta1.__call__`",
3339+
"line": 72,
3340+
"name": "missing-argument",
3341+
"severity": "error",
3342+
"stop_column": 10,
3343+
"stop_line": 72
3344+
},
3345+
{
3346+
"code": -2,
3347+
"column": 8,
3348+
"concise_description": "Missing argument `y` in function `Class2.__new__`",
3349+
"description": "Missing argument `y` in function `Class2.__new__`",
3350+
"line": 81,
3351+
"name": "missing-argument",
3352+
"severity": "error",
3353+
"stop_column": 11,
3354+
"stop_line": 81
3355+
},
3356+
{
3357+
"code": -2,
3358+
"column": 12,
3359+
"concise_description": "Argument `Literal[2]` is not assignable to parameter `y` with type `str` in function `Class2.__new__`",
3360+
"description": "Argument `Literal[2]` is not assignable to parameter `y` with type `str` in function `Class2.__new__`",
3361+
"line": 82,
3362+
"name": "bad-argument-type",
3363+
"severity": "error",
3364+
"stop_column": 13,
3365+
"stop_line": 82
33113366
}
33123367
],
33133368
"constructors_callable.py": [

conformance/third_party/conformance.result

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,7 @@
3636
"constructors_call_init.py": [],
3737
"constructors_call_metaclass.py": [],
3838
"constructors_call_new.py": [],
39-
"constructors_call_type.py": [
40-
"Line 64: Expected 1 errors",
41-
"Line 72: Expected 1 errors",
42-
"Line 81: Expected 1 errors",
43-
"Line 82: Expected 1 errors"
44-
],
39+
"constructors_call_type.py": [],
4540
"constructors_callable.py": [
4641
"Line 65: Expected 1 errors",
4742
"Line 66: Expected 1 errors",

conformance/third_party/results.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
{
22
"total": 138,
3-
"pass": 113,
4-
"fail": 25,
5-
"pass_rate": 0.82,
6-
"differences": 93,
3+
"pass": 114,
4+
"fail": 24,
5+
"pass_rate": 0.83,
6+
"differences": 89,
77
"passing": [
88
"aliases_explicit.py",
99
"aliases_newtype.py",
@@ -22,6 +22,7 @@
2222
"constructors_call_init.py",
2323
"constructors_call_metaclass.py",
2424
"constructors_call_new.py",
25+
"constructors_call_type.py",
2526
"constructors_consistency.py",
2627
"dataclasses_final.py",
2728
"dataclasses_frozen.py",
@@ -124,7 +125,6 @@
124125
"aliases_typealiastype.py": 2,
125126
"annotations_forward_refs.py": 2,
126127
"callables_annotation.py": 2,
127-
"constructors_call_type.py": 4,
128128
"constructors_callable.py": 8,
129129
"dataclasses_descriptors.py": 6,
130130
"dataclasses_slots.py": 2,

pyrefly/lib/alt/call.rs

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ pub enum CallTarget {
8484
/// Method of a class. The `Type` is the self/cls argument.
8585
BoundMethod(Type, TargetWithTParams<Function>),
8686
/// A class object.
87-
Class(ClassType, ConstructorKind),
87+
/// The optional Quantified argument is for the case where this call target
88+
/// occurs in a bounded type var, where the current class is being used as
89+
/// the upper bound.
90+
Class(ClassType, ConstructorKind, Option<Quantified>),
8891
/// A TypedDict.
8992
TypedDict(TypedDictInner),
9093
/// An overloaded function.
@@ -220,6 +223,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
220223
Type::ClassType(cls) => CallTargetLookup::Ok(Box::new(CallTarget::Class(
221224
cls,
222225
ConstructorKind::BareClassName,
226+
None,
223227
))),
224228
Type::TypedDict(TypedDict::TypedDict(typed_dict)) => {
225229
CallTargetLookup::Ok(Box::new(CallTarget::TypedDict(typed_dict)))
@@ -230,20 +234,53 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
230234
CallTargetLookup::Ok(Box::new(CallTarget::Class(
231235
cls,
232236
ConstructorKind::TypeOfClass,
237+
None,
233238
)))
234239
}
235-
Type::Type(box Type::Tuple(tuple)) => CallTargetLookup::Ok(Box::new(
236-
CallTarget::Class(self.erase_tuple_type(tuple), ConstructorKind::TypeOfClass),
237-
)),
238-
Type::Type(box Type::Quantified(quantified)) => {
239-
CallTargetLookup::Ok(Box::new(CallTarget::Callable(TargetWithTParams(
240+
Type::Type(box Type::Tuple(tuple)) => {
241+
CallTargetLookup::Ok(Box::new(CallTarget::Class(
242+
self.erase_tuple_type(tuple),
243+
ConstructorKind::TypeOfClass,
240244
None,
241-
Callable {
242-
// TODO: use upper bound to determine input parameters
243-
params: Params::Ellipsis,
244-
ret: Type::Quantified(quantified),
245-
},
246-
))))
245+
)))
246+
}
247+
Type::Type(box Type::Quantified(quantified)) => {
248+
let call_target = match quantified.restriction() {
249+
Restriction::Unrestricted => {
250+
// Assume this is object.__init__, reject any argument
251+
CallTarget::Callable(TargetWithTParams(
252+
None,
253+
Callable {
254+
params: Params::List(ParamList::new(vec![])),
255+
ret: Type::Quantified(quantified),
256+
},
257+
))
258+
}
259+
Restriction::Bound(Type::ClassType(cls)) => {
260+
// Use the bound to determine call target, but keep
261+
// the original quantified for the return type to allow
262+
// type variables in the return type to be resolved.
263+
CallTarget::Class(
264+
cls.clone(),
265+
ConstructorKind::TypeOfClass,
266+
Some(*quantified),
267+
)
268+
}
269+
// For unhandled cases, we accept any arguments and return
270+
// the quantified type itself.
271+
// We can't handle constraints because we need to take
272+
// intersection of constructor types of all constraints,
273+
// which is currently not possible.
274+
_ => CallTarget::Callable(TargetWithTParams(
275+
None,
276+
Callable {
277+
// TODO: use upper bound to determine input parameters
278+
params: Params::Ellipsis,
279+
ret: Type::Quantified(quantified),
280+
},
281+
)),
282+
};
283+
CallTargetLookup::Ok(Box::new(call_target))
247284
}
248285
Type::Type(inner) if let Type::Any(style) = *inner => {
249286
CallTargetLookup::Ok(Box::new(CallTarget::Any(style)))
@@ -332,6 +369,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
332369
CallTargetLookup::Ok(Box::new(CallTarget::Class(
333370
cls,
334371
ConstructorKind::TypeOfClass,
372+
None,
335373
)))
336374
}
337375
Type::Type(box Type::Intersect(box (_, fallback))) => {
@@ -852,7 +890,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
852890
}
853891
};
854892
let res = match call_target {
855-
CallTarget::Class(cls, constructor_kind) => {
893+
CallTarget::Class(cls, constructor_kind, as_quantified_bound) => {
856894
if cls.has_qname("typing", "Any") {
857895
return self.error(
858896
errors,
@@ -903,7 +941,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
903941
}
904942
}
905943
};
906-
self.construct_class(
944+
let constructed_type = self.construct_class(
907945
cls,
908946
args,
909947
keywords,
@@ -912,7 +950,16 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
912950
errors,
913951
context,
914952
hint,
915-
)
953+
);
954+
// Override the constructed type with the quantified bound if
955+
// this class is being called via a quantified type with a class
956+
// bound, to allow calls on TypeVars with class bounds to work
957+
// as expected.
958+
if let Some(quantified) = as_quantified_bound {
959+
Type::Quantified(Box::new(quantified))
960+
} else {
961+
constructed_type
962+
}
916963
}
917964
CallTarget::TypedDict(td) => self.construct_typed_dict(
918965
td,

pyrefly/lib/report/pysa/call_graph.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2288,7 +2288,11 @@ impl<'a> CallGraphVisitor<'a> {
22882288
})
22892289
.unwrap()
22902290
}
2291-
Some(CallTargetLookup::Ok(box crate::alt::call::CallTarget::Class(class_type, _))) => {
2291+
Some(CallTargetLookup::Ok(box crate::alt::call::CallTarget::Class(
2292+
class_type,
2293+
_,
2294+
_,
2295+
))) => {
22922296
// Constructing a class instance.
22932297
let (init_method, new_method) = self
22942298
.module_context

pyrefly/lib/test/generic_basic.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,34 @@ use crate::test::util::TestEnv;
1111
use crate::testcase;
1212

1313
testcase!(
14-
bug = "conformance: Should use bounds/constraints of type var to determine callable input type for type[T] constructors",
1514
test_tyvar_constructor,
1615
r#"
1716
def test[T](cls: type[T]) -> T:
18-
cls(1) # should error: no args for object constructor
17+
cls(1) # E: Expected 0 positional arguments, got 1
1918
return cls()
2019
class A:
2120
def __init__(self, x: int) -> None: pass
2221
def test2[T: A](cls: type[T]) -> T:
23-
a1: A = cls() # should error: missing required arg x
22+
a1: A = cls() # E: Missing argument `x` in function `A.__init__`
2423
a2: A = cls(1)
25-
return cls()
24+
return cls(1)
25+
"#,
26+
);
27+
28+
testcase!(
29+
bug = "When determining callable for type[T] where T is a constrained TypeVar, we should take intersection of constructor of all constraints",
30+
test_constrained_typevar_constructor,
31+
r#"
32+
from typing import TypeVar
33+
class A:
34+
def __init__(self, x: int) -> None: pass
35+
class B:
36+
def __init__(self, x: int, y: str = "default") -> None: pass
37+
T = TypeVar("T", A, B)
38+
def test(cls: type[T]) -> None:
39+
cls(1)
40+
cls("hello") # should error: incorrect type of x
41+
cls(1, "hello") # should error: too many arguments
2642
"#,
2743
);
2844

0 commit comments

Comments
 (0)