Skip to content

Assert at end of if statement does not help mypy value inference #3431

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
chrish42 opened this issue May 24, 2017 · 5 comments
Closed

Assert at end of if statement does not help mypy value inference #3431

chrish42 opened this issue May 24, 2017 · 5 comments
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-strict-optional

Comments

@chrish42
Copy link

While working on making zulip pass with mypy --strict-optional, I ran into the following piece of code. Mypy was essentially saying that message.get_realm() was not guaranteed to work ("Not all parts of the union have get_realm() method", or somesuch.)

if message is None:
    message = Message.objects.select_related().get(id=message_id)

rendered_content = render_markdown(message, content, realm=message.get_realm())

Let's assume, at least for here, that the get() call will never return None. I tried the following change, but mypy was still giving the exact same error:

if message is None:
    message = Message.objects.select_related().get(id=message_id)
    assert message is not None # ADDED

rendered_content = render_markdown(message, content, realm=message.get_realm())

However, with this change, mypy was now happy with the code snippet:

if message is None:
    message = Message.objects.select_related().get(id=message_id)

assert message is not None # ADDED, but outside the if, now.
rendered_content = render_markdown(message, content, realm=message.get_realm())

The two asserts clearly have the same code behavior of making it impossible for message to be None when we reach the rendered_content = ... line, but mypy only understands that currently (version 5.111) for the last code snippet only. Opening this bug report for discussion of that.

(Feel free to change the title of this bug report to a more meaningful / better one.)

@emmatyping
Copy link
Member

I discussed this with @pkch and we believe it is related to #2008.

@ilevkivskyi
Copy link
Member

I think this is something specific to None checks. The same snippet works perfectly with isinstance:

from typing import Union

x: Union[str, int]

if isinstance(x, int):
    # do something at runtime
    assert isinstance(x, str)

reveal_type(x)  # Revealed type is 'builtins.str'

@JukkaL
Copy link
Collaborator

JukkaL commented May 26, 2017

This works as expected:

from typing import Optional

def f(s: Optional[str]):
    if s is None:
        s = ''
    reveal_type(s)  # Revealed type is 'builtins.str'

However, this fails:

from typing import Optional, Any

def f(s: Optional[str], a: Any):
    if s is None:
        s = a
    reveal_type(s)  # Revealed type is 'Union[builtins.str, builtins.None]'

Assignment of Any doesn't narrow down the type:

from typing import Optional, Any

def f(s: Optional[str], a: Any):
    if s is None:
        s = a
        reveal_type(s)  # Revealed type is 'Union[builtins.str, builtins.None]'

A potential fix would be to make an assignment of Any to a variable with an optional type strip away optionality. It would be kind of ad hoc, but I'm not sure if we want to infer Any as the variable type, since that could result in false negatives. Example of how this could work:

from typing import Optional, Any

def f(s: Optional[str], a: Any):
    if s is None:
        s = a
        reveal_type(s)  # Maybe the inferred type should be just str?

A less ad hoc option would be to infer Any as the type of s after assignment, but only when the type of the variable is a union.

@JukkaL JukkaL added the bug mypy got something wrong label May 26, 2017
@ilevkivskyi ilevkivskyi changed the title Assert at end of if statement does not help mpyp value inference Assert at end of if statement does not help mypy value inference Jul 25, 2017
@JukkaL JukkaL added the false-positive mypy gave an error on correct code label May 19, 2018
@hauntsaninja
Copy link
Collaborator

I think mypy's current behaviour is good

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Aug 28, 2023
@ilevkivskyi
Copy link
Member

No mypy's behavior is not good, but after some thinking it seems to me it is likely a duplicate of #3724 (that I just re-opened), or at least they have the same root cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-strict-optional
Projects
None yet
Development

No branches or pull requests

6 participants