WIP: Warn on unused # type: ignores

Still needs tests.

Author: rwbarton

mypy/build.py

@@ -68,6 +68,8 @@
 CHECK_UNTYPED_DEFS = 'check-untyped-defs'
 # Also check typeshed for missing annotations
 WARN_INCOMPLETE_STUB = 'warn-incomplete-stub'
+# Warn about unused '# type: ignore' comments
+WARN_UNUSED_IGNORES = 'warn-unused-ignores'
 
 PYTHON_EXTENSIONS = ['.pyi', '.py']
 
@@ -1334,6 +1336,8 @@ def dispatch(sources: List[BuildSource], manager: BuildManager) -> None:
     graph = load_graph(sources, manager)
     manager.log("Loaded graph with %d nodes" % len(graph))
     process_graph(graph, manager)
+    if WARN_UNUSED_IGNORES in manager.flags:
+        manager.errors.generate_unused_ignore_notes()
 
 
 def load_graph(sources: List[BuildSource], manager: BuildManager) -> Graph:

mypy/checker.py

@@ -416,8 +416,7 @@ def visit_file(self, file_node: MypyFile, path: str) -> None:
         """Type check a mypy file with the given path."""
         self.pass_num = 0
         self.is_stub = file_node.is_stub
-        self.errors.set_file(path)
-        self.errors.set_ignored_lines(file_node.ignored_lines)
+        self.errors.set_file(path, file_node.ignored_lines)
         self.globals = file_node.names
         self.weak_opts = file_node.weak_opts
         self.enter_partial_types()
@@ -432,7 +431,6 @@ def visit_file(self, file_node: MypyFile, path: str) -> None:
         if self.deferred_nodes:
             self.check_second_pass()
 
-        self.errors.set_ignored_lines(set())
         self.current_node_deferred = False
 
         all_ = self.globals.get('__all__')
@@ -1522,7 +1520,7 @@ def set_inference_error_fallback_type(self, var: Var, lvalue: Node, type: Type,
 
         We implement this here by giving x a valid type (Any).
         """
-        if context.get_line() in self.errors.ignored_lines:
+        if context.get_line() in self.errors.ignored_lines[self.errors.file]:
             self.set_inferred_type(var, lvalue, AnyType())
 
     def narrow_type_from_binder(self, expr: Node, known_type: Type) -> Type:

mypy/errors.py

@@ -2,8 +2,9 @@
 import os.path
 import sys
 import traceback
+from collections import defaultdict, OrderedDict
 
-from typing import Tuple, List, TypeVar, Set
+from typing import Tuple, List, TypeVar, Set, Dict, Optional
 
 
 T = TypeVar('T')
@@ -79,8 +80,11 @@ class Errors:
     # Stack of short names of current functions or members (or None).
     function_or_member = None  # type: List[str]
 
-    # Ignore errors on these lines.
-    ignored_lines = None  # type: Set[int]
+    # Ignore errors on these lines of each file.
+    ignored_lines = None  # type: Dict[str, Set[int]]
+
+    # Lines on which an error was actually ignored.
+    used_ignored_lines = None  # type: Dict[str, Set[int]]
 
     # Collection of reported only_once messages.
     only_once_messages = None  # type: Set[str]
@@ -90,7 +94,8 @@ def __init__(self) -> None:
         self.import_ctx = []
         self.type_name = [None]
         self.function_or_member = [None]
-        self.ignored_lines = set()
+        self.ignored_lines = OrderedDict()
+        self.used_ignored_lines = defaultdict(set)
         self.only_once_messages = set()
 
     def copy(self) -> 'Errors':
@@ -109,13 +114,14 @@ def set_ignore_prefix(self, prefix: str) -> None:
             prefix += os.sep
         self.ignore_prefix = prefix
 
-    def set_file(self, file: str) -> None:
+    def set_file(self, file: str, ignored_lines: Set[int] = None) -> None:
         """Set the path of the current file."""
         file = os.path.normpath(file)
         self.file = remove_path_prefix(file, self.ignore_prefix)
-
-    def set_ignored_lines(self, ignored_lines: Set[int]) -> None:
-        self.ignored_lines = ignored_lines
+        if ignored_lines is not None:
+            if self.file in self.ignored_lines:
+                assert self.ignored_lines[self.file] == ignored_lines
+            self.ignored_lines[self.file] = ignored_lines
 
     def push_function(self, name: str) -> None:
         """Set the current function or member short name (it can be None)."""
@@ -170,15 +176,26 @@ def report(self, line: int, message: str, blocker: bool = False,
         self.add_error_info(info)
 
     def add_error_info(self, info: ErrorInfo) -> None:
-        if info.line in self.ignored_lines:
+        assert self.file == info.file
+        if self.file in self.ignored_lines and info.line in self.ignored_lines[self.file]:
             # Annotation requests us to ignore all errors on this line.
+            self.used_ignored_lines[self.file].add(info.line)
             return
         if info.only_once:
             if info.message in self.only_once_messages:
                 return
             self.only_once_messages.add(info.message)
         self.error_info.append(info)
 
+    def generate_unused_ignore_notes(self) -> None:
+        for file, ignored_lines in self.ignored_lines.items():
+            for line in ignored_lines - self.used_ignored_lines[file]:
+                # Don't use report since add_error_info will ignore the error!
+                info = ErrorInfo(self.import_context(), file, None, None,
+                                 line, 'note', "unused 'type: ignore' comment",
+                                 False, False)
+                self.error_info.append(info)
+
     def num_messages(self) -> int:
         """Return the number of generated messages."""
         return len(self.error_info)

mypy/main.py

@@ -153,6 +153,8 @@ def parse_version(v):
     parser.add_argument('--warn-incomplete-stub', action='store_true',
                         help="warn if missing type annotation in typeshed, only relevant with"
                         " --check-untyped-defs enabled")
+    parser.add_argument('--warn-unused-ignores', action='store_true',
+                        help="warn about unneeded '# type: ignore' comments")
     parser.add_argument('--fast-parser', action='store_true',
                         help="enable experimental fast parser")
     parser.add_argument('-i', '--incremental', action='store_true',
@@ -252,6 +254,9 @@ def parse_version(v):
     if args.warn_incomplete_stub:
         options.build_flags.append(build.WARN_INCOMPLETE_STUB)
 
+    if args.warn_unused_ignores:
+        options.build_flags.append(build.WARN_UNUSED_IGNORES)
+
     # experimental
     if args.fast_parser:
         options.build_flags.append(build.FAST_PARSER)

mypy/semanal.py

@@ -219,8 +219,7 @@ def __init__(self,
         self.all_exports = set()  # type: Set[str]
 
     def visit_file(self, file_node: MypyFile, fnam: str) -> None:
-        self.errors.set_file(fnam)
-        self.errors.set_ignored_lines(file_node.ignored_lines)
+        self.errors.set_file(fnam, file_node.ignored_lines)
         self.cur_mod_node = file_node
         self.cur_mod_id = file_node.fullname()
         self.is_stub_file = fnam.lower().endswith('.pyi')
@@ -243,8 +242,6 @@ def visit_file(self, file_node: MypyFile, fnam: str) -> None:
         if self.cur_mod_id == 'builtins':
             remove_imported_names_from_symtable(self.globals, 'builtins')
 
-        self.errors.set_ignored_lines(set())
-
         if '__all__' in self.globals:
             for name, g in self.globals.items():
                 if name not in self.all_exports:
@@ -2469,10 +2466,8 @@ def __init__(self, modules: Dict[str, MypyFile], errors: Errors) -> None:
         self.errors = errors
 
     def visit_file(self, file_node: MypyFile, fnam: str) -> None:
-        self.errors.set_file(fnam)
-        self.errors.set_ignored_lines(file_node.ignored_lines)
+        self.errors.set_file(fnam, file_node.ignored_lines)
         self.accept(file_node)
-        self.errors.set_ignored_lines(set())
 
     def accept(self, node: Node) -> None:
         try: