modulefinder: make -p actually handle namespace packages correctly (#9683)

Once we were in a directory with __init__.py we only recursed into subdirectories if those subdirectories also contained an __init__.py. This is what #9616 should have been.

Co-authored-by: hauntsaninja <>

Author: hauntsaninja

mypy/main.py

@@ -930,7 +930,7 @@ def set_strict_flags() -> None:
                                    ())
         targets = []
         # TODO: use the same cache that the BuildManager will
-        cache = FindModuleCache(search_paths, fscache, options, special_opts.packages)
+        cache = FindModuleCache(search_paths, fscache, options)
         for p in special_opts.packages:
             if os.sep in p or os.altsep and os.altsep in p:
                 fail("Package name '{}' cannot have a slash in it.".format(p),

mypy/modulefinder.py

@@ -105,9 +105,8 @@ class FindModuleCache:
 
     def __init__(self,
                  search_paths: SearchPaths,
-                 fscache: Optional[FileSystemCache] = None,
-                 options: Optional[Options] = None,
-                 ns_packages: Optional[List[str]] = None) -> None:
+                 fscache: Optional[FileSystemCache],
+                 options: Optional[Options]) -> None:
         self.search_paths = search_paths
         self.fscache = fscache or FileSystemCache()
         # Cache for get_toplevel_possibilities:
@@ -117,7 +116,6 @@ def __init__(self,
         self.results = {}  # type: Dict[str, ModuleSearchResult]
         self.ns_ancestors = {}  # type: Dict[str, str]
         self.options = options
-        self.ns_packages = ns_packages or []  # type: List[str]
 
     def clear(self) -> None:
         self.results.clear()
@@ -208,7 +206,7 @@ def _can_find_module_in_parent_dir(self, id: str) -> bool:
         of the current working directory.
         """
         working_dir = os.getcwd()
-        parent_search = FindModuleCache(SearchPaths((), (), (), ()))
+        parent_search = FindModuleCache(SearchPaths((), (), (), ()), self.fscache, self.options)
         while any(file.endswith(("__init__.py", "__init__.pyi"))
                   for file in os.listdir(working_dir)):
             working_dir = os.path.dirname(working_dir)
@@ -364,36 +362,45 @@ def find_modules_recursive(self, module: str) -> List[BuildSource]:
         if isinstance(module_path, ModuleNotFoundReason):
             return []
         result = [BuildSource(module_path, module, None)]
+
+        package_path = None
         if module_path.endswith(('__init__.py', '__init__.pyi')):
-            # Subtle: this code prefers the .pyi over the .py if both
-            # exists, and also prefers packages over modules if both x/
-            # and x.py* exist.  How?  We sort the directory items, so x
-            # comes before x.py and x.pyi.  But the preference for .pyi
-            # over .py is encoded in find_module(); even though we see
-            # x.py before x.pyi, find_module() will find x.pyi first.  We
-            # use hits to avoid adding it a second time when we see x.pyi.
-            # This also avoids both x.py and x.pyi when x/ was seen first.
-            hits = set()  # type: Set[str]
-            for item in sorted(self.fscache.listdir(os.path.dirname(module_path))):
-                abs_path = os.path.join(os.path.dirname(module_path), item)
-                if os.path.isdir(abs_path) and \
-                        (os.path.isfile(os.path.join(abs_path, '__init__.py')) or
-                        os.path.isfile(os.path.join(abs_path, '__init__.pyi'))):
-                    hits.add(item)
-                    result += self.find_modules_recursive(module + '.' + item)
-                elif item != '__init__.py' and item != '__init__.pyi' and \
-                        item.endswith(('.py', '.pyi')):
-                    mod = item.split('.')[0]
-                    if mod not in hits:
-                        hits.add(mod)
-                        result += self.find_modules_recursive(module + '.' + mod)
-        elif os.path.isdir(module_path):
-            # Even subtler: handle recursive decent into PEP 420
-            # namespace packages that are explicitly listed on the command
-            # line with -p/--packages.
-            for item in sorted(self.fscache.listdir(module_path)):
-                item, _ = os.path.splitext(item)
-                result += self.find_modules_recursive(module + '.' + item)
+            package_path = os.path.dirname(module_path)
+        elif self.fscache.isdir(module_path):
+            package_path = module_path
+        if package_path is None:
+            return result
+
+        # This logic closely mirrors that in find_sources. One small but important difference is
+        # that we do not sort names with keyfunc. The recursive call to find_modules_recursive
+        # calls find_module, which will handle the preference between packages, pyi and py.
+        # Another difference is it doesn't handle nested search paths / package roots.
+
+        seen = set()  # type: Set[str]
+        names = sorted(self.fscache.listdir(package_path))
+        for name in names:
+            # Skip certain names altogether
+            if name == '__pycache__' or name.startswith('.') or name.endswith('~'):
+                continue
+            path = os.path.join(package_path, name)
+
+            if self.fscache.isdir(path):
+                # Only recurse into packages
+                if (self.options and self.options.namespace_packages) or (
+                    self.fscache.isfile(os.path.join(path, "__init__.py"))
+                    or self.fscache.isfile(os.path.join(path, "__init__.pyi"))
+                ):
+                    seen.add(name)
+                    result.extend(self.find_modules_recursive(module + '.' + name))
+            else:
+                stem, suffix = os.path.splitext(name)
+                if stem == '__init__':
+                    continue
+                if stem not in seen and '.' not in stem and suffix in PYTHON_EXTENSIONS:
+                    # (If we sorted names) we could probably just make the BuildSource ourselves,
+                    # but this ensures compatibility with find_module / the cache
+                    seen.add(stem)
+                    result.extend(self.find_modules_recursive(module + '.' + stem))
         return result
 
 

mypy/stubgen.py

@@ -1308,7 +1308,7 @@ def find_module_paths_using_search(modules: List[str], packages: List[str],
     result = []  # type: List[StubSource]
     typeshed_path = default_lib_path(mypy.build.default_data_dir(), pyversion, None)
     search_paths = SearchPaths(('.',) + tuple(search_path), (), (), tuple(typeshed_path))
-    cache = FindModuleCache(search_paths)
+    cache = FindModuleCache(search_paths, fscache=None, options=None)
     for module in modules:
         m_result = cache.find_module(module)
         if isinstance(m_result, ModuleNotFoundReason):

mypy/stubtest.py

@@ -999,7 +999,9 @@ def build_stubs(modules: List[str], options: Options, find_submodules: bool = Fa
     """
     data_dir = mypy.build.default_data_dir()
     search_path = mypy.modulefinder.compute_search_paths([], options, data_dir)
-    find_module_cache = mypy.modulefinder.FindModuleCache(search_path)
+    find_module_cache = mypy.modulefinder.FindModuleCache(
+        search_path, fscache=None, options=options
+    )
 
     all_modules = []
     sources = []

mypy/test/testcheck.py

@@ -342,7 +342,7 @@ def parse_module(self,
             module_names = m.group(1)
             out = []
             search_paths = SearchPaths((test_temp_dir,), (), (), ())
-            cache = FindModuleCache(search_paths)
+            cache = FindModuleCache(search_paths, fscache=None, options=None)
             for module_name in module_names.split(' '):
                 path = cache.find_module(module_name)
                 assert isinstance(path, str), "Can't find ad hoc case file: %s" % module_name

mypy/test/testmodulefinder.py

@@ -32,11 +32,11 @@ def setUp(self) -> None:
         )
         options = Options()
         options.namespace_packages = True
-        self.fmc_ns = FindModuleCache(self.search_paths, options=options)
+        self.fmc_ns = FindModuleCache(self.search_paths, fscache=None, options=options)
 
         options = Options()
         options.namespace_packages = False
-        self.fmc_nons = FindModuleCache(self.search_paths, options=options)
+        self.fmc_nons = FindModuleCache(self.search_paths, fscache=None, options=options)
 
     def test__no_namespace_packages__nsx(self) -> None:
         """
@@ -159,11 +159,11 @@ def setUp(self) -> None:
         )
         options = Options()
         options.namespace_packages = True
-        self.fmc_ns = FindModuleCache(self.search_paths, options=options)
+        self.fmc_ns = FindModuleCache(self.search_paths, fscache=None, options=options)
 
         options = Options()
         options.namespace_packages = False
-        self.fmc_nons = FindModuleCache(self.search_paths, options=options)
+        self.fmc_nons = FindModuleCache(self.search_paths, fscache=None, options=options)
 
     def path(self, *parts: str) -> str:
         return os.path.join(self.package_dir, *parts)

test-data/unit/cmdline.test

@@ -810,15 +810,24 @@ def bar(a: int, b: int) -> str:
 src/anamespace/foo/bar.py:2: error: Incompatible return value type (got "int", expected "str")
 
 [case testNestedPEP420Packages]
-# cmd: mypy -p bottles --namespace-packages
-[file bottles/jars/secret/glitter.py]
+# cmd: mypy -p pkg --namespace-packages
+[file pkg/a1/b/c/d/e.py]
 x = 0  # type: str
-[file bottles/jars/sprinkle.py]
-from bottles.jars.secret.glitter import x
+[file pkg/a1/b/f.py]
+from pkg.a1.b.c.d.e import x
+x + 1
+
+[file pkg/a2/__init__.py]
+[file pkg/a2/b/c/d/e.py]
+x = 0  # type: str
+[file pkg/a2/b/f.py]
+from pkg.a2.b.c.d.e import x
 x + 1
 [out]
-bottles/jars/secret/glitter.py:1: error: Incompatible types in assignment (expression has type "int", variable has type "str")
-bottles/jars/sprinkle.py:2: error: Unsupported operand types for + ("str" and "int")
+pkg/a2/b/c/d/e.py:1: error: Incompatible types in assignment (expression has type "int", variable has type "str")
+pkg/a1/b/c/d/e.py:1: error: Incompatible types in assignment (expression has type "int", variable has type "str")
+pkg/a2/b/f.py:2: error: Unsupported operand types for + ("str" and "int")
+pkg/a1/b/f.py:2: error: Unsupported operand types for + ("str" and "int")
 
 [case testFollowImportStubs1]
 # cmd: mypy main.py