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

no-redef when import-importing a module to satisfy a protocol #13803

Closed
asottile opened this issue Oct 3, 2022 · 2 comments · Fixed by #13969
Closed

no-redef when import-importing a module to satisfy a protocol #13803

asottile opened this issue Oct 3, 2022 · 2 comments · Fixed by #13969
Labels
bug mypy got something wrong topic-protocols

Comments

@asottile
Copy link
Contributor

asottile commented Oct 3, 2022

Bug Report

modules-as-protocols triggers "no-redef" when using import-imports, but is fine with from-imports

(I'm testing out this unreleased feature on the primary branch)

# main.py
import os
from typing import Protocol

class P(Protocol):
    def f(self) -> str: ...

t: P
if os.environ.get('ENV') == 'prod':
    import t as t  # Name "t" already defined on line 7  [no-redef]
else:
    import u as t  # Name "t" already defined on line 7  [no-redef]
# t.py and u.py
def f() -> str:
    return ''

the errors triggered are inline above

oddly enough, a from import seems to work fine (so there's an easy workaround if the module isn't top-level):

mkdir y
touch y/__init__.py
mv {t,u}.py y
import os
from typing import Protocol

class P(Protocol):
    def f(self) -> str: ...

t: P
if os.environ.get('ENV') == 'prod':
    from y import t as t  # no errors!
else:
    from y import u as t  # no errors!

Your Environment

  • Mypy version used: mypy 0.990+dev.dc5c299aa190949f2300b163ccc10257a779006d (compiled: no)
  • Mypy command-line flags: none
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.10.6
@asottile asottile added the bug mypy got something wrong label Oct 3, 2022
@hauntsaninja
Copy link
Collaborator

Thanks for the report!

Note mainly for self: looks like the difference is between add_symbol_table_node

if (

and process_imported_symbol
existing_symbol = self.globals.get(imported_id)
. It's not obvious to me why process_import_over_existing_name isn't called for normal imports, but just changing that should probably fix

hauntsaninja added a commit to hauntsaninja/mypy that referenced this issue Oct 31, 2022
This changes our importing logic to be more consistent and to treat
import statements more like assignments.

Fixes python#13803

The primary motivation for this is when typing modules as protocols, as
in the linked issue. But it turns out we already allowed redefinition
with from imports, so this just seems like a nice consistency win.

We move shared logic from visit_import_all and visit_import_from (via
process_imported_symbol) into add_imported_symbol. We then reuse it in
visit_import.

To simplify stuff, we inline the code from add_module_symbol into
visit_import. Then we copy over logic from add_symbol, because MypyFile
is not a SymbolTableNode, but this isn't the worst thing ever.

Finally, we now need to check non-from import statements like
assignments, which was a thing we weren't doing earlier.
hauntsaninja added a commit that referenced this issue Nov 3, 2022
This changes our importing logic to be more consistent and to treat
import statements more like assignments.

Fixes #13803, fixes #13914, fixes half of #12965, probably fixes #12574

The primary motivation for this is when typing modules as protocols, as
in #13803. But it turns out we already allowed redefinition with "from"
imports, so this also seems like a nice consistency win.

We move shared logic from visit_import_all and visit_import_from (via
process_imported_symbol) into add_imported_symbol. We then reuse it in
visit_import.

To simplify stuff, we inline the code from add_module_symbol into
visit_import. Then we copy over logic from add_symbol, because MypyFile
is not a SymbolTableNode, but this isn't the worst thing ever.

Finally, we now need to check non-from import statements like
assignments, which was a thing we weren't doing earlier.
@hauntsaninja
Copy link
Collaborator

Thanks for reporting! This should now be fixed on master, but isn't part of the 0.990 release.

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

Successfully merging a pull request may close this issue.

3 participants