Fix class/def line reporting and ignoring for 3.8+ (#6753)

Fixes #3871. This change fixes a few things concerning class/function definition line numbers.

For users running mypy with Python 3.8+:
- Function definitions are now reported on the correct line, rather than the line of the first decorator plus the number of decorators.
- Class definitions are now reported on the correct line, rather than the line of the first decorator.
- **These changes are fully backward compatible with respect to `# type: ignore` lines**. In other words, existing comments will _not_ need to be moved. This is accomplished by defining an ignore "scope" just like we added to expressions with #6648. See the recent discussion at #3871 for reasoning.

For other users (running mypy with Python 3.7 or earlier):
- Class definition lines are reported by using the same decorator-offset estimate as functions. This is both more accurate, and necessary to get 15 or so tests to pass for both pre- and post-3.8 versions. This seems fine, since there [doesn't appear](#6539 (comment)) to be a compelling reason why it was different in the first place.
- **This change is also backward-compatible with existing ignores, as above.**

About 15 tests had to have their error messages / nodes moved down one line, and 4 new regression tests have been added to `check-38.test`.

Author: brandtbucher

mypy/errors.py

@@ -287,9 +287,14 @@ def add_error_info(self, info: ErrorInfo) -> None:
         file, line, end_line = info.origin
         if not info.blocker:  # Blockers cannot be ignored
             if file in self.ignored_lines:
+                # It's okay if end_line is *before* line.
+                # Function definitions do this, for example, because the correct
+                # error reporting line is at the *end* of the ignorable range
+                # (for compatibility reasons). If so, just flip 'em!
+                if end_line < line:
+                    line, end_line = end_line, line
                 # 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.
+                # line == end_line for most nodes, 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.

mypy/fastparse.py

@@ -524,13 +524,18 @@ def do_func_def(self, n: Union[ast3.FunctionDef, ast3.AsyncFunctionDef],
                 # Before 3.8, [typed_]ast the line number points to the first decorator.
                 # In 3.8, it points to the 'def' line, where we want it.
                 lineno += len(n.decorator_list)
+                end_lineno = None
+            else:
+                # Set end_lineno to the old pre-3.8 lineno, in order to keep
+                # existing "# type: ignore" comments working:
+                end_lineno = n.decorator_list[0].lineno + len(n.decorator_list)
 
             var = Var(func_def.name())
             var.is_ready = False
             var.set_line(lineno)
 
             func_def.is_decorated = True
-            func_def.set_line(lineno, n.col_offset)
+            func_def.set_line(lineno, n.col_offset, end_lineno)
             func_def.body.set_line(lineno)  # TODO: Why?
 
             deco = Decorator(func_def, self.translate_expr_list(n.decorator_list), var)
@@ -633,15 +638,15 @@ def visit_ClassDef(self, n: ast3.ClassDef) -> ClassDef:
                         metaclass=dict(keywords).get('metaclass'),
                         keywords=keywords)
         cdef.decorators = self.translate_expr_list(n.decorator_list)
-        if n.decorator_list and sys.version_info >= (3, 8):
-            # Before 3.8, n.lineno points to the first decorator; in
-            # 3.8, it points to the 'class' statement.  We always make
-            # it point to the first decorator.  (The node structure
-            # here is different than for decorated functions.)
-            cdef.line = n.decorator_list[0].lineno
-            cdef.column = n.col_offset
+        # Set end_lineno to the old mypy 0.700 lineno, in order to keep
+        # existing "# type: ignore" comments working:
+        if sys.version_info < (3, 8):
+            cdef.line = n.lineno + len(n.decorator_list)
+            cdef.end_line = n.lineno
         else:
-            self.set_line(cdef, n)
+            cdef.line = n.lineno
+            cdef.end_line = n.decorator_list[0].lineno if n.decorator_list else None
+        cdef.column = n.col_offset
         self.class_and_function_stack.pop()
         return cdef
 

mypy/fastparse2.py

@@ -529,7 +529,9 @@ def visit_ClassDef(self, n: ast27.ClassDef) -> ClassDef:
                         self.translate_expr_list(n.bases),
                         metaclass=None)
         cdef.decorators = self.translate_expr_list(n.decorator_list)
-        self.set_line(cdef, n)
+        cdef.line = n.lineno + len(n.decorator_list)
+        cdef.column = n.col_offset
+        cdef.end_line = n.lineno
         self.class_and_function_stack.pop()
         return cdef
 

mypy/nodes.py

@@ -29,7 +29,10 @@ def __init__(self, line: int = -1, column: int = -1) -> None:
         self.column = column
         self.end_line = None  # type: Optional[int]
 
-    def set_line(self, target: Union['Context', int], column: Optional[int] = None) -> None:
+    def set_line(self,
+                 target: Union['Context', int],
+                 column: Optional[int] = None,
+                 end_line: Optional[int] = None) -> None:
         """If target is a node, pull line (and column) information
         into this node. If column is specified, this will override any column
         information coming from a node.
@@ -44,6 +47,9 @@ def set_line(self, target: Union['Context', int], column: Optional[int] = None)
         if column is not None:
             self.column = column
 
+        if end_line is not None:
+            self.end_line = end_line
+
     def get_line(self) -> int:
         """Don't use. Use x.line."""
         return self.line
@@ -534,13 +540,16 @@ def __init__(self,
         self.initializer = initializer
         self.kind = kind  # must be an ARG_* constant
 
-    def set_line(self, target: Union[Context, int], column: Optional[int] = None) -> None:
-        super().set_line(target, column)
+    def set_line(self,
+                 target: Union[Context, int],
+                 column: Optional[int] = None,
+                 end_line: Optional[int] = None) -> None:
+        super().set_line(target, column, end_line)
 
         if self.initializer:
-            self.initializer.set_line(self.line, self.column)
+            self.initializer.set_line(self.line, self.column, self.end_line)
 
-        self.variable.set_line(self.line, self.column)
+        self.variable.set_line(self.line, self.column, self.end_line)
 
 
 FUNCITEM_FLAGS = FUNCBASE_FLAGS + [
@@ -595,10 +604,13 @@ def __init__(self,
     def max_fixed_argc(self) -> int:
         return self.max_pos
 
-    def set_line(self, target: Union[Context, int], column: Optional[int] = None) -> None:
-        super().set_line(target, column)
+    def set_line(self,
+                 target: Union[Context, int],
+                 column: Optional[int] = None,
+                 end_line: Optional[int] = None) -> None:
+        super().set_line(target, column, end_line)
         for arg in self.arguments:
-            arg.set_line(self.line, self.column)
+            arg.set_line(self.line, self.column, self.end_line)
 
     def is_dynamic(self) -> bool:
         return self.type is None

test-data/unit/check-38.test

@@ -1,3 +1,31 @@
+[case testDecoratedClassLine]
+def d(c): ...
+@d
+
+class C: ...
+class C: ...  # E: Name 'C' already defined on line 4
+
+[case testDecoratedFunctionLine]
+# flags: --disallow-untyped-defs
+def d(f): ...  # type: ignore
+@d
+
+def f(): ...  # E: Function is missing a type annotation
+
+[case testIgnoreDecoratedFunction1]
+# flags: --disallow-untyped-defs --warn-unused-ignores
+def d(f): ...  # type: ignore
+@d
+# type: ignore
+def f(): ...  # type: ignore  # E: unused 'type: ignore' comment
+
+[case testIgnoreDecoratedFunction2]
+# flags: --disallow-untyped-defs
+def d(f): ...  # type: ignore
+@d
+
+def f(): ...  # type: ignore
+
 [case testIgnoreScopeIssue1032]
 def f(a: int): ...
 f(

test-data/unit/check-attr.test

@@ -74,8 +74,8 @@ A(1, [2], '3', 4, 5)  # E: Too many arguments for "A"
 [case testAttrsUntypedNoUntypedDefs]
 # flags: --disallow-untyped-defs
 import attr
-@attr.s    # E: Function is missing a type annotation for one or more arguments
-class A:
+@attr.s
+class A:  # E: Function is missing a type annotation for one or more arguments
     a = attr.ib()  # E: Need type annotation for 'a'
     _b = attr.ib()  # E: Need type annotation for '_b'
     c = attr.ib(18)  # E: Need type annotation for 'c'
@@ -785,8 +785,8 @@ import attr
 @attr.s
 class Good(object):
     pass
-@attr.s  # E: attrs only works with new-style classes
-class Bad:
+@attr.s
+class Bad:  # E: attrs only works with new-style classes
     pass
 [builtins_py2 fixtures/bool.pyi]
 
@@ -1043,8 +1043,8 @@ reveal_type(B)  # E: Revealed type is 'def (x: __main__.C) -> __main__.B'
 # flags: --disallow-untyped-defs
 import attr
 
-@attr.s  # E: Function is missing a type annotation for one or more arguments
-class B:
+@attr.s
+class B:  # E: Function is missing a type annotation for one or more arguments
     x = attr.ib()  # E: Need type annotation for 'x'
 
 reveal_type(B)  # E: Revealed type is 'def (x: Any) -> __main__.B'

test-data/unit/check-classes.test

@@ -4850,19 +4850,19 @@ class C3(six.with_metaclass(A)): pass  # E: Metaclasses not inheriting from 'typ
                        # E: Metaclasses not inheriting from 'type' are not supported
 class D3(A): pass
 class C4(six.with_metaclass(M), metaclass=M): pass  # E: Multiple metaclass definitions
-@six.add_metaclass(M)  # E: Multiple metaclass definitions
-class D4(metaclass=M): pass
+@six.add_metaclass(M)
+class D4(metaclass=M): pass  # E: Multiple metaclass definitions
 class C5(six.with_metaclass(f())): pass  # E: Dynamic metaclass not supported for 'C5'
 @six.add_metaclass(f())  # E: Dynamic metaclass not supported for 'D5'
 class D5: pass
 
-@six.add_metaclass(M)  # E: Multiple metaclass definitions
-class CD(six.with_metaclass(M)): pass
+@six.add_metaclass(M)
+class CD(six.with_metaclass(M)): pass  # E: Multiple metaclass definitions
 
 class M1(type): pass
 class Q1(metaclass=M1): pass
-@six.add_metaclass(M)  # E: Inconsistent metaclass structure for 'CQA'
-class CQA(Q1): pass
+@six.add_metaclass(M)
+class CQA(Q1): pass  # E: Inconsistent metaclass structure for 'CQA'
 class CQW(six.with_metaclass(M, Q1)): pass  # E: Inconsistent metaclass structure for 'CQW'
 
 [case testSixMetaclassErrors_python2]
@@ -4972,20 +4972,20 @@ def decorate_forward_ref() -> Callable[[Type[A]], Type[A]]:
 @decorate(11)
 class A: pass
 
-@decorate  # E: Argument 1 to "decorate" has incompatible type "Type[A2]"; expected "int"
-class A2: pass
+@decorate
+class A2: pass  # E: Argument 1 to "decorate" has incompatible type "Type[A2]"; expected "int"
 
 [case testClassDecoratorIncorrect]
 def not_a_class_decorator(x: int) -> int: ...
-@not_a_class_decorator(7)  # E: "int" not callable
-class A3: pass
+@not_a_class_decorator(7)
+class A3: pass  # E: "int" not callable
 
 not_a_function = 17
 @not_a_function()  # E: "int" not callable
 class B: pass
 
-@not_a_function    # E: "int" not callable
-class B2: pass
+@not_a_function
+class B2: pass    # E: "int" not callable
 
 b = object()
 @b.nothing         # E: "object" has no attribute "nothing"

test-data/unit/check-dataclasses.test

@@ -387,8 +387,8 @@ app1 >= app3
 # flags: --python-version 3.6
 from dataclasses import dataclass
 
-@dataclass(eq=False, order=True)  # E: eq must be True if order is True
-class Application:
+@dataclass(eq=False, order=True)
+class Application:  # E: eq must be True if order is True
    ...
 
 [builtins fixtures/list.pyi]

test-data/unit/check-incremental.test

@@ -3267,9 +3267,9 @@ def foo() -> None:
     reveal_type(A)
 [builtins fixtures/list.pyi]
 [out1]
-main:8: error: Revealed type is 'def (x: builtins.str) -> __main__.A@5'
+main:8: error: Revealed type is 'def (x: builtins.str) -> __main__.A@6'
 [out2]
-main:8: error: Revealed type is 'def (x: builtins.str) -> __main__.A@5'
+main:8: error: Revealed type is 'def (x: builtins.str) -> __main__.A@6'
 
 [case testAttrsIncrementalConverterInSubmoduleForwardRef]
 from a.a import A

test-data/unit/check-protocols.test

@@ -1486,8 +1486,8 @@ class C(Protocol):
 [case testSimpleRuntimeProtocolCheck]
 from typing import Protocol, runtime
 
-@runtime # E: @runtime can only be used with protocol classes
-class C:
+@runtime
+class C:  # E: @runtime can only be used with protocol classes
     pass
 
 class P(Protocol):

test-data/unit/parse-python2.test

@@ -754,7 +754,7 @@ class C:
     pass
 [out]
 MypyFile:1(
-  ClassDef:1(
+  ClassDef:2(
     C
     Decorators(
       CallExpr:1(

test-data/unit/parse.test

@@ -2778,12 +2778,12 @@ class X: pass
 class Z: pass
 [out]
 MypyFile:1(
-  ClassDef:1(
+  ClassDef:2(
     X
     Decorators(
       NameExpr(foo))
     PassStmt:2())
-  ClassDef:3(
+  ClassDef:5(
     Z
     Decorators(
       CallExpr:3(

test-data/unit/semanal-classes.test

@@ -524,7 +524,7 @@ class A: pass
 [out]
 MypyFile:1(
   Import:1(typing)
-  ClassDef:2(
+  ClassDef:3(
     A
     Decorators(
       NameExpr(object [builtins.object]))

test-data/unit/semanal-types.test

@@ -1375,7 +1375,7 @@ class S: pass
 [out]
 MypyFile:1(
   ImportFrom:1(typing, [_promote])
-  ClassDef:2(
+  ClassDef:3(
     S
     Promote(builtins.str)
     Decorators(