Skip to content

Commit

Permalink
Add unbound typevar check (#13166)
Browse files Browse the repository at this point in the history
### Description

This should fix Issue #13061 .

Mypy should now raise an error when it encounters unbound plain TypeVar.
It should not get triggered when the return type contains a TypeVar (e.g. List[T]).
It should not get triggered when a same TypeVar is present in the scope (e.g. inner function returns T, and T is an argument of outer function).

## Test Plan

5 tests were added:

- a plain unbound typevar triggering the new error
- a typevar inner function not triggering the error
- a function returning an iterable of typevars, not triggering the error
- a plain bound typevar, not triggering the error
- nested bound typevars, not triggering the error

We also changed the other functions triggering our error for the tests to pass.

This is our 1st contribution to Mypy. Please, guide us if there is anything we can improve in this PR.

This PR was made with @anilbey 


* Add simple unbound typevar check and rudimentary test

* Add test for unbound func returning iterables of TypeVars

* Add appropriate error message and make the new test pass

* add CollectArgTypes to get set of argument types

* lint fix

* fix type error

* extract check_unbound_return_typevar as method

* check if return type is instantiated in typ.variables

* Fix some tests that are affected by new implementation

* add testInnerFunctionTypeVar test

* fix testErrorCodeNeedTypeAnnotation test

* add TYPE_VAR error code to unbound error failure

* Fix the tests

* move new tests in new test file

* add 'check-typevar-unbound.test' to testcheck

* add more nested tests for unbound type

* microoptimise check_unbound_return_typevar check ret_type first

* add missing builtins fixture to failing test

Co-authored-by: Jaquier Aurélien Tristan <ajaquier@bb-fvfcv295mnhx.epfl.ch>
Co-authored-by: Anil Tuncel <anil.tuncel@epfl.ch>
Co-authored-by: Anil Tuncel <tuncel.manil@gmail.com>
Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
  • Loading branch information
5 people authored Jul 19, 2022
1 parent 98f24f4 commit 2c152e6
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 13 deletions.
22 changes: 22 additions & 0 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from typing_extensions import Final, TypeAlias as _TypeAlias

from mypy.backports import nullcontext
from mypy.errorcodes import TYPE_VAR
from mypy.errors import Errors, report_internal_error, ErrorWatcher
from mypy.nodes import (
SymbolTable, Statement, MypyFile, Var, Expression, Lvalue, Node,
Expand Down Expand Up @@ -40,6 +41,7 @@
get_proper_types, is_literal_type, TypeAliasType, TypeGuardedType, ParamSpecType,
OVERLOAD_NAMES, UnboundType
)
from mypy.typetraverser import TypeTraverserVisitor
from mypy.sametypes import is_same_type
from mypy.messages import (
MessageBuilder, make_inferred_type_note, append_invariance_notes, pretty_seq,
Expand Down Expand Up @@ -918,6 +920,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])
if typ.ret_type.variance == CONTRAVARIANT:
self.fail(message_registry.RETURN_TYPE_CANNOT_BE_CONTRAVARIANT,
typ.ret_type)
self.check_unbound_return_typevar(typ)

# Check that Generator functions have the appropriate return type.
if defn.is_generator:
Expand Down Expand Up @@ -1062,6 +1065,16 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])

self.binder = old_binder

def check_unbound_return_typevar(self, typ: CallableType) -> None:
"""Fails when the return typevar is not defined in arguments."""
if (typ.ret_type in typ.variables):
arg_type_visitor = CollectArgTypes()
for argtype in typ.arg_types:
argtype.accept(arg_type_visitor)

if typ.ret_type not in arg_type_visitor.arg_types:
self.fail(message_registry.UNBOUND_TYPEVAR, typ.ret_type, code=TYPE_VAR)

def check_default_args(self, item: FuncItem, body_is_trivial: bool) -> None:
for arg in item.arguments:
if arg.initializer is None:
Expand Down Expand Up @@ -5862,6 +5875,15 @@ class Foo(Enum):
and member_type.fallback.type == parent_type.type_object())


class CollectArgTypes(TypeTraverserVisitor):
"""Collects the non-nested argument types in a set."""
def __init__(self) -> None:
self.arg_types: Set[TypeVarType] = set()

def visit_type_var(self, t: TypeVarType) -> None:
self.arg_types.add(t)


@overload
def conditional_types(current_type: Type,
proposed_type_ranges: Optional[List[TypeRange]],
Expand Down
3 changes: 3 additions & 0 deletions mypy/message_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ def format(self, *args: object, **kwargs: object) -> "ErrorMessage":
TYPEVAR_VARIANCE_DEF: Final = 'TypeVar "{}" may only be a literal bool'
TYPEVAR_BOUND_MUST_BE_TYPE: Final = 'TypeVar "bound" must be a type'
TYPEVAR_UNEXPECTED_ARGUMENT: Final = 'Unexpected argument to "TypeVar()"'
UNBOUND_TYPEVAR: Final = (
'A function returning TypeVar should receive at least '
'one argument containing the same Typevar')

# Super
TOO_MANY_ARGS_FOR_SUPER: Final = ErrorMessage('Too many arguments for "super"')
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -3269,10 +3269,11 @@ def new_pro(pro_c: Type[P]) -> P:
return new_user(pro_c)
wiz = new_pro(WizUser)
reveal_type(wiz)
def error(u_c: Type[U]) -> P:
def error(u_c: Type[U]) -> P: # Error here, see below
return new_pro(u_c) # Error here, see below
[out]
main:11: note: Revealed type is "__main__.WizUser"
main:12: error: A function returning TypeVar should receive at least one argument containing the same Typevar
main:13: error: Value of type variable "P" of "new_pro" cannot be "U"
main:13: error: Incompatible return value type (got "U", expected "P")

Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ z: y # E: Variable "__main__.y" is not valid as a type [valid-type] \
from typing import TypeVar

T = TypeVar('T')
def f() -> T: pass
def f() -> T: pass # E: A function returning TypeVar should receive at least one argument containing the same Typevar [type-var]
x = f() # E: Need type annotation for "x" [var-annotated]
y = [] # E: Need type annotation for "y" (hint: "y: List[<type>] = ...") [var-annotated]
[builtins fixtures/list.pyi]
Expand Down
10 changes: 5 additions & 5 deletions test-data/unit/check-generics.test
Original file line number Diff line number Diff line change
Expand Up @@ -1533,9 +1533,9 @@ A = TypeVar('A')
B = TypeVar('B')

def f1(x: A) -> A: ...
def f2(x: A) -> B: ...
def f2(x: A) -> B: ... # E: A function returning TypeVar should receive at least one argument containing the same Typevar
def f3(x: B) -> B: ...
def f4(x: int) -> A: ...
def f4(x: int) -> A: ... # E: A function returning TypeVar should receive at least one argument containing the same Typevar

y1 = f1
if int():
Expand Down Expand Up @@ -1584,8 +1584,8 @@ B = TypeVar('B')
T = TypeVar('T')
def outer(t: T) -> None:
def f1(x: A) -> A: ...
def f2(x: A) -> B: ...
def f3(x: T) -> A: ...
def f2(x: A) -> B: ... # E: A function returning TypeVar should receive at least one argument containing the same Typevar
def f3(x: T) -> A: ... # E: A function returning TypeVar should receive at least one argument containing the same Typevar
def f4(x: A) -> T: ...
def f5(x: T) -> T: ...

Expand Down Expand Up @@ -1754,7 +1754,7 @@ from typing import TypeVar
A = TypeVar('A')
B = TypeVar('B')
def f1(x: int, y: A) -> A: ...
def f2(x: int, y: A) -> B: ...
def f2(x: int, y: A) -> B: ... # E: A function returning TypeVar should receive at least one argument containing the same Typevar
def f3(x: A, y: B) -> B: ...
g = f1
g = f2
Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/check-inference.test
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ g(None) # Ok
f() # Ok because not used to infer local variable type
g(a)

def f() -> T: pass
def f() -> T: pass # E: A function returning TypeVar should receive at least one argument containing the same Typevar
def g(a: T) -> None: pass
[out]

Expand Down Expand Up @@ -2341,7 +2341,7 @@ def main() -> None:
[case testDontMarkUnreachableAfterInferenceUninhabited]
from typing import TypeVar
T = TypeVar('T')
def f() -> T: pass
def f() -> T: pass # E: A function returning TypeVar should receive at least one argument containing the same Typevar

class C:
x = f() # E: Need type annotation for "x"
Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/check-literal.test
Original file line number Diff line number Diff line change
Expand Up @@ -972,15 +972,15 @@ b: bt # E: Variable "__main__.bt" is not valid as a ty
[out]

[case testLiteralDisallowTypeVar]
from typing import TypeVar
from typing import TypeVar, Tuple
from typing_extensions import Literal

T = TypeVar('T')

at = Literal[T] # E: Parameter 1 of Literal[...] is invalid
a: at

def foo(b: Literal[T]) -> T: pass # E: Parameter 1 of Literal[...] is invalid
def foo(b: Literal[T]) -> Tuple[T]: pass # E: Parameter 1 of Literal[...] is invalid
[builtins fixtures/tuple.pyi]
[out]

Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-parameter-specification.test
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ def callback(func: Callable[[Any], Any]) -> None: ...
class Job(Generic[P]): ...

@callback
def run_job(job: Job[...]) -> T: ...
def run_job(job: Job[...]) -> T: ... # E: A function returning TypeVar should receive at least one argument containing the same Typevar
[builtins fixtures/tuple.pyi]

[case testTupleAndDictOperationsOnParamSpecArgsAndKwargs]
Expand Down
1 change: 0 additions & 1 deletion test-data/unit/check-typevar-tuple.test
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,3 @@ args: Tuple[bool, int, str, int, str, object]
reveal_type(g(args)) # N: Revealed type is "Tuple[builtins.str, builtins.str, builtins.int]"
reveal_type(h(args)) # N: Revealed type is "Tuple[builtins.str, builtins.str, builtins.int]"
[builtins fixtures/tuple.pyi]

60 changes: 60 additions & 0 deletions test-data/unit/check-typevar-unbound.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@

[case testUnboundTypeVar]
from typing import TypeVar

T = TypeVar('T')

def f() -> T: # E: A function returning TypeVar should receive at least one argument containing the same Typevar
...

f()


[case testInnerFunctionTypeVar]

from typing import TypeVar

T = TypeVar('T')

def g(a: T) -> T:
def f() -> T:
...
return f()


[case testUnboundIterableOfTypeVars]
from typing import Iterable, TypeVar

T = TypeVar('T')

def f() -> Iterable[T]:
...

f()

[case testBoundTypeVar]
from typing import TypeVar

T = TypeVar('T')

def f(a: T, b: T, c: int) -> T:
...


[case testNestedBoundTypeVar]
from typing import Callable, List, Union, Tuple, TypeVar

T = TypeVar('T')

def f(a: Union[int, T], b: str) -> T:
...

def g(a: Callable[..., T], b: str) -> T:
...

def h(a: List[Union[Callable[..., T]]]) -> T:
...

def j(a: List[Union[Callable[..., Tuple[T, T]], int]]) -> T:
...
[builtins fixtures/tuple.pyi]

0 comments on commit 2c152e6

Please sign in to comment.