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

Fine-grained: Inconsistent behavior for nested NewType #4615

Open
JukkaL opened this issue Feb 21, 2018 · 4 comments
Open

Fine-grained: Inconsistent behavior for nested NewType #4615

JukkaL opened this issue Feb 21, 2018 · 4 comments

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 21, 2018

The output of the second increment of this fine-grained incremental mode test case doesn't generate an error, even though nothing relevant has changed:

[case testRefreshNestedNewType]
import a
[file a.py]
from typing import Union, NewType
import b

def f(self) -> None:
    X = NewType('X', A)

A = str
[file b.py]
y = 0
[file b.py.2]
y = ''
[out]

Here's the output:

a.py:5: error: Argument 2 to NewType(...) must be subclassable (got A?)
==

The expected output would be identical for both runs.

The output from the first and second runs also does not match for this slightly different test case:

[case testRefreshNestedNewType]
import a
[file a.py]
from typing import Union, NewType
import b

def f(self) -> None:
    X = NewType('X', A)

A = Union[int, str]

[file b.py]
y = 0
[file b.py.2]
y = ''
[out]

Here's the output (again expected to be identical):

a.py:5: error: Argument 2 to NewType(...) must be subclassable (got A?)
==
a.py:5: error: Argument 2 to NewType(...) must be subclassable (got "Union[int, str]")

It looks like forward references within NewType declarations behave inconsistently.

@JukkaL JukkaL added bug mypy got something wrong topic-fine-grained-incremental labels Feb 21, 2018
@ilevkivskyi
Copy link
Member

ilevkivskyi commented Feb 21, 2018

I think that at least the second one (with union) should be fixed by my alias PR #4525. My guess is that things that should be stripped are not stripped (like .analyzed).

@ilevkivskyi
Copy link
Member

Also there is in fact no big difference between two examples from point of view of forward refs vs NewType, in the first case the analyzed forward ref is str which is a valid base class, therefore no error is shown.

@ilevkivskyi ilevkivskyi self-assigned this Feb 23, 2018
@ilevkivskyi
Copy link
Member

It looks like strip_target is not called on a.py, while it should. Therefore, the type aliases are not stripped, and appear in analyzed form on the second run. I am not yet sure how to fix this.

@ilevkivskyi
Copy link
Member

After a discussion with @JukkaL it looks like the right solution for now would be to support forward references in NewTypes. We anyway allow them in many places where they should be prohibited, so this is consistent.

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

2 participants