Refine parent type when narrowing "lookup" expressions #7917

Michael0x2a · 2019-11-09T23:10:50Z

This diff adds support for the following pattern:

from typing import Enum, List
from enum import Enum

class Key(Enum):
    A = 1
    B = 2

class Foo:
    key: Literal[Key.A]
    blah: List[int]

class Bar:
    key: Literal[Key.B]
    something: List[str]

x: Union[Foo, Bar]
if x.key is Key.A:
    reveal_type(x)  # Revealed type is 'Foo'
else:
    reveal_type(x)  # Revealed type is 'Bar'

In short, when we do x.key is Key.A, we "propagate" the information we discovered about x.key up one level to refine the type of x.

We perform this propagation only when x is a Union and only when we are doing member or index lookups into instances, typeddicts, namedtuples, and tuples. For indexing operations, we have one additional limitation: we must use a literal expression in order for narrowing to work at all. Using Literal types or Final instances won't work; See #7905 for more details.

To put it another way, this adds support for tagged unions, I guess.

This more or less resolves #7344. We currently don't have support for narrowing based on string or int literals, but that's a separate issue and should be resolved by #7169 (which I resumed work on earlier this week).

This diff adds support for the following pattern: ```python from typing import Enum, List from enum import Enum class Key(Enum): A = 1 B = 2 class Foo: key: Literal[Key.A] blah: List[int] class Bar: key: Literal[Key.B] something: List[str] x: Union[Foo, Bar] if x.key is Key.A: reveal_type(x) # Revealed type is 'Foo' else: reveal_type(x) # Revealed type is 'Bar' ``` In short, when we do `x.key is Key.A`, we "propagate" the information we discovered about `x.key` up one level to refine the type of `x`. We perform this propagation only when `x` is a Union and only when we are doing member or index lookups into instances, typeddicts, namedtuples, and tuples. For indexing operations, we have one additional limitation: we *must* use a literal expression in order for narrowing to work at all. Using Literal types or Final instances won't work; See python#7905 for more details. To put it another way, this adds support for tagged unions, I guess. This more or less resolves python#7344. We currently don't have support for narrowing based on string or int literals, but that's a separate issue and should be resolved by python#7169 (which I resumed work on earlier this week).

Michael0x2a · 2019-11-09T23:16:30Z

A few more notes:

I verified this PR produces no new errors in the larger of our two internal codebases. I got lazy and didn't check what happened on the smaller, sorry.
I'm planning on submitting docs for this in a separate PR, if that's all right.

ilevkivskyi

Thanks! This is a really great feature (and will be especially useful for people familiar with TypeScript). I just have one larger comment, and few small ones.

test-data/unit/check-narrowing.test

ilevkivskyi · 2019-11-11T13:06:14Z

test-data/unit/check-narrowing.test

+    reveal_type(x.key)  # N: Revealed type is 'Literal[__main__.Key.A]'
+    reveal_type(x)      # N: Revealed type is 'Union[__main__.Object1, Any]'
+else:
+    # TODO: Is this a bug? Should we skip inferring Any for singleton types?


I think we have some special-casing for None, but generally yes, unions with Any and singletons is indeed problematic.

test-data/unit/check-narrowing.test

ilevkivskyi · 2019-11-11T13:08:39Z

test-data/unit/fixtures/tuple.pyi

@@ -21,7 +21,8 @@ class function: pass
 class ellipsis: pass

 # We need int and slice for indexing tuples.
-class int: pass
+class int:
+    def __neg__(self) -> 'int': pass


I assume you need this for a test like if isinstance(t[-1], int): ..., but I can't find this test.

This was because of the one-line change in checkexpr.py: since we're deducing a type for index expressions now, a few unrelated tests about tuples and negative indices started failing.

ilevkivskyi · 2019-11-11T13:09:26Z

mypy/typeops.py

    if isinstance(expr, StrExpr):
        return [expr.value]

+    # TODO: See if we can eliminate this function and call the below one directly


What kind of problems does this cause?

Not sure, I didn't try. It seemed like the kind of refactoring that was out-of-scope for this PR and is something we're already sort of tracking in #6138 and #6123.

OK, let's leave this out for a separate PR.

mypy/checker.py

ilevkivskyi · 2019-11-11T13:52:34Z

mypy/checker.py

+            # Next, try using this information to refine the parent type, if applicable.
+            # Note that we currently refine just the immediate parent.
+            #
+            # TODO: Should we also try recursively refining any parents of the parents?


I think we should. This was one of the first things I thought of when looking at the PR. Supporting only immediate parents may look arbitrary to a user and potentially cause confusions. Imagine a simple situation:

class Model(Generic[T]): attr: T class A: model: Model[int] class B: model: Model[str] x: Union[A, B]: if isinstance(x.model.attr, int): ... # I would want 'x' to be an 'A' here

I think we should just cycle up until the expression is NameExpr (IIRC all bindable expressions root there). I don't think this will cause any performance impact (unless you have a proof of the opposite).

Michael0x2a · 2019-11-11T19:21:21Z

@ilevkivskyi -- Ok, this should be ready for a second look!

I added support for cycling up all parents, as suggested.

ilevkivskyi

Thanks for updates! Here are few more suggestions.

ilevkivskyi · 2019-11-11T20:51:19Z

mypy/checker.py

 from mypy import state, errorcodes as codes
 from mypy.traverser import has_return_statement, all_return_statements
 from mypy.errorcodes import ErrorCode

 T = TypeVar('T')

+T_contra = TypeVar('T_contra', contravariant=True)


I don't think the new line above is needed.

ilevkivskyi · 2019-11-11T20:52:38Z

mypy/checker.py

+
+        We perform this kind of "parent narrowing" for member lookup expressions and indexing
+        expressions into tuples, namedtuples, and typeddicts. This narrowing is also performed
+        only once, for the immediate parents of any "lookup" expressions in `new_types`.


This is now outdated.

ilevkivskyi · 2019-11-11T20:58:20Z

test-data/unit/check-narrowing.test

+    reveal_type(x.key)  # N: Revealed type is 'Union[Any, Literal[__main__.Key.B]]'
+    reveal_type(x)      # N: Revealed type is 'Union[__main__.Object1, __main__.Object2, Any]'
+
+[case testNarrowingParentsHierarchy]


Just in case I would add a test involving "double index" like if x['model']['kind'] is ...: ... for nested typed dicts.

ilevkivskyi · 2019-11-11T21:00:08Z

@Michael0x2a also note mypyc build failed.

Michael0x2a added 2 commits November 9, 2019 15:54

Import Type from typing_extensions for Python 3.5.1

b707f3d

Remove out-of-date comment

d183f1c

ilevkivskyi reviewed Nov 11, 2019

View reviewed changes

Michael0x2a added 2 commits November 11, 2019 10:57

Respond to code review

2c2506d

Refactor replay lookup for index exprs

0c33147

ilevkivskyi approved these changes Nov 11, 2019

View reviewed changes

Michael0x2a mentioned this pull request Nov 12, 2019

Isinstance checks against Type[SomeProtocol] unexpectedly fails #7932

Closed

Respond to second code review

68b4801

Michael0x2a closed this Nov 12, 2019

Michael0x2a reopened this Nov 12, 2019

Michael0x2a added 3 commits November 12, 2019 17:08

Revert index lookup helper refactoring

c2b2cce

Merge branch 'master' into add-parent-refinement

0c53aad

Fix lint error from bad merge

b4f35ea

Michael0x2a merged commit 648d99a into python:master Nov 13, 2019

Michael0x2a deleted the add-parent-refinement branch November 13, 2019 02:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refine parent type when narrowing "lookup" expressions #7917

Refine parent type when narrowing "lookup" expressions #7917

Michael0x2a commented Nov 9, 2019

Michael0x2a commented Nov 9, 2019

ilevkivskyi left a comment

ilevkivskyi Nov 11, 2019

ilevkivskyi Nov 11, 2019

Michael0x2a Nov 11, 2019

ilevkivskyi Nov 11, 2019

Michael0x2a Nov 11, 2019

ilevkivskyi Nov 11, 2019

ilevkivskyi Nov 11, 2019

Michael0x2a commented Nov 11, 2019

ilevkivskyi left a comment

ilevkivskyi Nov 11, 2019

ilevkivskyi Nov 11, 2019

ilevkivskyi Nov 11, 2019

ilevkivskyi commented Nov 11, 2019

Refine parent type when narrowing "lookup" expressions #7917

Refine parent type when narrowing "lookup" expressions #7917

Conversation

Michael0x2a commented Nov 9, 2019

Michael0x2a commented Nov 9, 2019

ilevkivskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michael0x2a commented Nov 11, 2019

ilevkivskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Nov 11, 2019