Skip to content

New semantic analyzer: Support idem forward aliases #6404

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
ilevkivskyi opened this issue Feb 15, 2019 · 8 comments · Fixed by #7144
Closed

New semantic analyzer: Support idem forward aliases #6404

ilevkivskyi opened this issue Feb 15, 2019 · 8 comments · Fixed by #7144
Assignees
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-0-high semantic-analyzer Problems that happen during semantic analysis

Comments

@ilevkivskyi
Copy link
Member

This is a follow-up for #6390

This tricky corner case is used in some real code and is supported by old semantic analyzer

C = C
class C:  # type: ignore
    pass

x: C

But with new analyzer this results in Invalid type since the first definition always wins. Probably we just need to special-case this somehow.

@ilevkivskyi ilevkivskyi added priority-1-normal semantic-analyzer Problems that happen during semantic analysis labels Feb 15, 2019
@ilevkivskyi
Copy link
Member Author

ilevkivskyi commented Mar 14, 2019

EDIT: The new behavior is actually correct, see below.

Another pattern that we need to support with new semantic analyzer is this:

class In:
    ...
class Out:
    In = In  # recognized as variable

this currently (both before and after assignment refactor) causes a bunch of troubles in internal code bases. Raising priority to high.

@ilevkivskyi
Copy link
Member Author

Oh, wait, this is actually correct, by the rules non-indexed assignments at class scope are always variables, so this was previously a false negative, OK back to normal priority.

@ilevkivskyi
Copy link
Member Author

Another repro for this issue with a weird error:

x: C
class C:
    pass
C = C

fails with

main:2: error: Name 'C' already defined on line 4

cc @JukkaL

@JukkaL JukkaL added bug mypy got something wrong false-positive mypy gave an error on correct code labels Jun 12, 2019
@JukkaL
Copy link
Collaborator

JukkaL commented Jul 3, 2019

The examples look meaningless, since the code won't work at runtime. I wonder if the actual code where this comes up is different (involving try/except perhaps). This could also affect the way we'd fix this.

@ilevkivskyi
Copy link
Member Author

Yes, the actual code is like

try:
    C = C
except NameError:
    class C:
        ...

Also the second example works without try/except.

I think we should fix this before the release since this is actually used in some projects.

@JukkaL
Copy link
Collaborator

JukkaL commented Jul 3, 2019

Yes, it would be good to make this better. I'm increasing priority.

But even that example seems incomplete, as that will always generate a NameError. I assume it's more like this:

try:
    ModuleNotFoundError = ModuleNotFoundError
except NameError:
    class ModuleNotFoundError(Exception):
        ...

The assignment will fail on some Python versions and pass on some.

@ilevkivskyi
Copy link
Member Author

Most likely, I don't remember TBH, also aiohttp (who first reported this) used to have tons of * imports. This may be some other name they define in another module depending on sys.platform and then star import it.

@ilevkivskyi
Copy link
Member Author

I did some digging and it looks like this happens sometimes after star imports from C acceleration modules.

@JukkaL JukkaL self-assigned this Jul 3, 2019
JukkaL added a commit that referenced this issue Jul 4, 2019
We special case these since a more general fix, such as looking
at line numbers of definitions, would break a ton of test cases.

Fixes #6404.
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-0-high semantic-analyzer Problems that happen during semantic analysis
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants