Restore all_are_submodules import logic as workaround for #4498

The logic in build to determine what imported modules are depended on
used to elide dependencies to m in `from m import a, b, c` if all of
a, b, c were submodules. This was removed in #4910 because it seemed
like it ought not be necessary (and that semantically there *was*
a dependency), and early versions of #4910 depended removing it.

The addition of this dependency, though, can cause cycles that
wouldn't be there otherwise, which can cause #4498 (invalid type when
using aliases in import cycles) to trip when it otherwise wouldn't.

We've seen this once in a bug report and once internally, so restore
the `all_are_submodules` logic in avoid triggering #4498 in these
cases.

Fixes #5015

Author: msullivan

mypy/build.py

@@ -696,7 +696,6 @@ def correct_rel_imp(imp: Union[ImportFrom, ImportAll]) -> str:
                                    blocker=True)
 
             return new_id
-
         res = []  # type: List[Tuple[int, str, int]]
         for imp in file.imports:
             if not imp.is_unreachable:
@@ -713,17 +712,24 @@ def correct_rel_imp(imp: Union[ImportFrom, ImportAll]) -> str:
                 elif isinstance(imp, ImportFrom):
                     cur_id = correct_rel_imp(imp)
                     pos = len(res)
+                    all_are_submodules = True
                     # Also add any imported names that are submodules.
                     pri = import_priority(imp, PRI_MED)
                     for name, __ in imp.names:
                         sub_id = cur_id + '.' + name
                         if self.is_module(sub_id):
                             res.append((pri, sub_id, imp.line))
-                    # Add cur_id as a dependency, even if all of the
-                    # imports are submodules. Processing import from will try
-                    # to look through cur_id, so we should depend on it.
-                    pri = import_priority(imp, PRI_HIGH)
-                    res.insert(pos, ((pri, cur_id, imp.line)))
+                        else:
+                            all_are_submodules = False
+                    # If all imported names are submodules, don't add
+                    # cur_id as a dependency. This is basically a workaround
+                    # of bugs in cycle handling (#4498).
+                    # Otherwise (i.e., if at least one imported name
+                    # isn't a submodule) cur_id is also a dependency,
+                    # and we should insert it *before* any submodules.
+                    if not all_are_submodules:
+                        pri = import_priority(imp, PRI_HIGH)
+                        res.insert(pos, ((pri, cur_id, imp.line)))
                 elif isinstance(imp, ImportAll):
                     pri = import_priority(imp, PRI_HIGH)
                     res.append((pri, correct_rel_imp(imp), imp.line))

test-data/unit/check-modules.test

@@ -1001,6 +1001,28 @@ from foo import bar
 [file foo/bar.py]
 pass
 
+[case testImportReExportFromChildrentInCycle]
+# cmd: mypy -m project.root project.study.a project.neighbor
+[file project/__init__.py]
+from project.study import CustomType
+x = 10
+[file project/root.py]
+[file project/study/__init__.py]
+from project.study.a import CustomType
+[file project/study/a.py]
+from project import root
+# TODO (#4498): This test is basically testing the `all_are_submodules` logic
+# in build, which skips generating a dependenecy to a module if
+# everything in it is a submodule. But that is still all just a
+# workaround for bugs in cycle handling. If we uncomment the next
+# line, we'll still break:
+# from project import x
+CustomType = str
+[file project/neighbor/__init__.py]
+from project.study import CustomType
+def m(arg: CustomType) -> str:
+    return 'test'
+
 [case testSuperclassInImportCycle]
 import a
 import d