Implementation of overloaded function should be automatically typed #3160
Comments
Merged
|
Unfortunately, we can't cover situations where the implementation takes in |
|
I think we should close this until we have more experience here. |
|
@sixolet Nothing is lost in the case of |
|
We considered this and decided against it. There would be too many special cases to consider (e.g. overloading on different arg counts). |
facebook-github-bot
pushed a commit
to facebook/pyre-check
that referenced
this issue
Oct 25, 2018
Summary:
The typechecker needs to ignore the existence of a signature if that signature is overloaded with typing.overload decorated defines. In other words, when selecting signatures, the overloaded define (containing the actual implementation of the function) should never be selected.
typing.overload designates a definition that is purely for the typechecker's benefit, and is ignored at runtime because it is immediately overwritten with a definition that contains the actual implementation. Calling a function decorated with typing.overload throws at runtime:
```
>>> import typing
>>> typing.overload
... def foo(x):
... return 1
...
>>> foo(1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/fbcode/gcc-5-glibc-2.23/lib/python3.6/typing.py", line 1591, in _overload_dummy
"You should not call an overloaded function. "
NotImplementedError: You should not call an overloaded function. A series of The controller you requested could not be found. functions outside a stub module should always be followed by an implementation that is not The controller you requested could not be found..
```
Therefore, the purpose of typing.overload defines is simply to clarify the type signature when the return annotation should vary depending on the parameter annotations (but maybe not in a way specifiable with type vars):
```
typing.overload
def foo(x: int) -> str: ...
typing.overload
def foo(x:str) -> int:...
def foo(x: Union[int, str]) -> Union[int, str]
```
Currently, when pyre sees the above set of signatures and then tries to resolve a call to `foo(x)` where `x` is an `int`, we look at all overloads equally and can select the `Union[int, str] -> Union[int, str]` signature and end up assuming the return type of `foo(x)` is `Union[int, str]` rather than the correct and more specific `str`.
If `typing.overload` definitions exist, we should be selecting only from those, and not from the implementation signature.
**NodeAPI Bug (T35334236)**
```
overload
staticmethod
def get(roots):
# type: (TRoot) -> NodeGetCity[NodeCity]
pass
overload # noqa
staticmethod
def get(roots):
# type: (TRoots) -> NodeGetCity[List[NodeCity]]
pass
staticmethod # noqa
def get(roots):
# type: (Union[TRoot, TRoots]) -> NodeGetCity
"""
Get the node object/s corresponding to the roots.
param roots: Node|fbid, or iterable<Node|fbid>.
return A Query object that will return a Node if a Node or NodeId
was passed as roots otherwise will return a list of Nodes.
"""
return NodeGetCity(roots, nodeapi_ext.NodeErrorControllerType.ENFORCE)
```
Pyre selects the third method, which has a return type of NodeGetCity with an unknown type parameter (which inherits from awaitable), so awaiting on this returns an unknown type. Pyre *should* realize that the overloaded methods are there to refine the type signature of get depending on the input type. Merging with a pre-existing task to handle overloads this way.
**More context:**
Pep:
https://docs.python.org/3/library/typing.html#typing.overload
Discussion:
python/typing#253
python/mypy#3160
python/typing#14
**Next steps - not urgent (T35517446):**
1. When typechecking a define, if it is decorated with an typing.overload and there does not exist a matching definition *not* decorated with typing.overload. (overload functions throw when called at runtime)
2. When typechecking a define, if there exist other same name defines decorated with typing.overload, throw if overloading functions do not comprehensively cover the type signature of this overloaded function
Reviewed By: dkgi
Differential Revision: D10494018
fbshipit-source-id: 48973008d94cc539198ec13552b8108412413f2a
mthuurne
added a commit
to boxingbeetle/softfab
that referenced
this issue
May 14, 2019
PEP 484 doesn't require an overloaded function implementation to be annotated, but mypy will not check it otherwise. python/mypy#3160 Note that in dispatchlib I replaced overload by type restriction, since that was a simpler way of expressing the same rules.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Without type annotation, implementation of overloaded functions would assume all arguments are of type
Any, and so will fail to properly type check the body (that's assuming one has--checked-untyped-defsflag on - otherwise, the body is ignored completely).But annotating the implementation of the overloaded function is annoying:
In addition, even with this annotation, precise type checking is impossible. For example, if the code returns a
Tinstead ofSequence[T]by mistake whenindexis aslice, it will pass type check.I think mypy should automatically type check the implementation several times, once for each of the possible overloaded variants, and each time assume the argument and return types as provided in that variant. It should also prohibit manual type annotation of implementation of overloaded functions to avoid since it would then be worse than redundant.
The text was updated successfully, but these errors were encountered: