Store line numbers of imports in the cache metadata (#4533)

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.

Author: msullivan

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)

mypy/server/update.py

@@ -562,13 +562,15 @@ 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,
                     state.dependencies,
                     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,

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)

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]

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]