Skip to content

report error for false type assertions #4306

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
chadrik opened this issue Dec 1, 2017 · 7 comments
Closed

report error for false type assertions #4306

chadrik opened this issue Dec 1, 2017 · 7 comments

Comments

@chadrik
Copy link
Contributor

chadrik commented Dec 1, 2017

I'm currently adding annotations to a complex module, and even after resolving most of the mypy errors within the module I suspect there are still quite a few inaccurately typed functions due to corner-cases which will only be detected in code that uses the module in specific ways. I already have unittests for this module (written using pytest) that do precisely this, so type checking these would be the ideal way to check my annotations -- all-in-one static and runtime analysis.

It would go a long way to easing adoption if mypy raised errors on type assertions that it can detect are false. e.g.:

i = 0
assert isinstance(i, str)  # error
assert type(i) is str  # error

Bonus points for supporting the python unittest module's assert*() methods, but most people these days are using pytest and as a feature request it seems much easier and more useful to simply support assert.

@elazarg
Copy link
Contributor

elazarg commented Dec 16, 2017

Note that this is somewhat in conflict with #3603. Right now, mypy gives the empty type to i, so it should be trivial to implement. But if #3603 will be resolved (e.g. using intersection types) this issue will require mypy to track the precise type of a name, instead of an upper bound, which is not something that mypy is designed to do (or, IMHO, can be expected to do; The last line is possible to handle, but it is not a common case, and not supported today at all).

@chadrik
Copy link
Contributor Author

chadrik commented Dec 16, 2017

@elazarg I don't completely follow why i = 0 would be assigned the empty type, though I do understand why mypy cannot check assert type(i) is str.

Would this be possible to check correctly:

i : int = 0
assert isinstance(i, str)  # error

@elazarg
Copy link
Contributor

elazarg commented Dec 16, 2017

It is not the i = 0 statement, but the meet of int and str that is empty.

In your specific example, it is obviously possible to check correctly since you know type(i) is int. But not in the following:

def f(i: int) -> None:
    assert isinstance(i, str)  # error

Currently mypy will assign i the empty type after the assert (and therefore will assume it is unreachable), since it does not have general intersection types; but as it is argued in (#3603) this is a false positive, since mypy does not consider multiple inheritance.

@chadrik
Copy link
Contributor Author

chadrik commented Dec 16, 2017

I see, so for some subclass of int and str the assert would be true.

Is this also the case for returned types? e.g.:

def foo() -> int:
    return 0

def test_foo():
    a = foo()
    assert isinstance(a, str)

If no, then the feature request -- generating an error when type analysis tells us the assert is false -- would still have validity.

@elazarg
Copy link
Contributor

elazarg commented Dec 16, 2017

Mypy will consider the latter type "inferred", but otherwise it is the same.

However, you can try

a: str = foo()

And mypy will complain, if that's what you want. This is a kind of static assertion, as opposed to the dynamic assertion in your examples. I think this is closer to what you actually looking for.

@ilevkivskyi
Copy link
Member

This is a duplicate of #2395

@hauntsaninja
Copy link
Collaborator

There is now typing.assert_type for this

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

No branches or pull requests

5 participants