Skip to content

Commit

Permalink
Expression-scoped ignores in Python 3.8. (#6648)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
brandtbucher authored and ilevkivskyi committed Apr 11, 2019
1 parent 94746d6 commit 153536b
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 13 deletions.
37 changes: 26 additions & 11 deletions mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand All @@ -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


Expand Down Expand Up @@ -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:
Expand All @@ -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()
Expand All @@ -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)

Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
9 changes: 8 additions & 1 deletion mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions mypy/test/testcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
79 changes: 79 additions & 0 deletions test-data/unit/check-38.test
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 153536b

Please sign in to comment.