Skip to content
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

Typing narrowing fails depending on the order of if clauses #15706

Open
bersbersbers opened this issue Jul 18, 2023 · 4 comments
Open

Typing narrowing fails depending on the order of if clauses #15706

bersbersbers opened this issue Jul 18, 2023 · 4 comments
Labels
bug mypy got something wrong

Comments

@bersbersbers
Copy link

Bug Report

Type narrowing fails in the code below.

To Reproduce

def takes_one_str(string: str) -> str:
    return string


# just the problematic case -> failing
def takes_two_optional_str0a(string: str | None, whatever: bool) -> str:
    if not (whatever and (string is None)) and whatever:
        return takes_one_str(string)
    return ""


# reformulated by reversing the order of if clauses -> working
def takes_two_optional_str0b(string: str | None, whatever: bool) -> str:
    if whatever and not (whatever and (string is None)):
        return takes_one_str(string)
    return ""


# what was I actually trying to achieve -> failing
def takes_two_optional_str1(string_a: str | None, string_b: str | None) -> str:
    if (string_a is None) and (string_b is None):
        return takes_one_str("")
    if string_a is None:
        # false-positive arg-type (same without return, with elif)
        # string_b cannot be None
        return takes_one_str(string_b)
    if string_b is None:
        return takes_one_str(string_a)
    return takes_one_str(string_a + string_b)


# workaround variant -> working
def takes_two_optional_str2(string_a: str | None, string_b: str | None) -> str:
    if (string_a is not None) and (string_b is not None):
        return takes_one_str(string_a + string_b)
    if string_a is not None:
        # false-positive arg-type (same without return, with elif)
        return takes_one_str(string_a)
    if string_b is not None:
        return takes_one_str(string_b)
    return takes_one_str("")

https://mypy-play.net/?mypy=latest&python=3.11&gist=3f3ce6b3021c1a71a4ecf31d35f62a98

Expected Behavior

No errors

Actual Behavior

bug.py:8: error: Argument 1 to "takes_one_str" has incompatible type "str | None"; expected "str" [arg-type]
bug.py:26: error: Argument 1 to "takes_one_str" has incompatible type "str | None"; expected "str" [arg-type]
Found 2 errors in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: mypy 1.4.1 (compiled: yes)
  • Mypy command-line flags: None
  • Mypy configuration options from mypy.ini (and other config files): None
  • Python version used: 3.11.4
@bersbersbers bersbersbers added the bug mypy got something wrong label Jul 18, 2023
@ikonst
Copy link
Contributor

ikonst commented Jul 18, 2023

Essentially the same answer as in #15332 (comment).

if (string_a is None) and (string_b is None):
  ... # we know string_a == string_b == None
if string_a is None:
  ... # we know string_a == None but we don't apply the condition from the previous 'if'

I've created #15653 to better document expectations.

@bersbersbers
Copy link
Author

Essentially the same answer as in #15332 (comment)

Does that also explain why takes_two_optional_str0a fails while takes_two_optional_str0b works?

@ikonst
Copy link
Contributor

ikonst commented Jul 19, 2023

Does that also explain why takes_two_optional_str0a fails while takes_two_optional_str0b works?

Kind of :)

The order in which mypy narrows types matches the Python evaluation order. For example:

if isinstance(a, str) and a.split('/') == 2:
   |                      ^ when we look at rhs, we already know 'a' is a str
   |
   ^ when we look at lhs, we don't yet know 'a.split("/") == 2'

📝 BTW, we should document this, but I cannot find an appropriate section...

So yes, in takes_two_optional_str0b we narrow whatever: Literal[True] and with that context we narrow the right-hand side.

Why lack of conditional narrowing explain why it doesn't work? Let's rewrite takes_two_optional_str0a to make it clearer:

 def takes_two_optional_str0a(string: str | None, whatever: bool) -> str:
-    if not (whatever and (string is None)) and whatever:
-        return takes_one_str(string)
+    if not (whatever and (string is None)):
+        if whatever:
+            return takes_one_str(string)
+        else:
+            pass
+    else:
+        pass
     return ""

Let's annotate the new code:

if not (whatever and (string is None)):
    # In the 'if', we can't narrow 'whatever' or 'string' on their own,
    # and mypy isn't smart enough to track conditions.
    if whatever:
        # In the 'if', 'whatever: Literal[True]'.
        # We don't track conditions, so we can't narrow 'string'
        return takes_one_str(string)
    else:
        # In the 'else', narrow 'whatever: Literal[False]'.
        pass
else:
    # In the 'else', narrow 'whatever: Literal[True]' and 'string: None'
    pass

@bersbersbers
Copy link
Author

bersbersbers commented Jul 19, 2023

Thanks for the detailed explanation! Now I fully understand why current mypy works the way it does. I guess future mypy users will appreciate having this documented (both the lack of aliased type narrowing as well as the effect of the execution order), and if aliased type narrowing were supported, that would be great as well ;)

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

No branches or pull requests

2 participants