Do not show unused-ignore errors in unreachable code, and make it a real error code (#15164)

Fixes #8823 

The issue above makes `--warn-unused-ignores` problematic for
cross-platform/cross-version code (btw it is one of the most upvoted
bugs). Due to how the unused ignore errors are generated, it is hard to
not generate them in unreachable code without having precise span
information for blocks. So my fix will not work on Python 3.7, where end
lines are not available, but taking into account that 3.7 reaches end of
life in less than 2 month, it is not worth the effort.

Also this PR makes `unused-ignore` work much more like a regular error
code, so it can be used with
`--enable-error-code`/`--disable-error-code` (including per-file) etc.
More importantly this will also allow to use `--warn-unused-ignores` in
code that uses e.g. `try/except` imports (instead of `if` version
checks) with the help of `# type: ignore[import,unused-ignore]` (which
btw will also help on Python 3.7).

Note also currently line numbers for `Block`s are actually wrong (and
hence few TODOs in `fastparse.py`). So in this PR I set them all
correctly and update a lot of parse tests (I went through all test
updates and verified they are reasonable).

---------

Co-authored-by: Jukka Lehtosalo <jukka.lehtosalo@iki.fi>

Author: ilevkivskyi

docs/source/error_code_list2.rst

@@ -347,3 +347,47 @@ silence the error:
 
     async def g() -> None:
         _ = asyncio.create_task(f())  # No error
+
+Check that ``# type: ignore`` comment is used [unused-ignore]
+-------------------------------------------------------------
+
+If you use :option:`--enable-error-code unused-ignore <mypy --enable-error-code>`,
+or :option:`--warn-unused-ignores <mypy --warn-unused-ignores>`
+mypy generates an error if you don't use a ``# type: ignore`` comment, i.e. if
+there is a comment, but there would be no error generated by mypy on this line
+anyway.
+
+Example:
+
+.. code-block:: python
+
+    # Use "mypy --warn-unused-ignores ..."
+
+    def add(a: int, b: int) -> int:
+        # Error: unused "type: ignore" comment
+        return a + b  # type: ignore
+
+Note that due to a specific nature of this comment, the only way to selectively
+silence it, is to include the error code explicitly. Also note that this error is
+not shown if the ``# type: ignore`` is not used due to code being statically
+unreachable (e.g. due to platform or version checks).
+
+Example:
+
+.. code-block:: python
+
+    # Use "mypy --warn-unused-ignores ..."
+
+    import sys
+
+    try:
+        # The "[unused-ignore]" is needed to get a clean mypy run
+        # on both Python 3.8, and 3.9 where this module was added
+        import graphlib  # type: ignore[import,unused-ignore]
+    except ImportError:
+        pass
+
+    if sys.version_info >= (3, 9):
+        # The following will not generate an error on either
+        # Python 3.8, or Python 3.9
+        42 + "testing..."  # type: ignore

mypy/build.py

@@ -2238,6 +2238,7 @@ def semantic_analysis_pass1(self) -> None:
         analyzer = SemanticAnalyzerPreAnalysis()
         with self.wrap_context():
             analyzer.visit_file(self.tree, self.xpath, self.id, options)
+        self.manager.errors.set_unreachable_lines(self.xpath, self.tree.unreachable_lines)
         # TODO: Do this while constructing the AST?
         self.tree.names = SymbolTable()
         if not self.tree.is_stub:
@@ -2572,7 +2573,10 @@ def dependency_lines(self) -> list[int]:
         return [self.dep_line_map.get(dep, 1) for dep in self.dependencies + self.suppressed]
 
     def generate_unused_ignore_notes(self) -> None:
-        if self.options.warn_unused_ignores:
+        if (
+            self.options.warn_unused_ignores
+            or codes.UNUSED_IGNORE in self.options.enabled_error_codes
+        ) and codes.UNUSED_IGNORE not in self.options.disabled_error_codes:
             # If this file was initially loaded from the cache, it may have suppressed
             # dependencies due to imports with ignores on them. We need to generate
             # those errors to avoid spuriously flagging them as unused ignores.

mypy/errorcodes.py

@@ -221,6 +221,9 @@ def __str__(self) -> str:
 USED_BEFORE_DEF: Final[ErrorCode] = ErrorCode(
     "used-before-def", "Warn about variables that are used before they are defined", "General"
 )
+UNUSED_IGNORE: Final = ErrorCode(
+    "unused-ignore", "Ensure that all type ignores are used", "General", default_enabled=False
+)
 
 
 # Syntax errors are often blocking.

mypy/errors.py

@@ -222,6 +222,9 @@ class Errors:
     # (path -> line -> error-codes)
     ignored_lines: dict[str, dict[int, list[str]]]
 
+    # Lines that are statically unreachable (e.g. due to platform/version check).
+    unreachable_lines: dict[str, set[int]]
+
     # Lines on which an error was actually ignored.
     used_ignored_lines: dict[str, dict[int, list[str]]]
 
@@ -277,6 +280,7 @@ def initialize(self) -> None:
         self.import_ctx = []
         self.function_or_member = [None]
         self.ignored_lines = {}
+        self.unreachable_lines = {}
         self.used_ignored_lines = defaultdict(lambda: defaultdict(list))
         self.ignored_files = set()
         self.only_once_messages = set()
@@ -325,6 +329,9 @@ def set_file_ignored_lines(
         if ignore_all:
             self.ignored_files.add(file)
 
+    def set_unreachable_lines(self, file: str, unreachable_lines: set[int]) -> None:
+        self.unreachable_lines[file] = unreachable_lines
+
     def current_target(self) -> str | None:
         """Retrieves the current target from the associated scope.
 
@@ -623,6 +630,10 @@ def generate_unused_ignore_errors(self, file: str) -> None:
         ignored_lines = self.ignored_lines[file]
         used_ignored_lines = self.used_ignored_lines[file]
         for line, ignored_codes in ignored_lines.items():
+            if line in self.unreachable_lines[file]:
+                continue
+            if codes.UNUSED_IGNORE.code in ignored_codes:
+                continue
             used_ignored_codes = used_ignored_lines[line]
             unused_ignored_codes = set(ignored_codes) - set(used_ignored_codes)
             # `ignore` is used
@@ -639,7 +650,7 @@ def generate_unused_ignore_errors(self, file: str) -> None:
             for unused in unused_ignored_codes:
                 narrower = set(used_ignored_codes) & codes.sub_code_map[unused]
                 if narrower:
-                    message += f", use narrower [{', '.join(narrower)}] instead of [{unused}]"
+                    message += f", use narrower [{', '.join(narrower)}] instead of [{unused}] code"
             # Don't use report since add_error_info will ignore the error!
             info = ErrorInfo(
                 self.import_context(),
@@ -653,7 +664,7 @@ def generate_unused_ignore_errors(self, file: str) -> None:
                 -1,
                 "error",
                 message,
-                None,
+                codes.UNUSED_IGNORE,
                 False,
                 False,
                 False,

mypy/fastparse.py

@@ -516,6 +516,7 @@ def translate_stmt_list(
                 codes.FILE.code
             )
             block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
+            self.set_block_lines(block, stmts)
             mark_block_unreachable(block)
             return [block]
 
@@ -611,30 +612,38 @@ def from_comp_operator(self, op: ast3.cmpop) -> str:
         else:
             return op_name
 
-    def as_block(self, stmts: list[ast3.stmt], lineno: int) -> Block | None:
+    def set_block_lines(self, b: Block, stmts: Sequence[ast3.stmt]) -> None:
+        first, last = stmts[0], stmts[-1]
+        b.line = first.lineno
+        b.column = first.col_offset
+        b.end_line = getattr(last, "end_lineno", None)
+        b.end_column = getattr(last, "end_col_offset", None)
+        if not b.body:
+            return
+        new_first = b.body[0]
+        if isinstance(new_first, (Decorator, OverloadedFuncDef)):
+            # Decorated function lines are different between Python versions.
+            # copy the normalization we do for them to block first lines.
+            b.line = new_first.line
+            b.column = new_first.column
+
+    def as_block(self, stmts: list[ast3.stmt]) -> Block | None:
         b = None
         if stmts:
             b = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
-            b.set_line(lineno)
+            self.set_block_lines(b, stmts)
         return b
 
     def as_required_block(
-        self,
-        stmts: list[ast3.stmt],
-        lineno: int,
-        *,
-        can_strip: bool = False,
-        is_coroutine: bool = False,
+        self, stmts: list[ast3.stmt], *, can_strip: bool = False, is_coroutine: bool = False
     ) -> Block:
         assert stmts  # must be non-empty
         b = Block(
             self.fix_function_overloads(
                 self.translate_stmt_list(stmts, can_strip=can_strip, is_coroutine=is_coroutine)
             )
         )
-        # TODO: in most call sites line is wrong (includes first line of enclosing statement)
-        # TODO: also we need to set the column, and the end position here.
-        b.set_line(lineno)
+        self.set_block_lines(b, stmts)
         return b
 
     def fix_function_overloads(self, stmts: list[Statement]) -> list[Statement]:
@@ -1023,7 +1032,7 @@ def do_func_def(
 
         self.class_and_function_stack.pop()
         self.class_and_function_stack.append("F")
-        body = self.as_required_block(n.body, lineno, can_strip=True, is_coroutine=is_coroutine)
+        body = self.as_required_block(n.body, can_strip=True, is_coroutine=is_coroutine)
         func_def = FuncDef(n.name, args, body, func_type)
         if isinstance(func_def.type, CallableType):
             # semanal.py does some in-place modifications we want to avoid
@@ -1052,9 +1061,6 @@ def do_func_def(
             func_def.is_decorated = True
             func_def.deco_line = deco_line
             func_def.set_line(lineno, n.col_offset, end_line, end_column)
-            # Set the line again after we updated it (to make value same in Python 3.7/3.8)
-            # Note that TODOs in as_required_block() apply here as well.
-            func_def.body.set_line(lineno)
 
             deco = Decorator(func_def, self.translate_expr_list(n.decorator_list), var)
             first = n.decorator_list[0]
@@ -1165,7 +1171,7 @@ def visit_ClassDef(self, n: ast3.ClassDef) -> ClassDef:
 
         cdef = ClassDef(
             n.name,
-            self.as_required_block(n.body, n.lineno),
+            self.as_required_block(n.body),
             None,
             self.translate_expr_list(n.bases),
             metaclass=dict(keywords).get("metaclass"),
@@ -1237,8 +1243,8 @@ def visit_For(self, n: ast3.For) -> ForStmt:
         node = ForStmt(
             self.visit(n.target),
             self.visit(n.iter),
-            self.as_required_block(n.body, n.lineno),
-            self.as_block(n.orelse, n.lineno),
+            self.as_required_block(n.body),
+            self.as_block(n.orelse),
             target_type,
         )
         return self.set_line(node, n)
@@ -1249,8 +1255,8 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
         node = ForStmt(
             self.visit(n.target),
             self.visit(n.iter),
-            self.as_required_block(n.body, n.lineno),
-            self.as_block(n.orelse, n.lineno),
+            self.as_required_block(n.body),
+            self.as_block(n.orelse),
             target_type,
         )
         node.is_async = True
@@ -1259,19 +1265,14 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
     # While(expr test, stmt* body, stmt* orelse)
     def visit_While(self, n: ast3.While) -> WhileStmt:
         node = WhileStmt(
-            self.visit(n.test),
-            self.as_required_block(n.body, n.lineno),
-            self.as_block(n.orelse, n.lineno),
+            self.visit(n.test), self.as_required_block(n.body), self.as_block(n.orelse)
         )
         return self.set_line(node, n)
 
     # If(expr test, stmt* body, stmt* orelse)
     def visit_If(self, n: ast3.If) -> IfStmt:
-        lineno = n.lineno
         node = IfStmt(
-            [self.visit(n.test)],
-            [self.as_required_block(n.body, lineno)],
-            self.as_block(n.orelse, lineno),
+            [self.visit(n.test)], [self.as_required_block(n.body)], self.as_block(n.orelse)
         )
         return self.set_line(node, n)
 
@@ -1281,7 +1282,7 @@ def visit_With(self, n: ast3.With) -> WithStmt:
         node = WithStmt(
             [self.visit(i.context_expr) for i in n.items],
             [self.visit(i.optional_vars) for i in n.items],
-            self.as_required_block(n.body, n.lineno),
+            self.as_required_block(n.body),
             target_type,
         )
         return self.set_line(node, n)
@@ -1292,7 +1293,7 @@ def visit_AsyncWith(self, n: ast3.AsyncWith) -> WithStmt:
         s = WithStmt(
             [self.visit(i.context_expr) for i in n.items],
             [self.visit(i.optional_vars) for i in n.items],
-            self.as_required_block(n.body, n.lineno),
+            self.as_required_block(n.body),
             target_type,
         )
         s.is_async = True
@@ -1309,15 +1310,15 @@ def visit_Try(self, n: ast3.Try) -> TryStmt:
             self.set_line(NameExpr(h.name), h) if h.name is not None else None for h in n.handlers
         ]
         types = [self.visit(h.type) for h in n.handlers]
-        handlers = [self.as_required_block(h.body, h.lineno) for h in n.handlers]
+        handlers = [self.as_required_block(h.body) for h in n.handlers]
 
         node = TryStmt(
-            self.as_required_block(n.body, n.lineno),
+            self.as_required_block(n.body),
             vs,
             types,
             handlers,
-            self.as_block(n.orelse, n.lineno),
-            self.as_block(n.finalbody, n.lineno),
+            self.as_block(n.orelse),
+            self.as_block(n.finalbody),
         )
         return self.set_line(node, n)
 
@@ -1326,15 +1327,15 @@ def visit_TryStar(self, n: TryStar) -> TryStmt:
             self.set_line(NameExpr(h.name), h) if h.name is not None else None for h in n.handlers
         ]
         types = [self.visit(h.type) for h in n.handlers]
-        handlers = [self.as_required_block(h.body, h.lineno) for h in n.handlers]
+        handlers = [self.as_required_block(h.body) for h in n.handlers]
 
         node = TryStmt(
-            self.as_required_block(n.body, n.lineno),
+            self.as_required_block(n.body),
             vs,
             types,
             handlers,
-            self.as_block(n.orelse, n.lineno),
-            self.as_block(n.finalbody, n.lineno),
+            self.as_block(n.orelse),
+            self.as_block(n.finalbody),
         )
         node.is_star = True
         return self.set_line(node, n)
@@ -1469,9 +1470,7 @@ def visit_Lambda(self, n: ast3.Lambda) -> LambdaExpr:
         body.col_offset = n.body.col_offset
 
         self.class_and_function_stack.append("L")
-        e = LambdaExpr(
-            self.transform_args(n.args, n.lineno), self.as_required_block([body], n.lineno)
-        )
+        e = LambdaExpr(self.transform_args(n.args, n.lineno), self.as_required_block([body]))
         self.class_and_function_stack.pop()
         e.set_line(n.lineno, n.col_offset)  # Overrides set_line -- can't use self.set_line
         return e
@@ -1743,7 +1742,7 @@ def visit_Match(self, n: Match) -> MatchStmt:
             self.visit(n.subject),
             [self.visit(c.pattern) for c in n.cases],
             [self.visit(c.guard) for c in n.cases],
-            [self.as_required_block(c.body, n.lineno) for c in n.cases],
+            [self.as_required_block(c.body) for c in n.cases],
         )
         return self.set_line(node, n)
 

mypy/nodes.py

@@ -287,6 +287,7 @@ class MypyFile(SymbolNode):
         "names",
         "imports",
         "ignored_lines",
+        "unreachable_lines",
         "is_stub",
         "is_cache_skeleton",
         "is_partial_stub_package",
@@ -313,6 +314,8 @@ class MypyFile(SymbolNode):
     # If the value is empty, ignore all errors; otherwise, the list contains all
     # error codes to ignore.
     ignored_lines: dict[int, list[str]]
+    # Lines that are statically unreachable (e.g. due to platform/version check).
+    unreachable_lines: set[int]
     # Is this file represented by a stub file (.pyi)?
     is_stub: bool
     # Is this loaded from the cache and thus missing the actual body of the file?
@@ -345,6 +348,7 @@ def __init__(
             self.ignored_lines = ignored_lines
         else:
             self.ignored_lines = {}
+        self.unreachable_lines = set()
 
         self.path = ""
         self.is_stub = False