From 0737912f3381e387de2a331aeacbca3a17e7294d Mon Sep 17 00:00:00 2001 From: Maxim Novikov Date: Wed, 22 Nov 2017 07:13:29 +0100 Subject: [PATCH 1/6] Namespace implementation (#1645) --- mypy/build.py | 321 ++++++++++++++++----------- mypy/main.py | 5 +- mypy/options.py | 3 + mypy/semanal.py | 11 +- mypy/stubgen.py | 3 +- mypy/test/testcheck.py | 4 +- mypy/test/testdmypy.py | 3 +- mypy/test/testgraph.py | 4 +- mypy/test/testmodulediscovery.py | 141 ++++++++++++ runtests.py | 1 + test-data/unit/check-namespaces.test | 87 ++++++++ 11 files changed, 443 insertions(+), 140 deletions(-) create mode 100644 mypy/test/testmodulediscovery.py create mode 100644 test-data/unit/check-namespaces.test diff --git a/mypy/build.py b/mypy/build.py index 21fde0239c16..26fa07637d12 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -200,11 +200,14 @@ def build(sources: List[BuildSource], source_set = BuildSourceSet(sources) errors = Errors(options.show_error_context, options.show_column_numbers) plugin = load_plugins(options, errors) + mod_discovery = ModuleDiscovery( + lib_path, namespaces_allowed=options.namespace_packages + ) # Construct a build manager object to hold state during the build. # # Ignore current directory prefix in error messages. - manager = BuildManager(data_dir, lib_path, + manager = BuildManager(data_dir, ignore_prefix=os.getcwd(), source_set=source_set, reports=reports, @@ -212,7 +215,8 @@ def build(sources: List[BuildSource], version_id=__version__, plugin=plugin, errors=errors, - saved_cache=saved_cache) + saved_cache=saved_cache, + module_discovery=mod_discovery) try: graph = dispatch(sources, manager) @@ -505,7 +509,6 @@ class BuildManager: Attributes: data_dir: Mypy data directory (contains stubs) - lib_path: Library path for looking up modules modules: Mapping of module ID to MypyFile (shared by the passes) semantic_analyzer: Semantic analyzer, pass 2 @@ -524,7 +527,6 @@ class BuildManager: """ def __init__(self, data_dir: str, - lib_path: List[str], ignore_prefix: str, source_set: BuildSourceSet, reports: Reports, @@ -532,22 +534,23 @@ def __init__(self, data_dir: str, version_id: str, plugin: Plugin, errors: Errors, + module_discovery: 'ModuleDiscovery', saved_cache: Optional[SavedCache] = None, ) -> None: self.start_time = time.time() self.data_dir = data_dir self.errors = errors self.errors.set_ignore_prefix(ignore_prefix) - self.lib_path = tuple(lib_path) self.source_set = source_set self.reports = reports self.options = options self.version_id = version_id self.modules = {} # type: Dict[str, MypyFile] self.missing_modules = set() # type: Set[str] + self.module_discovery = module_discovery self.plugin = plugin self.semantic_analyzer = SemanticAnalyzerPass2(self.modules, self.missing_modules, - lib_path, self.errors, self.plugin) + self.errors, self.plugin) self.semantic_analyzer_pass3 = SemanticAnalyzerPass3(self.modules, self.errors, self.semantic_analyzer) self.all_types = {} # type: Dict[Expression, Type] # Used by tests only @@ -628,9 +631,12 @@ def correct_rel_imp(imp: Union[ImportFrom, ImportAll]) -> str: return res + def find_module(self, id: str) -> Optional[str]: + return self.module_discovery.find_module(id) + def is_module(self, id: str) -> bool: """Is there a file in the file system corresponding to module id?""" - return find_module(id, self.lib_path) is not None + return self.find_module(id) is not None def parse_file(self, id: str, path: str, source: str, ignore_errors: bool) -> MypyFile: """Parse the source of a file with the given name. @@ -732,9 +738,6 @@ def remove_cwd_prefix_from_path(p: str) -> str: return p -# Cache find_module: (id, lib_path) -> result. -find_module_cache = {} # type: Dict[Tuple[str, Tuple[str, ...]], Optional[str]] - # Cache some repeated work within distinct find_module calls: finding which # elements of lib_path have even the subdirectory they'd need for the module # to exist. This is shared among different module ids when they differ only @@ -756,7 +759,6 @@ def remove_cwd_prefix_from_path(p: str) -> str: def find_module_clear_caches() -> None: - find_module_cache.clear() find_module_dir_cache.clear() find_module_listdir_cache.clear() find_module_is_file_cache.clear() @@ -797,99 +799,162 @@ def is_file(path: str) -> bool: return res -def find_module(id: str, lib_path_arg: Iterable[str]) -> Optional[str]: - """Return the path of the module source file, or None if not found.""" - lib_path = tuple(lib_path_arg) - - def find() -> Optional[str]: - # If we're looking for a module like 'foo.bar.baz', it's likely that most of the - # many elements of lib_path don't even have a subdirectory 'foo/bar'. Discover - # that only once and cache it for when we look for modules like 'foo.bar.blah' - # that will require the same subdirectory. - components = id.split('.') - dir_chain = os.sep.join(components[:-1]) # e.g., 'foo/bar' - if (dir_chain, lib_path) not in find_module_dir_cache: - dirs = [] - for pathitem in lib_path: - # e.g., '/usr/lib/python3.4/foo/bar' - isdir = find_module_isdir_cache.get((pathitem, dir_chain)) - if isdir is None: - dir = os.path.normpath(os.path.join(pathitem, dir_chain)) - isdir = os.path.isdir(dir) - find_module_isdir_cache[pathitem, dir_chain] = isdir - if isdir: - dirs.append(dir) - find_module_dir_cache[dir_chain, lib_path] = dirs - candidate_base_dirs = find_module_dir_cache[dir_chain, lib_path] - - # If we're looking for a module like 'foo.bar.baz', then candidate_base_dirs now - # contains just the subdirectories 'foo/bar' that actually exist under the - # elements of lib_path. This is probably much shorter than lib_path itself. - # Now just look for 'baz.pyi', 'baz/__init__.py', etc., inside those directories. - seplast = os.sep + components[-1] # so e.g. '/baz' - sepinit = os.sep + '__init__' - for base_dir in candidate_base_dirs: - base_path = base_dir + seplast # so e.g. '/usr/lib/python3.4/foo/bar/baz' - # Prefer package over module, i.e. baz/__init__.py* over baz.py*. - for extension in PYTHON_EXTENSIONS: - path = base_path + sepinit + extension - if is_file(path) and verify_module(id, path): - return path - # No package, look for module. - for extension in PYTHON_EXTENSIONS: - path = base_path + extension - if is_file(path) and verify_module(id, path): - return path - return None +class ModuleType: + PACKAGE = 1 + MODULE = 2 + NAMESPACE = 3 - key = (id, lib_path) - if key not in find_module_cache: - find_module_cache[key] = find() - return find_module_cache[key] +class ImportContext: + def __init__(self) -> None: + self.has_py = False # type: bool + self.type = None # type: Optional[int] + self.paths = [] # type: List[str] -def find_modules_recursive(module: str, lib_path: List[str]) -> List[BuildSource]: - module_path = find_module(module, lib_path) - if not module_path: - return [] - result = [BuildSource(module_path, module, 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(os.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 += find_modules_recursive(module + '.' + item, lib_path) - 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 += find_modules_recursive( - module + '.' + mod, lib_path) - return result - - -def verify_module(id: str, path: str) -> bool: - """Check that all packages containing id have a __init__ file.""" - if path.endswith(('__init__.py', '__init__.pyi')): - path = dirname(path) - for i in range(id.count('.')): - path = dirname(path) - if not any(is_file(os.path.join(path, '__init__{}'.format(extension))) - for extension in PYTHON_EXTENSIONS): + def maybe_add_path(self, path: str, type: int) -> None: + if self.type is not None and self.type != type: + return None + + # We can add at most one module implementation to paths + # But module can have multiple stubs + py_path = path.endswith('.py') + if self.has_py and py_path: + return None + + if type == ModuleType.NAMESPACE: + ok = self._verify_namespace(path) + else: + ok = self._verify_module(path) + + if not ok: + return None + + if py_path: + self.has_py = True + + self.type = type + self.paths.append(path) + + def _verify_module(self, path: str) -> bool: + if not is_file(path): return False - return True + + return True + + def _verify_namespace(self, path: str) -> bool: + if os.path.isdir(path): + files = set(list_dir(path) or []) # type: Set[str] + if not ('__init__.py' in files or '__init__.pyi' in files): + return True + + return False + + +def is_pkg_path(path: str) -> bool: + return path.endswith(('__init__.py', '__init__.pyi')) + + +def is_module_path(path: str) -> bool: + return not is_pkg_path(path) and path.endswith(('.py', '.pyi')) + + +def is_namespace_path(path: str) -> bool: + return path.endswith(os.path.sep) + + +class ModuleDiscovery: + def __init__(self, + lib_path: Iterable[str], + namespaces_allowed: bool = False) -> None: + + self.lib_path = tuple(os.path.normpath(p) for p in lib_path) # type: Tuple[str, ...] + self.namespaces_allowed = namespaces_allowed + self._find_module_cache = {} # type: Dict[str, List[str]] + + def find_module(self, id: str) -> Optional[str]: + paths = self._find_module(id) + if paths: + return paths[0] + else: + return None + + def find_modules_recursive(self, module: str) -> List[BuildSource]: + hits = set() # type: Set[str] + result = [] # type: List[BuildSource] + for src in self._find_modules_recursive(module): + if src.module not in hits: + hits.add(src.module) + result.append(src) + return result + + def _find_modules_recursive(self, module: str) -> List[BuildSource]: + module_paths = self._find_module(module) + + srcs = [] # type: List[BuildSource] + for path in module_paths: + if is_module_path(path) or is_pkg_path(path): + srcs.append(BuildSource(path, module, None)) + + if is_pkg_path(path): + path = dirname(path) + for submodule in self._find_submodules(module, path): + srcs += self._find_modules_recursive(submodule) + elif is_namespace_path(path): + for submodule in self._find_submodules(module, path): + srcs += self._find_modules_recursive(submodule) + + return srcs + + def _find_submodules(self, module: str, path: str) -> Iterator[str]: + for item in list_dir(path) or []: + if item == '__init__.py' or item == '__init__.pyi': + continue + + if item.endswith(tuple(PYTHON_EXTENSIONS)): + item = item.split('.')[0] + + yield module + '.' + item + + def _collect_paths(self, paths: List[str], last_comp: str) -> List[str]: + """ + Collect all available module paths + """ + sepinit = '__init__' + ctx = ImportContext() + + for path_item in paths: + if is_module_path(path_item): + continue + + if is_pkg_path(path_item): + path_item = dirname(path_item) + + for ext in PYTHON_EXTENSIONS: + path = os.path.join(path_item, last_comp, sepinit + ext) + ctx.maybe_add_path(path, ModuleType.PACKAGE) + + for ext in PYTHON_EXTENSIONS: + path = os.path.join(path_item, last_comp + ext) + ctx.maybe_add_path(path, ModuleType.MODULE) + + if self.namespaces_allowed: + path = os.path.join(path_item, last_comp) + ctx.maybe_add_path(path + os.sep, ModuleType.NAMESPACE) + + return ctx.paths + + def _find_module(self, id: str) -> List[str]: + if id not in self._find_module_cache: + components = id.split('.') + parent_paths = list(self.lib_path) + + if len(components) > 1: + parent_id = '.'.join(components[:-1]) + parent_paths = self._find_module(parent_id) + + last_comp = components[-1] + self._find_module_cache[id] = self._collect_paths(parent_paths, last_comp) + return self._find_module_cache[id] def read_with_python_encoding(path: str, pyversion: Tuple[int, int]) -> Tuple[str, str]: @@ -1521,32 +1586,36 @@ def __init__(self, # difference and just assume 'builtins' everywhere, # which simplifies code. file_id = '__builtin__' - path = find_module(file_id, manager.lib_path) + path = manager.find_module(file_id) if path: - # For non-stubs, look at options.follow_imports: - # - normal (default) -> fully analyze - # - silent -> analyze but silence errors - # - skip -> don't analyze, make the type Any - follow_imports = self.options.follow_imports - if (follow_imports != 'normal' - and not root_source # Honor top-level modules - and path.endswith('.py') # Stubs are always normal - and id != 'builtins'): # Builtins is always normal - if follow_imports == 'silent': - # Still import it, but silence non-blocker errors. - manager.log("Silencing %s (%s)" % (path, id)) - self.ignore_all = True - else: - # In 'error' mode, produce special error messages. - manager.log("Skipping %s (%s)" % (path, id)) - if follow_imports == 'error': - if ancestor_for: - self.skipping_ancestor(id, path, ancestor_for) - else: - self.skipping_module(id, path) - path = None - manager.missing_modules.add(id) - raise ModuleNotFound + if not is_namespace_path(path): + # For non-stubs, look at options.follow_imports: + # - normal (default) -> fully analyze + # - silent -> analyze but silence errors + # - skip -> don't analyze, make the type Any + follow_imports = self.options.follow_imports + if (follow_imports != 'normal' + and not root_source # Honor top-level modules + and path.endswith('.py') # Stubs are always normal + and id != 'builtins'): # Builtins is always normal + if follow_imports == 'silent': + # Still import it, but silence non-blocker errors. + manager.log("Silencing %s (%s)" % (path, id)) + self.ignore_all = True + else: + # In 'error' mode, produce special error messages. + manager.log("Skipping %s (%s)" % (path, id)) + if follow_imports == 'error': + if ancestor_for: + self.skipping_ancestor(id, path, ancestor_for) + else: + self.skipping_module(id, path) + path = None + manager.missing_modules.add(id) + raise ModuleNotFound + else: + source = '' + self.source_hash = '' else: # Could not find a module. Typically the reason is a # misspelled module name, missing stub, module not in @@ -1788,6 +1857,7 @@ def parse_file(self) -> None: except (UnicodeDecodeError, DecodeError) as decodeerr: raise CompileError([ "mypy: can't decode file '{}': {}".format(self.path, str(decodeerr))]) + assert source is not None self.tree = manager.parse_file(self.id, self.xpath, source, self.ignore_all or self.options.ignore_errors) @@ -1873,6 +1943,7 @@ def semantic_analysis_apply_patches(self) -> None: patch_func() def type_check_first_pass(self) -> None: + assert self.tree is not None, "Internal error: method must be called on parsed file only" if self.options.semantic_analysis_only: return with self.wrap_context(): diff --git a/mypy/main.py b/mypy/main.py index ef30c02d7ef4..34ada0c42a5f 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -323,6 +323,8 @@ def add_invertible_flag(flag: str, parser.add_argument('--show-traceback', '--tb', action='store_true', help="show traceback on fatal error") parser.add_argument('--stats', action='store_true', dest='dump_type_stats', help="dump stats") + parser.add_argument('--namespace-packages', action='store_true', dest='namespace_packages', + help='Allow implicit namespace packages (PEP420)') parser.add_argument('--inferstats', action='store_true', dest='dump_inference_stats', help="dump type inference stats") parser.add_argument('--custom-typing', metavar='MODULE', dest='custom_typing_module', @@ -508,7 +510,8 @@ def add_invertible_flag(flag: str, .format(special_opts.package)) options.build_type = BuildType.MODULE lib_path = [os.getcwd()] + build.mypy_path() - targets = build.find_modules_recursive(special_opts.package, lib_path) + mod_discovery = build.ModuleDiscovery(lib_path, options.namespace_packages) + targets = mod_discovery.find_modules_recursive(special_opts.package) if not targets: fail("Can't find package '{}'".format(special_opts.package)) return targets, options diff --git a/mypy/options.py b/mypy/options.py index dd9bfe08c095..1e429aba945a 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -164,6 +164,9 @@ def __init__(self) -> None: # Use stub builtins fixtures to speed up tests self.use_builtins_fixtures = False + # Allow implicit namespace packages (PEP420) + self.namespace_packages = False + # -- experimental options -- self.shadow_file = None # type: Optional[Tuple[str, str]] self.show_column_numbers = False # type: bool diff --git a/mypy/semanal.py b/mypy/semanal.py index 52c45b4dc162..8ac22ef1805f 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -182,8 +182,6 @@ class SemanticAnalyzerPass2(NodeVisitor[None]): This is the second phase of semantic analysis. """ - # Library search paths - lib_path = None # type: List[str] # Module name space modules = None # type: Dict[str, MypyFile] # Global name space for current module @@ -229,13 +227,9 @@ class SemanticAnalyzerPass2(NodeVisitor[None]): def __init__(self, modules: Dict[str, MypyFile], missing_modules: Set[str], - lib_path: List[str], errors: Errors, + errors: Errors, plugin: Plugin) -> None: - """Construct semantic analyzer. - - Use lib_path to search for modules, and report analysis errors - using the Errors instance. - """ + """Construct semantic analyzer.""" self.locals = [None] self.imports = set() self.type = None @@ -244,7 +238,6 @@ def __init__(self, self.function_stack = [] self.block_depth = [0] self.loop_depth = 0 - self.lib_path = lib_path self.errors = errors self.modules = modules self.msg = MessageBuilder(errors, modules) diff --git a/mypy/stubgen.py b/mypy/stubgen.py index 707fb32f06d3..5a7e6c7bdade 100644 --- a/mypy/stubgen.py +++ b/mypy/stubgen.py @@ -156,7 +156,8 @@ def find_module_path_and_all(module: str, pyversion: Tuple[int, int], module_all = getattr(mod, '__all__', None) else: # Find module by going through search path. - module_path = mypy.build.find_module(module, ['.'] + search_path) + md = mypy.build.ModuleDiscovery(['.'] + search_path) + module_path = md.find_module(module) if not module_path: raise SystemExit( "Can't find module '{}' (consider using --search-path)".format(module)) diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py index b33dfba6405f..a6c800a78796 100644 --- a/mypy/test/testcheck.py +++ b/mypy/test/testcheck.py @@ -76,6 +76,7 @@ 'check-incomplete-fixture.test', 'check-custom-plugin.test', 'check-default-plugin.test', + 'check-namespaces.test', ] @@ -320,7 +321,8 @@ def parse_module(self, module_names = m.group(1) out = [] for module_name in module_names.split(' '): - path = build.find_module(module_name, [test_temp_dir]) + md = build.ModuleDiscovery([test_temp_dir], namespaces_allowed=False) + path = md.find_module(module_name) assert path is not None, "Can't find ad hoc case file" with open(path) as f: program_text = f.read() diff --git a/mypy/test/testdmypy.py b/mypy/test/testdmypy.py index 4be4ed8259d5..4bf3530fd19d 100644 --- a/mypy/test/testdmypy.py +++ b/mypy/test/testdmypy.py @@ -253,7 +253,8 @@ def parse_module(self, module_names = m.group(1) out = [] for module_name in module_names.split(' '): - path = build.find_module(module_name, [test_temp_dir]) + md = build.ModuleDiscovery([test_temp_dir]) + path = md.find_module(module_name) assert path is not None, "Can't find ad hoc case file" with open(path) as f: program_text = f.read() diff --git a/mypy/test/testgraph.py b/mypy/test/testgraph.py index dbbe4872aa75..f88a63692608 100644 --- a/mypy/test/testgraph.py +++ b/mypy/test/testgraph.py @@ -3,7 +3,7 @@ from typing import AbstractSet, Dict, Set, List from mypy.myunit import Suite, assert_equal -from mypy.build import BuildManager, State, BuildSourceSet +from mypy.build import BuildManager, State, BuildSourceSet, ModuleDiscovery from mypy.build import topsort, strongly_connected_components, sorted_components, order_ascc from mypy.version import __version__ from mypy.options import Options @@ -41,7 +41,6 @@ def _make_manager(self) -> BuildManager: options = Options() manager = BuildManager( data_dir='', - lib_path=[], ignore_prefix='', source_set=BuildSourceSet([]), reports=Reports('', {}), @@ -49,6 +48,7 @@ def _make_manager(self) -> BuildManager: version_id=__version__, plugin=Plugin(options), errors=errors, + module_discovery=ModuleDiscovery([]), ) return manager diff --git a/mypy/test/testmodulediscovery.py b/mypy/test/testmodulediscovery.py new file mode 100644 index 000000000000..4f09e9893813 --- /dev/null +++ b/mypy/test/testmodulediscovery.py @@ -0,0 +1,141 @@ +import os + +from unittest import mock, TestCase +from typing import List, Set + +from mypy.build import ModuleDiscovery, find_module_clear_caches +from mypy.myunit import Suite, assert_equal + + +class ModuleDiscoveryTestCase(Suite): + def set_up(self) -> None: + self.files = set() # type: Set[str] + + self._setup_mock_filesystem() + + def tear_down(self) -> None: + self._teardown_mock_filesystem() + find_module_clear_caches() + + def _list_dir(self, path: str) -> List[str]: + res = set() + + if not path.endswith(os.path.sep): + path = path + os.path.sep + + for item in self.files: + if item.startswith(path): + remnant = item.replace(path, '') + segments = remnant.split(os.path.sep) + if segments: + res.add(segments[0]) + + return list(res) + + def _is_file(self, path: str) -> bool: + return path in self.files + + def _is_dir(self, path: str) -> bool: + for item in self.files: + if not item.endswith('/'): + item += '/' + if item.startswith(path): + return True + return False + + def _setup_mock_filesystem(self) -> None: + self._listdir_patcher = mock.patch('os.listdir', side_effect=self._list_dir) + self._listdir_mock = self._listdir_patcher.start() + self._isfile_patcher = mock.patch('os.path.isfile', side_effect=self._is_file) + self._isfile_mock = self._isfile_patcher.start() + self._isdir_patcher = mock.patch('os.path.isdir', side_effect=self._is_dir) + self._isdir_mock = self._isdir_patcher.start() + + def _teardown_mock_filesystem(self) -> None: + self._listdir_patcher.stop() + self._isfile_patcher.stop() + self._isdir_patcher.stop() + + def test_module_vs_package(self) -> None: + self.files = { + os.path.join('dir1', 'mod.py'), + os.path.join('dir2', 'mod', '__init__.py'), + } + m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=False) + path = m.find_module('mod') + assert_equal(path, os.path.join('dir1', 'mod.py')) + + m = ModuleDiscovery(['dir2', 'dir1'], namespaces_allowed=False) + path = m.find_module('mod') + assert_equal(path, os.path.join('dir2', 'mod', '__init__.py')) + + def test_package_in_different_directories(self) -> None: + self.files = { + os.path.join('dir1', 'mod', '__init__.py'), + os.path.join('dir1', 'mod', 'a.py'), + os.path.join('dir2', 'mod', '__init__.py'), + os.path.join('dir2', 'mod', 'b.py'), + } + m = ModuleDiscovery(['./dir1', './dir2'], namespaces_allowed=False) + path = m.find_module('mod.a') + assert_equal(path, os.path.join('dir1', 'mod', 'a.py')) + + path = m.find_module('mod.b') + assert_equal(path, None) + + def test_package_with_stubs(self) -> None: + self.files = { + os.path.join('dir1', 'mod', '__init__.py'), + os.path.join('dir1', 'mod', 'a.py'), + os.path.join('dir2', 'mod', '__init__.pyi'), + os.path.join('dir2', 'mod', 'b.pyi'), + } + m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=False) + path = m.find_module('mod.a') + assert_equal(path, os.path.join('dir1', 'mod', 'a.py')) + + path = m.find_module('mod.b') + assert_equal(path, os.path.join('dir2', 'mod', 'b.pyi')) + + def test_namespaces(self) -> None: + self.files = { + os.path.join('dir1', 'mod', 'a.py'), + os.path.join('dir2', 'mod', 'b.py'), + } + m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) + path = m.find_module('mod.a') + assert_equal(path, os.path.join('dir1', 'mod', 'a.py')) + + path = m.find_module('mod.b') + assert_equal(path, os.path.join('dir2', 'mod', 'b.py')) + + def test_find_modules_recursive(self) -> None: + self.files = { + os.path.join('dir1', 'mod', '__init__.py'), + os.path.join('dir1', 'mod', 'a.py'), + os.path.join('dir2', 'mod', '__init__.pyi'), + os.path.join('dir2', 'mod', 'b.pyi'), + } + m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) + srcs = m.find_modules_recursive('mod') + assert_equal([s.module for s in srcs], ['mod', 'mod.a', 'mod.b']) + + def test_find_modules_recursive_with_namespace(self) -> None: + self.files = { + os.path.join('dir1', 'mod', 'a.py'), + os.path.join('dir2', 'mod', 'b.py'), + } + m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) + srcs = m.find_modules_recursive('mod') + assert_equal([s.module for s in srcs], ['mod.a', 'mod.b']) + + def test_find_modules_recursive_with_stubs(self) -> None: + self.files = { + os.path.join('dir1', 'mod', '__init__.py'), + os.path.join('dir1', 'mod', 'a.py'), + os.path.join('dir2', 'mod', '__init__.pyi'), + os.path.join('dir2', 'mod', 'a.pyi'), + } + m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) + srcs = m.find_modules_recursive('mod') + assert_equal([s.module for s in srcs], ['mod', 'mod.a']) diff --git a/runtests.py b/runtests.py index d4712bbfbabb..8f0903449fb2 100755 --- a/runtests.py +++ b/runtests.py @@ -232,6 +232,7 @@ def test_path(*names: str): 'testsolve', 'testsubtypes', 'testtypes', + 'testmodulediscovery', ) for f in find_files('mypy', prefix='test', suffix='.py'): diff --git a/test-data/unit/check-namespaces.test b/test-data/unit/check-namespaces.test new file mode 100644 index 000000000000..1c0f55c331fe --- /dev/null +++ b/test-data/unit/check-namespaces.test @@ -0,0 +1,87 @@ +-- Type checker test cases dealing with namespaces imports + +[case testAccessModuleInsideNamespace] +# flags: --namespace-packages +from ns import a +[file ns/a.py] +class A: pass +def f(a: A) -> None: pass + +[case testAccessModuleInsideNamespaceNoNamespacePackages] +from ns import a +[file ns/a.py] +class A: pass +def f(a: A) -> None: pass +[out] +main:1: error: Cannot find module named 'ns' +main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) + +[case testAccessPackageInsideNamespace] +# flags: --namespace-packages +from ns import a +[file ns/a/__init__.py] +class A: pass +def f(a: A) -> None: pass + +[case testAccessPackageInsideNamespaceLocatedInSeparateDirectories] +# flags: --config-file tmp/mypy.ini +from ns import a, b +[file mypy.ini] +[[mypy] +namespace_packages = True +mypy_path = ./tmp/dir1:./tmp/dir2 +[file dir1/ns/a/__init__.py] +class A: pass +def f(a: A) -> None: pass +[file dir2/ns/b.py] +class B: pass +def f(a: B) -> None: pass + +[case testConflictingPackageAndNamespaceFromImport] +# flags: --config-file tmp/mypy.ini +from pkg_or_ns import a +from pkg_or_ns import b # E: Module 'pkg_or_ns' has no attribute 'b' +[file mypy.ini] +[[mypy] +namespace_packages = True +mypy_path = ./tmp/dir:./tmp/other_dir +[file dir/pkg_or_ns/__init__.py] +[file dir/pkg_or_ns/a.py] +[file other_dir/pkg_or_ns/b.py] + +[case testConflictingPackageAndNamespaceImport] +# flags: --config-file tmp/mypy.ini +import pkg_or_ns.a +import pkg_or_ns.b +[file mypy.ini] +[[mypy] +namespace_packages = True +mypy_path = ./tmp/dir:./tmp/other_dir +[file dir/pkg_or_ns/__init__.py] +[file dir/pkg_or_ns/a.py] +[file other_dir/pkg_or_ns/b.py] +[out] +main:3: error: Cannot find module named 'pkg_or_ns.b' +main:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) + +[case testConflictingModuleAndNamespace] +# flags: --config-file tmp/mypy.ini +from mod_or_ns import a +from mod_or_ns import b # E: Module 'mod_or_ns' has no attribute 'b' +[file mypy.ini] +[[mypy] +namespace_packages = True +mypy_path = ./tmp/dir:./tmp/other_dir +[file dir/mod_or_ns.py] +a = None +[file other_dir/mod_or_ns/b.py] + +[case testeNamespaceInsidePackage] +# flags: --config-file tmp/mypy.ini +from pkg.ns import a +[file mypy.ini] +[[mypy] +namespace_packages = True +[file pkg/__init__.py] +[file pkg/ns/a.py] +[out] From 5c77c2d7dedb9f8dc226d41aa001c2902e15a244 Mon Sep 17 00:00:00 2001 From: Maxim Novikov Date: Wed, 22 Nov 2017 07:23:29 +0100 Subject: [PATCH 2/6] Added clarifying comments --- mypy/build.py | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 26fa07637d12..3bbd235c5ccf 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -19,6 +19,7 @@ import json import os.path import re +import enum import site import stat import sys @@ -799,19 +800,32 @@ def is_file(path: str) -> bool: return res -class ModuleType: - PACKAGE = 1 - MODULE = 2 - NAMESPACE = 3 +class ModuleType(enum.Enum): + package = 'package' + module = 'module' + namespace = 'namespace' class ImportContext: + """ + Describes module import context + + Do we already discovered implementation? + What kind of module we discovered? + """ def __init__(self) -> None: self.has_py = False # type: bool - self.type = None # type: Optional[int] + self.type = None # type: Optional[ModuleType] + # Paths can contain only one ".py" path, but multiple stubs self.paths = [] # type: List[str] - def maybe_add_path(self, path: str, type: int) -> None: + def maybe_add_path(self, path: str, type: ModuleType) -> None: + """ + Add path to import context. + Modifies self.paths in case if arguments satisfy import context state + """ + assert path.endswith((os.path.sep,) + tuple(PYTHON_EXTENSIONS)) + if self.type is not None and self.type != type: return None @@ -821,7 +835,7 @@ def maybe_add_path(self, path: str, type: int) -> None: if self.has_py and py_path: return None - if type == ModuleType.NAMESPACE: + if type == ModuleType.namespace: ok = self._verify_namespace(path) else: ok = self._verify_module(path) @@ -836,6 +850,8 @@ def maybe_add_path(self, path: str, type: int) -> None: self.paths.append(path) def _verify_module(self, path: str) -> bool: + # At this point we already know that that it's valid python path + # We only need to check file existence if not is_file(path): return False @@ -879,6 +895,10 @@ def find_module(self, id: str) -> Optional[str]: return None def find_modules_recursive(self, module: str) -> List[BuildSource]: + """ + Discover module and all it's children + Remove duplicates from discovered paths + """ hits = set() # type: Set[str] result = [] # type: List[BuildSource] for src in self._find_modules_recursive(module): @@ -922,6 +942,9 @@ def _collect_paths(self, paths: List[str], last_comp: str) -> List[str]: sepinit = '__init__' ctx = ImportContext() + # Detect modules in following order: package, module, namespace. + # First hit determines module type, consistency of paths to given type + # ensured in ImportContext for path_item in paths: if is_module_path(path_item): continue @@ -931,15 +954,15 @@ def _collect_paths(self, paths: List[str], last_comp: str) -> List[str]: for ext in PYTHON_EXTENSIONS: path = os.path.join(path_item, last_comp, sepinit + ext) - ctx.maybe_add_path(path, ModuleType.PACKAGE) + ctx.maybe_add_path(path, ModuleType.package) for ext in PYTHON_EXTENSIONS: path = os.path.join(path_item, last_comp + ext) - ctx.maybe_add_path(path, ModuleType.MODULE) + ctx.maybe_add_path(path, ModuleType.module) if self.namespaces_allowed: path = os.path.join(path_item, last_comp) - ctx.maybe_add_path(path + os.sep, ModuleType.NAMESPACE) + ctx.maybe_add_path(path + os.sep, ModuleType.namespace) return ctx.paths From c84a63c319b64e8a9ca10a951b5ed1624c73e82e Mon Sep 17 00:00:00 2001 From: Maxim Novikov Date: Thu, 30 Nov 2017 00:07:28 +0100 Subject: [PATCH 3/6] Prefer modules and packages to namespaces regardless of path order --- mypy/build.py | 6 +++++- test-data/unit/check-namespaces.test | 29 +++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 3bbd235c5ccf..75ae1683bb8e 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -960,7 +960,11 @@ def _collect_paths(self, paths: List[str], last_comp: str) -> List[str]: path = os.path.join(path_item, last_comp + ext) ctx.maybe_add_path(path, ModuleType.module) - if self.namespaces_allowed: + if self.namespaces_allowed and not ctx.paths: + for path_item in paths: + if is_pkg_path(path_item): + path_item = dirname(path_item) + path = os.path.join(path_item, last_comp) ctx.maybe_add_path(path + os.sep, ModuleType.namespace) diff --git a/test-data/unit/check-namespaces.test b/test-data/unit/check-namespaces.test index 1c0f55c331fe..a7ce0990b18c 100644 --- a/test-data/unit/check-namespaces.test +++ b/test-data/unit/check-namespaces.test @@ -64,6 +64,21 @@ mypy_path = ./tmp/dir:./tmp/other_dir main:3: error: Cannot find module named 'pkg_or_ns.b' main:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) +[case testConflictingPackageAndNamespaceImportPackageLaterInPath] +# flags: --config-file tmp/mypy.ini +import pkg_or_ns.a +import pkg_or_ns.b +[file mypy.ini] +[[mypy] +namespace_packages = True +mypy_path = ./tmp/other_dir:./tmp/dir +[file dir/pkg_or_ns/__init__.py] +[file dir/pkg_or_ns/a.py] +[file other_dir/pkg_or_ns/b.py] +[out] +main:3: error: Cannot find module named 'pkg_or_ns.b' +main:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) + [case testConflictingModuleAndNamespace] # flags: --config-file tmp/mypy.ini from mod_or_ns import a @@ -76,7 +91,19 @@ mypy_path = ./tmp/dir:./tmp/other_dir a = None [file other_dir/mod_or_ns/b.py] -[case testeNamespaceInsidePackage] +[case testConflictingModuleAndNamespaceModuleLaterInPath] +# flags: --config-file tmp/mypy.ini +from mod_or_ns import a +from mod_or_ns import b # E: Module 'mod_or_ns' has no attribute 'b' +[file mypy.ini] +[[mypy] +namespace_packages = True +mypy_path = ./tmp/other_dir:./tmp/dir +[file dir/mod_or_ns.py] +a = None +[file other_dir/mod_or_ns/b.py] + +[case testNamespaceInsidePackage] # flags: --config-file tmp/mypy.ini from pkg.ns import a [file mypy.ini] From ea64ec0817c7e8f1b61254a9cfd83c7e4909e0ac Mon Sep 17 00:00:00 2001 From: Maxim Novikov Date: Thu, 30 Nov 2017 15:56:33 +0100 Subject: [PATCH 4/6] Fix lib_path discovery when using -p with namespaces --- mypy/build.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 75ae1683bb8e..e5bde77bfe9d 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -174,7 +174,9 @@ def build(sources: List[BuildSource], for source in sources: if source.path: # Include directory of the program file in the module search path. - dir = remove_cwd_prefix_from_path(dirname(source.path)) + dir = remove_cwd_prefix_from_path( + dirname(source.path), namespaces_allowed=options.namespace_packages + ) if dir not in lib_path: lib_path.insert(0, dir) @@ -711,21 +713,23 @@ def stats_summary(self) -> Mapping[str, object]: return self.stats -def remove_cwd_prefix_from_path(p: str) -> str: +def remove_cwd_prefix_from_path(p: str, namespaces_allowed: bool) -> str: """Remove current working directory prefix from p, if present. Also crawl up until a directory without __init__.py is found. If the result would be empty, return '.' instead. """ + def is_pkg(p: str) -> bool: + return (os.path.isfile(os.path.join(p, '__init__.py')) + or os.path.isfile(os.path.join(p, '__init__.pyi'))) + cur = os.getcwd() # Add separator to the end of the path, unless one is already present. if basename(cur) != '': cur += os.sep # Compute root path. - while (p and - (os.path.isfile(os.path.join(p, '__init__.py')) or - os.path.isfile(os.path.join(p, '__init__.pyi')))): + while (p and (namespaces_allowed or is_pkg(p))): dir, base = os.path.split(p) if not base: break From d488c2f4b94bb3761831e6a609c6799396bc179c Mon Sep 17 00:00:00 2001 From: Maxim Novikov Date: Wed, 13 Dec 2017 08:59:07 +0100 Subject: [PATCH 5/6] Review fixes --- mypy/build.py | 228 +++++++++++++++---------------- mypy/stubgen.py | 5 +- mypy/test/testcheck.py | 8 +- mypy/test/testdmypy.py | 8 +- mypy/test/testmodulediscovery.py | 78 ++++++----- runtests.py | 4 +- 6 files changed, 166 insertions(+), 165 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index e5bde77bfe9d..87d2d1855563 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -85,11 +85,28 @@ def __init__(self, manager: 'BuildManager', graph: Graph) -> None: class BuildSource: - def __init__(self, path: Optional[str], module: Optional[str], - text: Optional[str]) -> None: - self.path = path + def __init__(self, paths: Optional[Union[str, List[str]]], module: Optional[str], + text: Optional[str], type: Optional['ModuleType'] = None) -> None: + + if isinstance(paths, list): + self.paths = paths + elif paths is None: + self.paths = [] + else: + self.paths = [paths] + self.module = module or '__main__' self.text = text + self.type = type + + def __repr__(self) -> str: + return 'BuildSource(%s)' % self.module + + @property + def path(self) -> Optional[str]: + if self.paths: + return self.paths[0] + return None def __repr__(self) -> str: return '' % (self.path, @@ -634,7 +651,7 @@ def correct_rel_imp(imp: Union[ImportFrom, ImportAll]) -> str: return res - def find_module(self, id: str) -> Optional[str]: + def find_module(self, id: str) -> Optional[BuildSource]: return self.module_discovery.find_module(id) def is_module(self, id: str) -> bool: @@ -837,12 +854,13 @@ def maybe_add_path(self, path: str, type: ModuleType) -> None: # But module can have multiple stubs py_path = path.endswith('.py') if self.has_py and py_path: + # Found more than one implementation for module, skip it return None if type == ModuleType.namespace: - ok = self._verify_namespace(path) + ok = os.path.isdir(path) else: - ok = self._verify_module(path) + ok = is_file(path) if not ok: return None @@ -853,50 +871,20 @@ def maybe_add_path(self, path: str, type: ModuleType) -> None: self.type = type self.paths.append(path) - def _verify_module(self, path: str) -> bool: - # At this point we already know that that it's valid python path - # We only need to check file existence - if not is_file(path): - return False - - return True - - def _verify_namespace(self, path: str) -> bool: - if os.path.isdir(path): - files = set(list_dir(path) or []) # type: Set[str] - if not ('__init__.py' in files or '__init__.pyi' in files): - return True - - return False - - -def is_pkg_path(path: str) -> bool: - return path.endswith(('__init__.py', '__init__.pyi')) - - -def is_module_path(path: str) -> bool: - return not is_pkg_path(path) and path.endswith(('.py', '.pyi')) - - -def is_namespace_path(path: str) -> bool: - return path.endswith(os.path.sep) - class ModuleDiscovery: def __init__(self, lib_path: Iterable[str], namespaces_allowed: bool = False) -> None: - self.lib_path = tuple(os.path.normpath(p) for p in lib_path) # type: Tuple[str, ...] + self.lib_path = [os.path.normpath(p) for p in lib_path] # type: List[str] self.namespaces_allowed = namespaces_allowed - self._find_module_cache = {} # type: Dict[str, List[str]] + self._find_module_cache = {} # type: Dict[str, Optional[BuildSource]] - def find_module(self, id: str) -> Optional[str]: - paths = self._find_module(id) - if paths: - return paths[0] - else: - return None + def find_module(self, id: str) -> Optional[BuildSource]: + if id not in self._find_module_cache: + self._find_module_cache[id] = self._find_module(id) + return self._find_module_cache[id] def find_modules_recursive(self, module: str) -> List[BuildSource]: """ @@ -911,27 +899,32 @@ def find_modules_recursive(self, module: str) -> List[BuildSource]: result.append(src) return result + def _iter_module_dir_paths(self, source: Optional[BuildSource]) -> Iterator[str]: + if not (source and source.paths): + return + + if source.type == ModuleType.package: + if source.path: + yield dirname(source.path) + elif source.type == ModuleType.namespace: + yield from source.paths + def _find_modules_recursive(self, module: str) -> List[BuildSource]: - module_paths = self._find_module(module) + src = self.find_module(module) - srcs = [] # type: List[BuildSource] - for path in module_paths: - if is_module_path(path) or is_pkg_path(path): - srcs.append(BuildSource(path, module, None)) + if not src: + return [] - if is_pkg_path(path): - path = dirname(path) - for submodule in self._find_submodules(module, path): - srcs += self._find_modules_recursive(submodule) - elif is_namespace_path(path): - for submodule in self._find_submodules(module, path): - srcs += self._find_modules_recursive(submodule) + srcs = [src] # type: List[BuildSource] + for path in self._iter_module_dir_paths(src): + for submodule in self._find_submodules(module, path): + srcs += self._find_modules_recursive(submodule) return srcs def _find_submodules(self, module: str, path: str) -> Iterator[str]: for item in list_dir(path) or []: - if item == '__init__.py' or item == '__init__.pyi': + if item.startswith(('__', '.')): continue if item.endswith(tuple(PYTHON_EXTENSIONS)): @@ -939,53 +932,46 @@ def _find_submodules(self, module: str, path: str) -> Iterator[str]: yield module + '.' + item - def _collect_paths(self, paths: List[str], last_comp: str) -> List[str]: - """ - Collect all available module paths - """ + def _find_module(self, id: str) -> Optional[BuildSource]: + components = id.split('.') + + if len(components) > 1: + parent_id = '.'.join(components[:-1]) + parent = self.find_module(parent_id) + if not parent: + return None + search_paths = list(self._iter_module_dir_paths(parent)) + else: + search_paths = self.lib_path + + leaf_module_name = components[-1] sepinit = '__init__' - ctx = ImportContext() # Detect modules in following order: package, module, namespace. # First hit determines module type, consistency of paths to given type # ensured in ImportContext - for path_item in paths: - if is_module_path(path_item): - continue - - if is_pkg_path(path_item): - path_item = dirname(path_item) - + for path in search_paths: for ext in PYTHON_EXTENSIONS: - path = os.path.join(path_item, last_comp, sepinit + ext) - ctx.maybe_add_path(path, ModuleType.package) + candidate_path = os.path.join(path, leaf_module_name, sepinit + ext) + if is_file(candidate_path): + return BuildSource(candidate_path, id, None, type=ModuleType.package) for ext in PYTHON_EXTENSIONS: - path = os.path.join(path_item, last_comp + ext) - ctx.maybe_add_path(path, ModuleType.module) - - if self.namespaces_allowed and not ctx.paths: - for path_item in paths: - if is_pkg_path(path_item): - path_item = dirname(path_item) + candidate_path = os.path.join(path, leaf_module_name + ext) + if is_file(candidate_path): + return BuildSource(candidate_path, id, None, type=ModuleType.module) - path = os.path.join(path_item, last_comp) - ctx.maybe_add_path(path + os.sep, ModuleType.namespace) - - return ctx.paths - - def _find_module(self, id: str) -> List[str]: - if id not in self._find_module_cache: - components = id.split('.') - parent_paths = list(self.lib_path) + if self.namespaces_allowed: + namespace_paths = [] + for path in search_paths: + candidate_path = os.path.join(path, leaf_module_name) + if os.path.isdir(candidate_path): + namespace_paths.append(candidate_path) - if len(components) > 1: - parent_id = '.'.join(components[:-1]) - parent_paths = self._find_module(parent_id) + if namespace_paths: + return BuildSource(namespace_paths, id, None, type=ModuleType.namespace) - last_comp = components[-1] - self._find_module_cache[id] = self._collect_paths(parent_paths, last_comp) - return self._find_module_cache[id] + return None def read_with_python_encoding(path: str, pyversion: Tuple[int, int]) -> Tuple[str, str]: @@ -1617,36 +1603,36 @@ def __init__(self, # difference and just assume 'builtins' everywhere, # which simplifies code. file_id = '__builtin__' - path = manager.find_module(file_id) - if path: - if not is_namespace_path(path): - # For non-stubs, look at options.follow_imports: - # - normal (default) -> fully analyze - # - silent -> analyze but silence errors - # - skip -> don't analyze, make the type Any - follow_imports = self.options.follow_imports - if (follow_imports != 'normal' - and not root_source # Honor top-level modules - and path.endswith('.py') # Stubs are always normal - and id != 'builtins'): # Builtins is always normal - if follow_imports == 'silent': - # Still import it, but silence non-blocker errors. - manager.log("Silencing %s (%s)" % (path, id)) - self.ignore_all = True - else: - # In 'error' mode, produce special error messages. - manager.log("Skipping %s (%s)" % (path, id)) - if follow_imports == 'error': - if ancestor_for: - self.skipping_ancestor(id, path, ancestor_for) - else: - self.skipping_module(id, path) - path = None - manager.missing_modules.add(id) - raise ModuleNotFound - else: - source = '' - self.source_hash = '' + src = manager.find_module(file_id) + if src and src.type == ModuleType.namespace: + source = '' + self.source_hash = '' + elif src and src.path: + path = src.path + # For non-stubs, look at options.follow_imports: + # - normal (default) -> fully analyze + # - silent -> analyze but silence errors + # - skip -> don't analyze, make the type Any + follow_imports = self.options.follow_imports + if (follow_imports != 'normal' + and not root_source # Honor top-level modules + and path.endswith('.py') # Stubs are always normal + and id != 'builtins'): # Builtins is always normal + if follow_imports == 'silent': + # Still import it, but silence non-blocker errors. + manager.log("Silencing %s (%s)" % (path, id)) + self.ignore_all = True + else: + # In 'error' mode, produce special error messages. + manager.log("Skipping %s (%s)" % (path, id)) + if follow_imports == 'error': + if ancestor_for: + self.skipping_ancestor(id, path, ancestor_for) + else: + self.skipping_module(id, path) + path = None + manager.missing_modules.add(id) + raise ModuleNotFound else: # Could not find a module. Typically the reason is a # misspelled module name, missing stub, module not in diff --git a/mypy/stubgen.py b/mypy/stubgen.py index 5a7e6c7bdade..e5db9824d8bb 100644 --- a/mypy/stubgen.py +++ b/mypy/stubgen.py @@ -157,10 +157,11 @@ def find_module_path_and_all(module: str, pyversion: Tuple[int, int], else: # Find module by going through search path. md = mypy.build.ModuleDiscovery(['.'] + search_path) - module_path = md.find_module(module) - if not module_path: + src = md.find_module(module) + if not (src and src.path): raise SystemExit( "Can't find module '{}' (consider using --search-path)".format(module)) + module_path = src.path module_all = None return module_path, module_all diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py index a6c800a78796..ea53643d7c82 100644 --- a/mypy/test/testcheck.py +++ b/mypy/test/testcheck.py @@ -322,11 +322,11 @@ def parse_module(self, out = [] for module_name in module_names.split(' '): md = build.ModuleDiscovery([test_temp_dir], namespaces_allowed=False) - path = md.find_module(module_name) - assert path is not None, "Can't find ad hoc case file" - with open(path) as f: + src = md.find_module(module_name) + assert src is not None and src.path is not None, "Can't find ad hoc case file" + with open(src.path) as f: program_text = f.read() - out.append((module_name, path, program_text)) + out.append((module_name, src.path, program_text)) return out else: return [('__main__', 'main', program_text)] diff --git a/mypy/test/testdmypy.py b/mypy/test/testdmypy.py index 4bf3530fd19d..af27cd13e9cf 100644 --- a/mypy/test/testdmypy.py +++ b/mypy/test/testdmypy.py @@ -254,11 +254,11 @@ def parse_module(self, out = [] for module_name in module_names.split(' '): md = build.ModuleDiscovery([test_temp_dir]) - path = md.find_module(module_name) - assert path is not None, "Can't find ad hoc case file" - with open(path) as f: + src = md.find_module(module_name) + assert src is not None and src.path is not None, "Can't find ad hoc case file" + with open(src.path) as f: program_text = f.read() - out.append((module_name, path, program_text)) + out.append((module_name, src.path, program_text)) return out else: return [('__main__', 'main', program_text)] diff --git a/mypy/test/testmodulediscovery.py b/mypy/test/testmodulediscovery.py index 4f09e9893813..d2de7009fb34 100644 --- a/mypy/test/testmodulediscovery.py +++ b/mypy/test/testmodulediscovery.py @@ -1,24 +1,22 @@ import os -from unittest import mock, TestCase -from typing import List, Set +from unittest import TestCase, mock +from typing import List, Set, Union from mypy.build import ModuleDiscovery, find_module_clear_caches -from mypy.myunit import Suite, assert_equal -class ModuleDiscoveryTestCase(Suite): - def set_up(self) -> None: - self.files = set() # type: Set[str] - +class TestModuleDiscovery(TestCase): + def setUp(self) -> None: + self.files = set() # type: Union[Set[str], List[str]] self._setup_mock_filesystem() - def tear_down(self) -> None: + def tearDown(self) -> None: self._teardown_mock_filesystem() find_module_clear_caches() def _list_dir(self, path: str) -> List[str]: - res = set() + res = [] if not path.endswith(os.path.sep): path = path + os.path.sep @@ -28,17 +26,17 @@ def _list_dir(self, path: str) -> List[str]: remnant = item.replace(path, '') segments = remnant.split(os.path.sep) if segments: - res.add(segments[0]) + res.append(segments[0]) - return list(res) + return res def _is_file(self, path: str) -> bool: return path in self.files def _is_dir(self, path: str) -> bool: for item in self.files: - if not item.endswith('/'): - item += '/' + if not item.endswith(os.path.sep): + item += os.path.sep if item.startswith(path): return True return False @@ -62,12 +60,24 @@ def test_module_vs_package(self) -> None: os.path.join('dir2', 'mod', '__init__.py'), } m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=False) - path = m.find_module('mod') - assert_equal(path, os.path.join('dir1', 'mod.py')) + src = m.find_module('mod') + assert src is not None + assert src.path == os.path.join('dir1', 'mod.py') m = ModuleDiscovery(['dir2', 'dir1'], namespaces_allowed=False) - path = m.find_module('mod') - assert_equal(path, os.path.join('dir2', 'mod', '__init__.py')) + src = m.find_module('mod') + assert src is not None + assert src.path == os.path.join('dir2', 'mod', '__init__.py') + + def test_stubs_priority_module(self) -> None: + self.files = [ + os.path.join('dir1', 'mod.py'), + os.path.join('dir1', 'mod.pyi'), + ] + m = ModuleDiscovery(['dir1'], namespaces_allowed=False) + src = m.find_module('mod') + assert src is not None + assert src.path == os.path.join('dir1', 'mod.pyi') def test_package_in_different_directories(self) -> None: self.files = { @@ -77,11 +87,12 @@ def test_package_in_different_directories(self) -> None: os.path.join('dir2', 'mod', 'b.py'), } m = ModuleDiscovery(['./dir1', './dir2'], namespaces_allowed=False) - path = m.find_module('mod.a') - assert_equal(path, os.path.join('dir1', 'mod', 'a.py')) + src = m.find_module('mod.a') + assert src is not None + assert src.path == os.path.join('dir1', 'mod', 'a.py') - path = m.find_module('mod.b') - assert_equal(path, None) + src = m.find_module('mod.b') + assert src is None def test_package_with_stubs(self) -> None: self.files = { @@ -91,11 +102,12 @@ def test_package_with_stubs(self) -> None: os.path.join('dir2', 'mod', 'b.pyi'), } m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=False) - path = m.find_module('mod.a') - assert_equal(path, os.path.join('dir1', 'mod', 'a.py')) + src = m.find_module('mod.a') + assert src is not None + assert src.path == os.path.join('dir1', 'mod', 'a.py') - path = m.find_module('mod.b') - assert_equal(path, os.path.join('dir2', 'mod', 'b.pyi')) + src = m.find_module('mod.b') + assert src is None def test_namespaces(self) -> None: self.files = { @@ -103,11 +115,13 @@ def test_namespaces(self) -> None: os.path.join('dir2', 'mod', 'b.py'), } m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) - path = m.find_module('mod.a') - assert_equal(path, os.path.join('dir1', 'mod', 'a.py')) + src = m.find_module('mod.a') + assert src is not None + assert src.path == os.path.join('dir1', 'mod', 'a.py') - path = m.find_module('mod.b') - assert_equal(path, os.path.join('dir2', 'mod', 'b.py')) + src = m.find_module('mod.b') + assert src is not None + assert src.path == os.path.join('dir2', 'mod', 'b.py') def test_find_modules_recursive(self) -> None: self.files = { @@ -118,7 +132,7 @@ def test_find_modules_recursive(self) -> None: } m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) srcs = m.find_modules_recursive('mod') - assert_equal([s.module for s in srcs], ['mod', 'mod.a', 'mod.b']) + assert [s.module for s in srcs] == ['mod', 'mod.a'] def test_find_modules_recursive_with_namespace(self) -> None: self.files = { @@ -127,7 +141,7 @@ def test_find_modules_recursive_with_namespace(self) -> None: } m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) srcs = m.find_modules_recursive('mod') - assert_equal([s.module for s in srcs], ['mod.a', 'mod.b']) + assert [s.module for s in srcs] == ['mod', 'mod.a', 'mod.b'] def test_find_modules_recursive_with_stubs(self) -> None: self.files = { @@ -138,4 +152,4 @@ def test_find_modules_recursive_with_stubs(self) -> None: } m = ModuleDiscovery(['dir1', 'dir2'], namespaces_allowed=True) srcs = m.find_modules_recursive('mod') - assert_equal([s.module for s in srcs], ['mod', 'mod.a']) + assert [s.module for s in srcs] == ['mod', 'mod.a'] diff --git a/runtests.py b/runtests.py index 8f0903449fb2..a199cc2b67d1 100755 --- a/runtests.py +++ b/runtests.py @@ -213,7 +213,8 @@ def test_path(*names: str): 'testtransform', 'testtypegen', 'testparse', - 'testsemanal' + 'testsemanal', + 'testmodulediscovery', ) SLOW_FILES = test_path( @@ -232,7 +233,6 @@ def test_path(*names: str): 'testsolve', 'testsubtypes', 'testtypes', - 'testmodulediscovery', ) for f in find_files('mypy', prefix='test', suffix='.py'): From b6b0e9a1969db8f5ea0f926357f6f2b13bf36f66 Mon Sep 17 00:00:00 2001 From: Maxim Novikov Date: Wed, 13 Dec 2017 09:26:09 +0100 Subject: [PATCH 6/6] merger --- mypy/build.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 87d2d1855563..23a04586939a 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -99,9 +99,6 @@ def __init__(self, paths: Optional[Union[str, List[str]]], module: Optional[str] self.text = text self.type = type - def __repr__(self) -> str: - return 'BuildSource(%s)' % self.module - @property def path(self) -> Optional[str]: if self.paths: @@ -771,7 +768,7 @@ def is_pkg(p: str) -> bool: # of os.stat() calls is quickly more expensive than caching the # os.listdir() outcome, and the advantage of the latter is that it # gives us the case-correct filename on Windows and Mac. -find_module_listdir_cache = {} # type: Dict[str, Optional[List[str]]] +find_module_listdir_cache = {} # type: Dict[str, Set[str]] # Cache for is_file() find_module_is_file_cache = {} # type: Dict[str, bool] @@ -787,7 +784,7 @@ def find_module_clear_caches() -> None: find_module_isdir_cache.clear() -def list_dir(path: str) -> Optional[List[str]]: +def list_dir(path: str) -> Set[str]: """Return a cached directory listing. Returns None if the path doesn't exist or isn't a directory. @@ -797,7 +794,7 @@ def list_dir(path: str) -> Optional[List[str]]: try: res = os.listdir(path) except OSError: - res = None + res = set() find_module_listdir_cache[path] = res return res @@ -816,7 +813,7 @@ def is_file(path: str) -> bool: res = False else: names = list_dir(head) - res = names is not None and tail in names and os.path.isfile(path) + res = tail in names and os.path.isfile(path) find_module_is_file_cache[path] = res return res @@ -879,12 +876,12 @@ def __init__(self, self.lib_path = [os.path.normpath(p) for p in lib_path] # type: List[str] self.namespaces_allowed = namespaces_allowed - self._find_module_cache = {} # type: Dict[str, Optional[BuildSource]] + self.find_module_cache = {} # type: Dict[str, Optional[BuildSource]] def find_module(self, id: str) -> Optional[BuildSource]: - if id not in self._find_module_cache: - self._find_module_cache[id] = self._find_module(id) - return self._find_module_cache[id] + if id not in self.find_module_cache: + self.find_module_cache[id] = self._find_module(id) + return self.find_module_cache[id] def find_modules_recursive(self, module: str) -> List[BuildSource]: """ @@ -923,7 +920,7 @@ def _find_modules_recursive(self, module: str) -> List[BuildSource]: return srcs def _find_submodules(self, module: str, path: str) -> Iterator[str]: - for item in list_dir(path) or []: + for item in list_dir(path): if item.startswith(('__', '.')): continue @@ -2072,7 +2069,7 @@ def dispatch(sources: List[BuildSource], manager: BuildManager) -> Graph: stubs_found=sum(g.path is not None and g.path.endswith('.pyi') for g in graph.values()), graph_load_time=(t1 - t0), - fm_cache_size=len(find_module_cache), + fm_cache_size=len(manager.module_discovery.find_module_cache), fm_dir_cache_size=len(find_module_dir_cache), fm_listdir_cache_size=len(find_module_listdir_cache), fm_is_file_cache_size=len(find_module_is_file_cache),