Skip to content

Returning T instead of UserDefinedClass[T] is not reported as an error #2905

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.

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

Closed
pkch opened this issue Feb 24, 2017 · 5 comments
Closed

Returning T instead of UserDefinedClass[T] is not reported as an error #2905

pkch opened this issue Feb 24, 2017 · 5 comments

Comments

@pkch
Copy link
Contributor

pkch commented Feb 24, 2017

At first glance, the following code should not pass type verification because f and g return or yield T instead of Node[T]:

from typing import TypeVar, Generic, Iterator

T = TypeVar('T')
class Node(Generic[T]):
    def __init__(self, value: T) -> None:
        self.value = value

def f(v: Node) -> Node:
    return v.value

def g(v: Node) -> Iterator[Node]:
    yield v.value

However, mypy --strict --python-version 3.6 doesn't report any errors. It does produce a warning:

Returning Any from function with declared return type node.Node[Any]

but even the warning is only reported for f(), not for g().

To be fair, there are ways to call these functions that doesn't violate type expectations, for example f(Node(Node(1))) would return Node(1) which fits its promised return type. But I was hoping mypy might notice that the only way this code would be correct is if the argument to f() is a Node[Node] rather than just arbitrary Node. And so I was expecting mypy to force me to write:

def f(v: Node[Node]) -> Node:
    return v.value

def g(v: Node[Node]) -> Node:
    yield v.value

if that's what I meant of course. (Although it's much more likely that it's actually a bug, and that I meant to return v rather than return v.value; in that case, mypy would help me catch that bug.)

Furthermore, if I add the following code at the end of the original code snippet:

v = Node(1)
f(v)
for x in g(v):
    pass

mypy should be able to see that something is definitely wrong; at this point it's completely clear that f(v) won't return the promised Node, and g(v) won't yield one. But adding those lines does not add any new error or warning messages.

@miedzinski
Copy link
Contributor

This is because v.value is Any. Bare Node is equivalent to Node[Any], so this happens

def f(v: Node) -> Node:
    reveal_type(v.value)  # E: Revealed type is 'Any'
    return v.value

This is why mypy has --warn-return-any CLI flag, which catches these errors

def f(v: Node) -> Node:
    return v.value  # W: Returning Any from function with declared return type "x.Node[Any]"

In this case you probably wanted to define the function as

def f(v: Node[T]) -> Node[T]:
    return v.value  # E: Incompatible return value type (got "T", expected Node[T])

and mypy would find an error.

@pkch
Copy link
Contributor Author

pkch commented Feb 24, 2017

Ah that makes sense. Maybe --warn-return-any should be included in --strict ? The strict mode seems like a good way to learn mypy, so it might be helpful if it can point out when Any is used unintentionally.

@pkch
Copy link
Contributor Author

pkch commented Feb 24, 2017

Opps, sorry --strict does include --warn-return-any, and so I had that flag on. I understand why in case of function f() I get a warning about returning Any instead of an outright error in type validation.

The only remaining issue is that mypy --strict doesn't report any warning at all when Node[Any] is yielded (like in the case of g()). Similarly, there's no warning when Node[Any] is implicitly used in any other situations (apart from return), for example as a variable type.

@miedzinski
Copy link
Contributor

Looks like a bug to me. This check is only done in TypeChecker.visit_return_stmt, but probably should be extended to generators as well.

def g(v: Node[T]) -> Iterator[Node[T]]:
    yield v.value

is catched.

I see another issue with --warn-return-any - it reports errors on

def g(v: Node) -> Iterator[Node]:
    yield v.value
    return v.value

but return x here is equivalent to raise StopIteration(x), so I think mypy should ignore it.

@ddfisher
Copy link
Collaborator

Agreed. --warn-return-any is too specific -- you should be able to get warnings about Any in more positions. We're tracking that issue in #2901.

With regards to the new issue @miedzinski raised: you shouldn't really be returning a value there anyway, as it's unusable. Feel free to file a separate issue about it, though.

(Closing as a duplicate of #2901.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants