Expression-scoped ignores in Python 3.8. (#6648)

Fixes #1032. On Python 3.8, we use `end_lineno` information to scope `# type: ignore` comments to enclosing expressions. Auto-formatter users, rejoice!

The `--warn-unused-ignores` semantics should be clear from the test cases. Basically, ignores in inner scopes take precedence over ignores in enclosing scopes, and the first of multiple ignores in a scope "wins".

A few notes:
 - This adds a new slot, `Context.end_line`. It defaults to `None` in the absence of an `end_lineno` or if `Context` is not an expression.
 - `ErrorInfo.origin` now has a third member, representing the end line of the error context. It is used to determine the range of lines to search for `# type: ignore` comments. If unavailable, it defaults to the same value as the origin line. 
 - Because this uses 3.8-only AST features, a new `check-38.test` file has been created, and is hidden behind a version guard in `testcheck.py`.

Author: brandtbucher

mypy/errors.py

@@ -54,8 +54,9 @@ class ErrorInfo:
     # Only report this particular messages once per program.
     only_once = False
 
-    # Actual origin of the error message as tuple (path, line number)
-    origin = None  # type: Tuple[str, int]
+    # Actual origin of the error message as tuple (path, line number, end line number)
+    # If end line number is unknown, use line number.
+    origin = None  # type: Tuple[str, int, int]
 
     # Fine-grained incremental target where this was reported
     target = None  # type: Optional[str]
@@ -72,7 +73,7 @@ def __init__(self,
                  message: str,
                  blocker: bool,
                  only_once: bool,
-                 origin: Optional[Tuple[str, int]] = None,
+                 origin: Optional[Tuple[str, int, int]] = None,
                  target: Optional[str] = None) -> None:
         self.import_ctx = import_ctx
         self.file = file
@@ -85,7 +86,7 @@ def __init__(self,
         self.message = message
         self.blocker = blocker
         self.only_once = only_once
-        self.origin = origin or (file, line)
+        self.origin = origin or (file, line, line)
         self.target = target
 
 
@@ -233,7 +234,8 @@ def report(self,
                file: Optional[str] = None,
                only_once: bool = False,
                origin_line: Optional[int] = None,
-               offset: int = 0) -> None:
+               offset: int = 0,
+               end_line: Optional[int] = None) -> None:
         """Report message at the given line using the current error context.
 
         Args:
@@ -244,6 +246,7 @@ def report(self,
             file: if non-None, override current file as context
             only_once: if True, only report this exact message once per build
             origin_line: if non-None, override current context as origin
+            end_line: if non-None, override current context as end
         """
         if self.scope:
             type = self.scope.current_type_name()
@@ -260,10 +263,17 @@ def report(self,
             file = self.file
         if offset:
             message = " " * offset + message
+
+        if origin_line is None:
+            origin_line = line
+
+        if end_line is None:
+            end_line = origin_line
+
         info = ErrorInfo(self.import_context(), file, self.current_module(), type,
                          function, line, column, severity, message,
                          blocker, only_once,
-                         origin=(self.file, origin_line) if origin_line else None,
+                         origin=(self.file, origin_line, end_line),
                          target=self.current_target())
         self.add_error_info(info)
 
@@ -274,12 +284,17 @@ def _add_error_info(self, file: str, info: ErrorInfo) -> None:
         self.error_info_map[file].append(info)
 
     def add_error_info(self, info: ErrorInfo) -> None:
-        file, line = info.origin
+        file, line, end_line = info.origin
         if not info.blocker:  # Blockers cannot be ignored
-            if file in self.ignored_lines and line in self.ignored_lines[file]:
-                # Annotation requests us to ignore all errors on this line.
-                self.used_ignored_lines[file].add(line)
-                return
+            if file in self.ignored_lines:
+                # Check each line in this context for "type: ignore" comments.
+                # For anything other than Python 3.8 expressions, line == end_line,
+                # so we only loop once.
+                for scope_line in range(line, end_line + 1):
+                    if scope_line in self.ignored_lines[file]:
+                        # Annotation requests us to ignore all errors on this line.
+                        self.used_ignored_lines[file].add(scope_line)
+                        return
             if file in self.ignored_files:
                 return
         if info.only_once:

mypy/fastparse.py

@@ -280,6 +280,7 @@ def visit(self, node: Optional[AST]) -> Any:
     def set_line(self, node: N, n: Union[ast3.expr, ast3.stmt]) -> N:
         node.line = n.lineno
         node.column = n.col_offset
+        node.end_line = getattr(n, "end_lineno", None) if isinstance(n, ast3.expr) else None
         return node
 
     def translate_expr_list(self, l: Sequence[AST]) -> List[Expression]:

mypy/messages.py

@@ -111,11 +111,18 @@ def report(self, msg: str, context: Optional[Context], severity: str,
                file: Optional[str] = None, origin: Optional[Context] = None,
                offset: int = 0) -> None:
         """Report an error or note (unless disabled)."""
+        if origin is not None:
+            end_line = origin.end_line
+        elif context is not None:
+            end_line = context.end_line
+        else:
+            end_line = None
         if self.disable_count <= 0:
             self.errors.report(context.get_line() if context else -1,
                                context.get_column() if context else -1,
                                msg, severity=severity, file=file, offset=offset,
-                               origin_line=origin.get_line() if origin else None)
+                               origin_line=origin.get_line() if origin else None,
+                               end_line=end_line)
 
     def fail(self, msg: str, context: Optional[Context], file: Optional[str] = None,
              origin: Optional[Context] = None) -> None:

mypy/nodes.py

@@ -22,11 +22,12 @@
 
 class Context:
     """Base type for objects that are valid as error message locations."""
-    __slots__ = ('line', 'column')
+    __slots__ = ('line', 'column', 'end_line')
 
     def __init__(self, line: int = -1, column: int = -1) -> None:
         self.line = line
         self.column = column
+        self.end_line = None  # type: Optional[int]
 
     def set_line(self, target: Union['Context', int], column: Optional[int] = None) -> None:
         """If target is a node, pull line (and column) information
@@ -38,6 +39,7 @@ def set_line(self, target: Union['Context', int], column: Optional[int] = None)
         else:
             self.line = target.line
             self.column = target.column
+            self.end_line = target.end_line
 
         if column is not None:
             self.column = column

mypy/test/testcheck.py

@@ -85,6 +85,10 @@
     'check-newsemanal.test',
 ]
 
+# Tests that use Python 3.8-only AST features (like expression-scoped ignores):
+if sys.version_info >= (3, 8):
+    typecheck_files.append('check-38.test')
+
 
 class TypeCheckSuite(DataSuite):
     files = typecheck_files

test-data/unit/check-38.test

@@ -0,0 +1,79 @@
+[case testIgnoreScopeIssue1032]
+def f(a: int): ...
+f(
+    "IGNORE"
+)  # type: ignore
+
+[case testIgnoreScopeNested1]
+def f(a: int) -> int: ...
+f(
+    f(
+        "IGNORE"
+    )  # type: ignore
+)
+
+[case testIgnoreScopeNested2]
+[
+    "IGNORE"  # type: ignore
+    &
+    "IGNORE",
+]
+[builtins fixtures/list.pyi]
+
+[case testIgnoreScopeNested3]
+{
+    "IGNORE"
+    |  # type: ignore
+    "IGNORE",
+}
+[builtins fixtures/set.pyi]
+
+[case testIgnoreScopeNested4]
+{
+    None: "IGNORE"
+    ^
+    "IGNORE",  # type: ignore
+}
+[builtins fixtures/dict.pyi]
+
+[case testIgnoreScopeNestedNonOverlapping]
+def f(x: int): ...
+def g(x: int): ...
+(
+    f("ERROR"),  # E: Argument 1 to "f" has incompatible type "str"; expected "int"
+    g("IGNORE"),  # type: ignore
+    f("ERROR"),  # E: Argument 1 to "f" has incompatible type "str"; expected "int"
+)
+
+[case testIgnoreScopeNestedOverlapping]
+def f(x: int): ...
+def g(x: int): ...
+(
+    f("ERROR"), g(  # E: Argument 1 to "f" has incompatible type "str"; expected "int"
+        "IGNORE"  # type: ignore
+    ), f("ERROR"),  # E: Argument 1 to "f" has incompatible type "str"; expected "int"
+)
+
+[case testIgnoreScopeUnused1]
+# flags: --warn-unused-ignores
+(  # type: ignore  # N: unused 'type: ignore' comment
+    "IGNORE"  # type: ignore
+    +  # type: ignore  # N: unused 'type: ignore' comment
+    0  # type: ignore  # N: unused 'type: ignore' comment
+)  # type: ignore  # N: unused 'type: ignore' comment
+
+[case testIgnoreScopeUnused2]
+# flags: --warn-unused-ignores
+(  # type: ignore  # N: unused 'type: ignore' comment
+    "IGNORE"
+    -  # type: ignore
+    0  # type: ignore  # N: unused 'type: ignore' comment
+)  # type: ignore  # N: unused 'type: ignore' comment
+
+[case testIgnoreScopeUnused3]
+# flags: --warn-unused-ignores
+(  # type: ignore  # N: unused 'type: ignore' comment
+    "IGNORE"
+    /
+    0  # type: ignore
+)  # type: ignore  # N: unused 'type: ignore' comment