Skip to content

Support instantiation of type[SomeTypeVar] -> SomeTypeVar #14791

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
sterliakov wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Support instantiation of type[SomeTypeVar] -> SomeTypeVar #14791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
52d349b
Support instantiation of type[SomeTypeVar] -> SomeTypeVar
sterliakov Feb 24, 2023
9a8ab64
Fix isinstance check: return type is 2-tuple
sterliakov Feb 26, 2023
2f8e972
Lint
sterliakov Feb 26, 2023
710a2fa
Add missing get_proper_type
sterliakov Feb 26, 2023
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
12 changes: 11 additions & 1 deletion mypy/checkexpr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,17 @@ def check_call(
)
elif isinstance(callee, TypeType):
item = self.analyze_type_type_callee(callee.item, context)
return self.check_call(item, args, arg_kinds, context, arg_names, callable_node)
# We always check the validity of the call (constructor args)
result_type, callee_type = self.check_call(
item, args, arg_kinds, context, arg_names, callable_node
)
if isinstance(callee.item, TypeVarType) and not isinstance(
get_proper_type(result_type), AnyType
):
# If it was `type[SomeTypeVar]`, successful instantiation produces SomeTypeVar
return callee.item, callee_type
else:
return result_type, callee_type

@JukkaL JukkaL Mar 6, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you investigated why the return type is inferred incorrectly in check_call? It may be better to fix this there instead. Right now this feels like we may have a bug in type inference and we are papering over it by patching the result afterwards. It's still possible that there is some fundamental reason why we can't do the right thing in check_call and this approach is the best option, but I'd prefer to better understand if that is the case.

@sterliakov sterliakov Mar 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I considered this, but it looks like this scenario is specific to one context. check_call is not responsible here, the issue arises when analyze_type_type_callee is called with TypeVar(bound=A|B) (tvar with union upper bound). This case is expanded to a Union of two callables corresponding to constructors of union items (line 1589). Currently analyze_type_type_callee gives

Union[def (x: builtins.str) -> a.A, def (x: builtins.int) -> a.B]

Alternative solution would be to handle UnionType there, but it looks less clear to me: we'll have then several TypeVar instances with same ids (so that they still correspond to original TypeVar), but different upper bounds, like the following:

Union[def (x: builtins.str) -> T`-1, def (x: builtins.int) -> T`-1]

where first T`-1 is bound to A and second - to B. This is at least confusing and, I guess, error prone.

The mentioned problem is related to specifically Type[...] instantiation, and it's easier to say that Type[SomeTVar] gives SomeTVar after successful instantiation. This is a "hack" mentioned in comment in analyze_type_type_callee: an upper bound is analyzed instead of typevar itself.

@sterliakov sterliakov Mar 18, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm no longer sure that the latter solution is worse.

@JukkaL Is there any precedent of creating TypeVar copies with same id and different bounds? Is there any fundamental reason not to do so? Should I open a separate issue with this question?

With such approach we'll have proper typevar narrowing for free in cases like

_T = TypeVar('_T', bound=int|str)

def foo(x: _T) -> _T:
    if isinstance(x, int): return x
    else: return x

(this currently fails in if due to narrowing that discards TypeVar part)

elif isinstance(callee, TupleType):
return self.check_call(
tuple_fallback(callee),
Expand Down
98 changes: 98 additions & 0 deletions test-data/unit/check-type-typevar.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
-- Test cases for type[SomeTypeVar] treatment


[case testTypeOfTypeVarInstantiation]
from typing import TypeVar, Type
T = TypeVar('T')
def f(x: Type[T]) -> T:
return x()

[case testTypeOfTypeVarBoundInstantiationInitParams]
from typing import TypeVar, Type
class A:
def __init__(self, x: str) -> None: pass
T = TypeVar('T', bound=A)
def f(x: Type[T]) -> T:
return x('a')
def f2(x: Type[T]) -> T:
return x(1) # E: Argument 1 to "A" has incompatible type "int"; expected "str"

[case testTypeOfTypeVarBoundInstantiationGeneric]
from typing import Generic, TypeVar, Type
class A:
def __init__(self, x: str) -> None: pass

T = TypeVar('T', bound=object)
V = TypeVar('V', bound=A)

class Test(Generic[T, V]):
X: Type[T]
Y: Type[V]
def f1(self) -> T:
return self.X()
def f2(self) -> V:
return self.Y() # E: Missing positional argument "x" in call to "A"
def f3(self) -> V:
return self.Y(1) # E: Argument 1 to "A" has incompatible type "int"; expected "str"
def f4(self) -> V:
return self.Y('foo')

[case testTypeOfTypeVarInstantiationInitParamsUnionBound]
from typing import TypeVar, Type, Union
class A:
def __init__(self, x: str) -> None: pass
class B:
def __init__(self, x: int) -> None: pass
class C:
def __init__(self, x: int) -> None: pass

T = TypeVar('T', bound=Union[A, B])
def f(x: Type[T]) -> T:
return x(1) # E: Argument 1 to "A" has incompatible type "int"; expected "str"

T2 = TypeVar('T2', bound=Union[B, C])
def f2(x: Type[T2]) -> T2:
return x(1)

[case testTypeOfTypeVarInstantiationVariants]
from typing import TypeVar, Type
class A:
def __init__(self, x: str) -> None: pass
class B:
def __init__(self, x: int) -> None: pass
class C:
def __init__(self, x: int) -> None: pass

T = TypeVar('T', A, B)
def f(x: Type[T]) -> T:
return x(1) # E: Argument 1 to "A" has incompatible type "int"; expected "str"

T2 = TypeVar('T2', B, C)
def f2(x: Type[T2]) -> T2:
return x(1)

def return_indirect(x: Type[T2]) -> T2:

@sobolevn sobolevn Feb 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need indirect tests? 🤔

@sterliakov sterliakov Feb 28, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because return some_immediate_expression_or_literal() and return some_var_with_already_fixed_type are not 100% equivalent (consider Literal or TypedDict return, like this), and I'd like to be sure here. It may have no special treatment now (and it doesn't), but we may have a case in future where similar stricter inference is necessary.

ret = x(1)
return ret

def return_indirect_condition(x: Type[T2], flag: bool) -> T2:
if flag:
ret = x(1)
else:
ret = x(2)
return ret

def return_indirect_condition_invalid(x: Type[T2], flag: bool) -> T2:
if flag:
ret = x(1)
else:
ret = x('foo') # E: Argument 1 to "B" has incompatible type "str"; expected "int" \
# E: Argument 1 to "C" has incompatible type "str"; expected "int"
return ret

def return_indirect_condition_invalid_different(x: Type[T], flag: bool) -> T:
if flag:
ret = x(1) # E: Argument 1 to "A" has incompatible type "int"; expected "str"
else:
ret = x('foo') # E: Argument 1 to "B" has incompatible type "str"; expected "int"
return ret