From 434bb7fed5a5fb1cc5d37642fee6af2c29f4840f Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 11:39:11 +0000 Subject: [PATCH 1/8] Cleanup and update docs --- mypy/dmypy_server.py | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 57955fd08821c..1c81ff993f466 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -534,7 +534,6 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L orig_modules = list(graph.keys()) - # TODO: Are the differences from fine_grained_increment(), necessary? self.update_sources(sources) changed_paths = self.fswatcher.find_changed() manager.search_paths = compute_search_paths(sources, manager.options, manager.data_dir) @@ -546,13 +545,13 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L # Find changed modules reachable from roots (or in roots) already in graph. seen = set() # type: Set[str] - changed, removed, new_files = self.follow_imports( + changed, new_files = self.find_reachable_changed_modules( sources, graph, seen, changed_paths, sources_set ) sources.extend(new_files) # Process changes directly reachable from roots. - messages = fine_grained_manager.update(changed, removed) + messages = fine_grained_manager.update(changed, []) # Follow deps from changed modules (still within graph). worklist = changed[:] @@ -561,13 +560,12 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L if module[0] not in graph: continue sources2 = self.direct_imports(module, graph) - changed, removed, new_files = self.follow_imports( + changed, new_files = self.find_reachable_changed_modules( sources2, graph, seen, changed_paths, sources_set ) sources.extend(new_files) self.update_sources(new_files) - messages = fine_grained_manager.update(changed, removed) - # TODO: Removed? + messages = fine_grained_manager.update(changed, []) worklist.extend(changed) t2 = time.time() @@ -631,28 +629,32 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L return messages - def follow_imports(self, - sources: List[BuildSource], - graph: mypy.build.Graph, - seen: Set[str], - changed_paths: AbstractSet[str], - sources_set: Set[str]) -> Tuple[List[Tuple[str, str]], - List[Tuple[str, str]], - List[BuildSource]]: - """Follow imports within graph from given sources. + def find_reachable_changed_modules( + self, + roots: List[BuildSource], + graph: mypy.build.Graph, + seen: Set[str], + changed_paths: AbstractSet[str], + sources_set: Set[str]) -> Tuple[List[Tuple[str, str]], + List[BuildSource]]: + """Follow imports within graph from given sources until hitting changed modules. + + If we find a changed module, we can't continue following imports as the imports + may have changed. Args: - sources: roots of modules to search + roots: modules where to start search from graph: module graph to use for the search seen: modules we've seen before that won't be visited (mutated here!) changed_paths: which paths have changed (stop search here and return any found) sources_set: set of sources (TODO: relationship with seen) - Return (reachable changed modules, removed modules, updated file list). + Return (encountered reachable changed modules, + unchanged files not in sources_set traversed). """ changed = [] new_files = [] - worklist = sources[:] + worklist = roots[:] seen.update(source.module for source in worklist) while worklist: nxt = worklist.pop() @@ -669,7 +671,7 @@ def follow_imports(self, seen.add(dep) worklist.append(BuildSource(graph[dep].path, graph[dep].id)) - return changed, [], new_files + return changed, new_files def direct_imports(self, module: Tuple[str, str], From da78ed0ee5fdc5296bd2df47a1089418eda14f34 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 11:42:23 +0000 Subject: [PATCH 2/8] Merge seen and sources_set --- mypy/dmypy_server.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 1c81ff993f466..d85bb9ca07148 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -541,12 +541,11 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L t1 = time.time() manager.log("fine-grained increment: find_changed: {:.3f}s".format(t1 - t0)) - sources_set = {source.module for source in sources} + seen = {source.module for source in sources} # Find changed modules reachable from roots (or in roots) already in graph. - seen = set() # type: Set[str] changed, new_files = self.find_reachable_changed_modules( - sources, graph, seen, changed_paths, sources_set + sources, graph, seen, changed_paths ) sources.extend(new_files) @@ -561,9 +560,8 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L continue sources2 = self.direct_imports(module, graph) changed, new_files = self.find_reachable_changed_modules( - sources2, graph, seen, changed_paths, sources_set + sources2, graph, seen, changed_paths ) - sources.extend(new_files) self.update_sources(new_files) messages = fine_grained_manager.update(changed, []) worklist.extend(changed) @@ -634,9 +632,8 @@ def find_reachable_changed_modules( roots: List[BuildSource], graph: mypy.build.Graph, seen: Set[str], - changed_paths: AbstractSet[str], - sources_set: Set[str]) -> Tuple[List[Tuple[str, str]], - List[BuildSource]]: + changed_paths: AbstractSet[str]) -> Tuple[List[Tuple[str, str]], + List[BuildSource]]: """Follow imports within graph from given sources until hitting changed modules. If we find a changed module, we can't continue following imports as the imports @@ -647,7 +644,6 @@ def find_reachable_changed_modules( graph: module graph to use for the search seen: modules we've seen before that won't be visited (mutated here!) changed_paths: which paths have changed (stop search here and return any found) - sources_set: set of sources (TODO: relationship with seen) Return (encountered reachable changed modules, unchanged files not in sources_set traversed). @@ -658,8 +654,8 @@ def find_reachable_changed_modules( seen.update(source.module for source in worklist) while worklist: nxt = worklist.pop() - if nxt.module not in sources_set: - sources_set.add(nxt.module) + if nxt.module not in seen: + seen.add(nxt.module) new_files.append(nxt) if nxt.path in changed_paths: assert nxt.path is not None # TODO From 9f36c3323f9351fcf1d5e08a15a16c053414b506 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 13:35:10 +0000 Subject: [PATCH 3/8] Minor simplification --- mypy/dmypy_server.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index d85bb9ca07148..7419686b3837b 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -579,7 +579,7 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L seen_suppressed = set() # type: Set[str] while True: # TODO: Merge seen and seen_suppressed? - new_unsuppressed, seen_suppressed = self.find_added_suppressed( + new_unsuppressed = self.find_added_suppressed( graph, seen_suppressed, manager.search_paths ) if not new_unsuppressed: @@ -642,7 +642,7 @@ def find_reachable_changed_modules( Args: roots: modules where to start search from graph: module graph to use for the search - seen: modules we've seen before that won't be visited (mutated here!) + seen: modules we've seen before that won't be visited (mutated here!!) changed_paths: which paths have changed (stop search here and return any found) Return (encountered reachable changed modules, @@ -680,8 +680,14 @@ def direct_imports(self, def find_added_suppressed(self, graph: mypy.build.Graph, seen: Set[str], - search_paths: SearchPaths) -> Tuple[List[Tuple[str, str]], Set[str]]: - """Find suppressed modules that have been added (and not included in seen).""" + search_paths: SearchPaths) -> List[Tuple[str, str]]: + """Find suppressed modules that have been added (and not included in seen). + + Args: + seen: reachable modules we've seen before (mutated here!!) + + Return suppressed, added modules. + """ all_suppressed = set() for module, state in graph.items(): all_suppressed |= state.suppressed_set @@ -703,7 +709,7 @@ def find_added_suppressed(self, found.append((module, result)) seen.add(module) - return found, seen + return found def increment_output(self, messages: List[str], From e40c94af3001f0ab8c4a655affa9b94374885fca Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 13:36:13 +0000 Subject: [PATCH 4/8] Simplify more --- mypy/dmypy_server.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 7419686b3837b..07db90df10528 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -576,12 +576,8 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L # There may be new files that became available, currently treated as # suppressed imports. Process them. - seen_suppressed = set() # type: Set[str] while True: - # TODO: Merge seen and seen_suppressed? - new_unsuppressed = self.find_added_suppressed( - graph, seen_suppressed, manager.search_paths - ) + new_unsuppressed = self.find_added_suppressed(graph, seen, manager.search_paths) if not new_unsuppressed: break new_files = [BuildSource(mod[1], mod[0]) for mod in new_unsuppressed] @@ -600,7 +596,7 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L for module_id in orig_modules: if module_id not in graph: continue - if module_id not in seen and module_id not in seen_suppressed: + if module_id not in seen: module_path = graph[module_id].path assert module_path is not None to_delete.append((module_id, module_path)) From 82384acec39d7327efd8cd2ff362cd64505d7957 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 13:39:33 +0000 Subject: [PATCH 5/8] Remove TODO --- mypy/dmypy_server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 07db90df10528..4dc35ac6320e6 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -693,7 +693,6 @@ def find_added_suppressed(self, all_suppressed = {module for module in all_suppressed if module not in graph} # TODO: Namespace packages - # TODO: Handle seen? finder = FindModuleCache(search_paths, self.fscache, self.options) From 09f34a193f46fd62626b8492fa2c7af97a6f1709 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 16:22:19 +0000 Subject: [PATCH 6/8] Fix refresh_suppressed_submodules when target is cached We need to ensure fine-grained deps are loaded and read the AST for a cached module in case it's missing. Reading the AST may also generate errors, so we must be able to propagate them. --- mypy/dmypy_server.py | 23 +++++++++++++---- mypy/server/update.py | 25 ++++++++++++++++--- .../unit/fine-grained-follow-imports.test | 19 ++++++++++++++ 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 4dc35ac6320e6..c7f499ec9944b 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -568,9 +568,15 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L t2 = time.time() - for module_id, state in graph.items(): - refresh_suppressed_submodules(module_id, state.path, fine_grained_manager.deps, graph, - self.fscache) + def refresh_file(module: str, path: str) -> List[str]: + return fine_grained_manager.update([(module, path)], []) + + for module_id, state in list(graph.items()): + new_messages = refresh_suppressed_submodules( + module_id, state.path, fine_grained_manager.deps, graph, self.fscache, refresh_file + ) + if new_messages is not None: + messages = new_messages t3 = time.time() @@ -586,8 +592,15 @@ def fine_grained_increment_follow_imports(self, sources: List[BuildSource]) -> L messages = fine_grained_manager.update(new_unsuppressed, []) for module_id, path in new_unsuppressed: - refresh_suppressed_submodules(module_id, path, fine_grained_manager.deps, graph, - self.fscache) + new_messages = refresh_suppressed_submodules( + module_id, path, + fine_grained_manager.deps, + graph, + self.fscache, + refresh_file + ) + if new_messages is not None: + messages = new_messages t4 = time.time() diff --git a/mypy/server/update.py b/mypy/server/update.py index 6b7332035470f..2daa9430e0056 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -116,7 +116,7 @@ import sys import time from typing import ( - Dict, List, Set, Tuple, Union, Optional, NamedTuple, Sequence + Dict, List, Set, Tuple, Union, Optional, NamedTuple, Sequence, Callable ) from typing_extensions import Final @@ -1127,7 +1127,8 @@ def refresh_suppressed_submodules( path: Optional[str], deps: Dict[str, Set[str]], graph: Graph, - fscache: FileSystemCache) -> None: + fscache: FileSystemCache, + refresh_file: Callable[[str, str], List[str]]) -> Optional[List[str]]: """Look for submodules that are now suppressed in target package. If a submodule a.b gets added, we need to mark it as suppressed @@ -1139,10 +1140,15 @@ def refresh_suppressed_submodules( Args: module: target package in which to look for submodules path: path of the module + refresh_file: function that reads the AST of a module (returns error messages) + + Return a list of errors from refresh_file() if it was called. If the + return value is None, we didn't call refresh_file(). """ + messages = None if path is None or not path.endswith(INIT_SUFFIXES): # Only packages have submodules. - return + return None # Find any submodules present in the directory. pkgdir = os.path.dirname(path) for fnam in fscache.listdir(pkgdir): @@ -1153,6 +1159,10 @@ def refresh_suppressed_submodules( shortname = fnam.split('.')[0] submodule = module + '.' + shortname trigger = make_trigger(submodule) + + # We may be missing the required fine-grained deps. + ensure_deps_loaded(module, deps, graph) + if trigger in deps: for dep in deps[trigger]: # TODO: <...> deps, etc. @@ -1163,8 +1173,14 @@ def refresh_suppressed_submodules( if dep_module is not None: state = graph.get(dep_module) if state: + # Is the file may missing an AST in case it's read from cache? + if state.tree is None: + # Create AST for the file. This may produce some new errors + # that we need to propagate. + assert state.path is not None + messages = refresh_file(state.id, state.path) tree = state.tree - assert tree # TODO: What if doesn't exist? + assert tree # Will be fine, due to refresh_file() above for imp in tree.imports: if isinstance(imp, ImportFrom): if (imp.id == module @@ -1172,3 +1188,4 @@ def refresh_suppressed_submodules( # TODO: Only if does not exist already state.suppressed.append(submodule) state.suppressed_set.add(submodule) + return messages diff --git a/test-data/unit/fine-grained-follow-imports.test b/test-data/unit/fine-grained-follow-imports.test index e3368329d4502..a819bbb7720b1 100644 --- a/test-data/unit/fine-grained-follow-imports.test +++ b/test-data/unit/fine-grained-follow-imports.test @@ -700,3 +700,22 @@ import bar == src/foo.py:2: error: "str" not callable src/bar.py:1: error: "int" not callable + +[case testFollowImportsNormalSuppressedAffectsCachedFile-only_when_cache] +# flags: --follow-imports=normal +# cmd: mypy main.py + +[file main.py] +from p import m # type: ignore +m.f(1) + +[file p/__init__.py] + +[file p/m.py.2] +# This change will trigger a cached file (main.py) through a supressed +# submodule, resulting in additional errors in main.py. +def f() -> None: pass + +[out] +== +main.py:2: error: Too many arguments for "f" From 82004ccd613ff7d656a6f477e2a398b971c18656 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 16:23:48 +0000 Subject: [PATCH 7/8] Minor optimization --- mypy/server/update.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/server/update.py b/mypy/server/update.py index 2daa9430e0056..424648d49c7c4 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -1184,8 +1184,8 @@ def refresh_suppressed_submodules( for imp in tree.imports: if isinstance(imp, ImportFrom): if (imp.id == module - and any(name == shortname for name, _ in imp.names)): - # TODO: Only if does not exist already + and any(name == shortname for name, _ in imp.names) + and submodule not in state.suppressed_set): state.suppressed.append(submodule) state.suppressed_set.add(submodule) return messages From 578b935c75f3796e2e5d9374d16981f169efcb06 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 27 Mar 2020 16:51:29 +0000 Subject: [PATCH 8/8] Update comment --- mypy/server/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/server/update.py b/mypy/server/update.py index 424648d49c7c4..3e4cebf58dc2d 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -1165,7 +1165,7 @@ def refresh_suppressed_submodules( if trigger in deps: for dep in deps[trigger]: - # TODO: <...> deps, etc. + # We can ignore <...> deps since a submodule can't trigger any. state = graph.get(dep) if not state: # Maybe it's a non-top-level target. We only care about the module.