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

Circular import of name defined in same module, reported as an error. #12574

Closed
mrolle45 opened this issue Apr 13, 2022 · 0 comments · Fixed by #13969
Closed

Circular import of name defined in same module, reported as an error. #12574

mrolle45 opened this issue Apr 13, 2022 · 0 comments · Fixed by #13969
Labels
bug mypy got something wrong topic-import-cycles

Comments

@mrolle45
Copy link

I have a project, which I've reduced to a minimum which shows the same problem. It has 9 modules, excerpts are:

debugging.py:

from typing import *
if TYPE_CHECKING:
	from .instr_common import InstrCtx
	from .instruct import InstrTab
----------
instr_common.py:

from __future__ import annotations
from .common import *
from . import debugging

class InstrCtx(NamedTuple):
	tab : InstrTab
----------------
instruct.py:

from __future__ import annotations
from .instr_merge import *

The other modules, with the above, form an SCC of imports, so all 9 are analyzed together.

Expected Behavior
solver\debugging.py:12:2: error: Module "solver.instruct" has no attribute "InstrTab"
The InstrTab import is an error because the name does not exist in the imported module.
The class def of InstrCtx should be OK, since the import from common is importing the same thing.

Actual Behavior

solver\debugging.py:11:2: error: Module "solver.instr_common" has no attribute "InstrCtx"
solver\debugging.py:12:2: error: Module "solver.instruct" has no attribute "InstrTab"
solver\instr_common.py:12:1: error: Name "InstrCtx" already defined (possibly by an import)

Discussion
In some cases, a circular import would be a programming error, as the runtime results could vary depending on which of the modules in the SCC is imported first.
In my case, the import in solver.debugging is guarded by TYPE_CHECKING, and so it has no effect at runtime. mypy makes a placeholder for solver.debugging.InstrCtx, then later this is imported into solver.instr_common as a Var object.
Now if the placeholder resulted from a guarded import, then mypy should not consider that InstrCtx is imported at runtime, and so it should be ignored.
In effect, the import of solver.debugging.InstrCtx is private. It is not visible to other modules, and is visible within solver.debugging only for type analysis purposes.

I propose adding the following to semanal.py to make imported names non-public when MYPY-guarded:

in SemanticAnalyzer.visit_import:
			if as_id is not None:
				base_id = id
				imported_id = as_id
				module_public = use_implicit_reexport or id.split(".")[-1] == as_id
			else:
				base_id = id.split('.')[0]
				imported_id = base_id
				module_public = use_implicit_reexport
--->			if i.is_mypy_only: module_public = False

in SemanticAnalyzer.visit_import_all:
					module_public = self.is_stub_file or self.options.implicit_reexport
--->					if i.is_mypy_only: module_public = False

in SemanticAnalyzer.visit_import_from:
			module_public = use_implicit_reexport or (as_id is not None and id == as_id)
--->			if imp.is_mypy_only: module_public = False

Attached is a zip file with:

  • All the project modules.
  • The config file.
  • log.txt from mypy run before making changes.
  • log2.txt from mypy run after making changes.
    Exp3.zip

Your Environment

  • Mypy version used: 0.931
  • Mypy command-line flags: -v
  • Mypy configuration options: see attached
  • Python version used: 3.7
  • Operating system and version: Windows 10
@mrolle45 mrolle45 added the bug mypy got something wrong label Apr 13, 2022
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.
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-import-cycles
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants