Restore all_are_submodules import logic as workaround for #4498 (#5016)

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 on 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.

Unfortunately the dependency on the module is actually required for
correctness in some corner cases, so instead of eliding the import, we
lower its priority. This causes the cycles in the regressions we are
looking at to get processed in the order that works.

This is obviously just a workaround.

Author: msullivan

mypy/build.py

@@ -713,16 +713,22 @@ 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))
+                        else:
+                            all_are_submodules = False
                     # 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)
+                    # As a workaround for for some bugs in cycle handling (#4498),
+                    # if all of the imports are submodules, do the import at a lower
+                    # priority.
+                    pri = import_priority(imp, PRI_HIGH if not all_are_submodules else PRI_LOW)
                     res.insert(pos, ((pri, cur_id, imp.line)))
                 elif isinstance(imp, ImportAll):
                     pri = import_priority(imp, PRI_HIGH)

test-data/unit/check-modules.test

@@ -1001,6 +1001,48 @@ from foo import bar
 [file foo/bar.py]
 pass
 
+[case testImportReExportFromChildrenInCycle1]
+# 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 testImportReExportFromChildrenInCycle2]
+# cmd: mypy -m project project.b project.ba project.c
+# See comments in above test about this being a workaround.
+[file foo.py]
+def get_foo() -> int: return 12
+
+[file project/ba.py]
+from . import b
+b.FOO
+
+[file project/b.py]
+import foo
+from . import c
+FOO = foo.get_foo()
+
+[file project/c.py]
+
+[file project/__init__.py]
+from . import ba
+
 [case testSuperclassInImportCycle]
 import a
 import d