From dbd9282922d9f850c77c74a2251e87a96728f299 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Thu, 1 Feb 2018 17:59:05 -0800 Subject: [PATCH] Store line numbers of imports in the cache metadata This allows incremental modes to generate error messages that are both better and more consistent with non-incremental modes. Storing the data in a parallel array is kind of ugly and was done for consistency with dep_prios. It might be worth doing a refactor, though any other way will make the json bigger, which is presumbably why dep_prios was done with an array. --- mypy/build.py | 20 ++++++++++++++++---- mypy/server/update.py | 4 ++++ test-data/unit/check-incremental.test | 11 +++++++++++ test-data/unit/fine-grained-modules.test | 4 ---- test-data/unit/fine-grained.test | 17 +++++------------ 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 8aa95b6079e6..18958903a12f 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -393,6 +393,7 @@ def default_lib_path(data_dir: str, ('child_modules', List[str]), # all submodules of the given module ('options', Optional[Dict[str, object]]), # build options ('dep_prios', List[int]), + ('dep_lines', List[int]), ('interface_hash', str), # hash representing the public interface ('version_id', str), # mypy version for cache invalidation ('ignore_all', bool), # if errors were ignored @@ -418,6 +419,7 @@ def cache_meta_from_dict(meta: Dict[str, Any], data_json: str) -> CacheMeta: meta.get('child_modules', []), meta.get('options'), meta.get('dep_prios', []), + meta.get('dep_lines', []), meta.get('interface_hash', ''), meta.get('version_id', sentinel), meta.get('ignore_all', True), @@ -1040,7 +1042,8 @@ def find_cache_meta(id: str, path: str, manager: BuildManager) -> Optional[Cache # Ignore cache if generated by an older mypy version. if ((m.version_id != manager.version_id and not manager.options.skip_version_check) or m.options is None - or len(m.dependencies) != len(m.dep_prios)): + or len(m.dependencies) != len(m.dep_prios) + or len(m.dependencies) != len(m.dep_lines)): manager.log('Metadata abandoned for {}: new attributes are missing'.format(id)) return None @@ -1157,6 +1160,7 @@ def validate_meta(meta: Optional[CacheMeta], id: str, path: Optional[str], 'options': (manager.options.clone_for_module(id) .select_options_affecting_cache()), 'dep_prios': meta.dep_prios, + 'dep_lines': meta.dep_lines, 'interface_hash': meta.interface_hash, 'version_id': manager.version_id, 'ignore_all': meta.ignore_all, @@ -1186,7 +1190,7 @@ def compute_hash(text: str) -> str: def write_cache(id: str, path: str, tree: MypyFile, serialized_fine_grained_deps: Dict[str, List[str]], dependencies: List[str], suppressed: List[str], - child_modules: List[str], dep_prios: List[int], + child_modules: List[str], dep_prios: List[int], dep_lines: List[int], old_interface_hash: str, source_hash: str, ignore_all: bool, manager: BuildManager) -> Tuple[str, Optional[CacheMeta]]: """Write cache files for a module. @@ -1203,6 +1207,7 @@ def write_cache(id: str, path: str, tree: MypyFile, suppressed: module IDs which were suppressed as dependencies child_modules: module IDs which are this package's direct submodules dep_prios: priorities (parallel array to dependencies) + dep_lines: import line locations (parallel array to dependencies) old_interface_hash: the hash from the previous version of the data cache file source_hash: the hash of the source code ignore_all: the ignore_all flag for this module @@ -1286,6 +1291,7 @@ def write_cache(id: str, path: str, tree: MypyFile, 'child_modules': child_modules, 'options': options.select_options_affecting_cache(), 'dep_prios': dep_prios, + 'dep_lines': dep_lines, 'interface_hash': interface_hash, 'version_id': manager.version_id, 'ignore_all': ignore_all, @@ -1633,8 +1639,10 @@ def __init__(self, assert len(self.meta.dependencies) == len(self.meta.dep_prios) self.priorities = {id: pri for id, pri in zip(self.meta.dependencies, self.meta.dep_prios)} + assert len(self.meta.dependencies) == len(self.meta.dep_lines) + self.dep_line_map = {id: line + for id, line in zip(self.meta.dependencies, self.meta.dep_lines)} self.child_modules = set(self.meta.child_modules) - self.dep_line_map = {} else: # Parse the file (and then some) to get the dependencies. self.parse_file() @@ -2023,11 +2031,12 @@ def write_cache(self) -> None: self.mark_interface_stale(on_errors=True) return dep_prios = self.dependency_priorities() + dep_lines = self.dependency_lines() new_interface_hash, self.meta = write_cache( self.id, self.path, self.tree, {k: list(v) for k, v in self.fine_grained_deps.items()}, list(self.dependencies), list(self.suppressed), list(self.child_modules), - dep_prios, self.interface_hash, self.source_hash, self.ignore_all, + dep_prios, dep_lines, self.interface_hash, self.source_hash, self.ignore_all, self.manager) if new_interface_hash == self.interface_hash: self.manager.log("Cached module {} has same interface".format(self.id)) @@ -2039,6 +2048,9 @@ def write_cache(self) -> None: def dependency_priorities(self) -> List[int]: return [self.priorities.get(dep, PRI_HIGH) for dep in self.dependencies] + def dependency_lines(self) -> List[int]: + return [self.dep_line_map.get(dep, 1) for dep in self.dependencies] + def generate_unused_ignore_notes(self) -> None: if self.options.warn_unused_ignores: self.manager.errors.generate_unused_ignore_notes(self.xpath) diff --git a/mypy/server/update.py b/mypy/server/update.py index 0cb81b5dc2df..6b0a9721c833 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -562,6 +562,7 @@ def preserve_full_cache(graph: Graph, manager: BuildManager) -> SavedCache: # There is no corresponding JSON so create partial "memory-only" metadata. assert state.path dep_prios = state.dependency_priorities() + dep_lines = state.dependency_lines() meta = memory_only_cache_meta( id, state.path, @@ -569,6 +570,7 @@ def preserve_full_cache(graph: Graph, manager: BuildManager) -> SavedCache: state.suppressed, list(state.child_modules), dep_prios, + dep_lines, state.source_hash, state.ignore_all, manager) @@ -584,6 +586,7 @@ def memory_only_cache_meta(id: str, suppressed: List[str], child_modules: List[str], dep_prios: List[int], + dep_lines: List[int], source_hash: str, ignore_all: bool, manager: BuildManager) -> CacheMeta: @@ -603,6 +606,7 @@ def memory_only_cache_meta(id: str, 'child_modules': child_modules, 'options': options.select_options_affecting_cache(), 'dep_prios': dep_prios, + 'dep_lines': dep_lines, 'interface_hash': '', 'version_id': manager.version_id, 'ignore_all': ignore_all, diff --git a/test-data/unit/check-incremental.test b/test-data/unit/check-incremental.test index 653db572564c..ebd17c2b3411 100644 --- a/test-data/unit/check-incremental.test +++ b/test-data/unit/check-incremental.test @@ -3466,3 +3466,14 @@ tmp/main.py:2: error: Expression has type "Any" [out2] tmp/main.py:2: error: Expression has type "Any" + +[case testDeletedDepLineNumber] +# The import is not on line 1 and that data should be preserved +import a +[file a.py] +[delete a.py.2] +[out1] + +[out2] +main:2: error: Cannot find module named 'a' +main:2: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) diff --git a/test-data/unit/fine-grained-modules.test b/test-data/unit/fine-grained-modules.test index 91c7589e6362..0a254e2ae51f 100644 --- a/test-data/unit/fine-grained-modules.test +++ b/test-data/unit/fine-grained-modules.test @@ -485,8 +485,6 @@ def g() -> None: pass == main:1: error: Cannot find module named 'a' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) --- TODO: Remove redundant error message -main:1: error: Cannot find module named 'b' main:2: error: Cannot find module named 'b' [case testDeleteTwoFilesNoErrors] @@ -528,8 +526,6 @@ a.py:3: error: Too many arguments for "g" == main:1: error: Cannot find module named 'a' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) --- TODO: Remove redundant error message -main:1: error: Cannot find module named 'b' main:2: error: Cannot find module named 'b' [case testAddFileWhichImportsLibModule] diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index d0a0933b9c80..b97b3eeeb5c4 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -1081,9 +1081,8 @@ def f() -> Iterator[None]: [out] main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]' == -a.py:1: error: Cannot find module named 'b' -a.py:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) a.py:3: error: Cannot find module named 'b' +a.py:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]' == main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]' @@ -1129,9 +1128,8 @@ def g() -> None: [out] a.py:11: error: Too many arguments for "h" == -a.py:1: error: Cannot find module named 'b' -a.py:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) a.py:10: error: Cannot find module named 'b' +a.py:10: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) == a.py:11: error: Too many arguments for "h" == @@ -1164,9 +1162,8 @@ def f(x: List[int]) -> Iterator[None]: [builtins fixtures/list.pyi] [out] == -a.py:1: error: Cannot find module named 'b' -a.py:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) a.py:3: error: Cannot find module named 'b' +a.py:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) == == @@ -1223,10 +1220,8 @@ def g() -> None: pass [delete n.py.2] [out] == -main:1: error: Cannot find module named 'm' -main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) -main:1: error: Cannot find module named 'n' main:2: error: Cannot find module named 'm' +main:2: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) main:9: error: Cannot find module named 'n' [case testOverloadSpecialCase] @@ -1253,10 +1248,8 @@ def g() -> None: pass [builtins fixtures/ops.pyi] [out] == -main:1: error: Cannot find module named 'm' -main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) -main:1: error: Cannot find module named 'n' main:2: error: Cannot find module named 'm' +main:2: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) main:14: error: Cannot find module named 'n' [case testRefreshGenericClass]