Skip to content

Commit 45b9b1d

Browse files
authored
Add basic implementation for mypy: ignore comments (#22180)
Fixes #12358 Implementation is mostly straightforward, but there are few things: * This requires mypyc/ast_serialize#95, so tests will fail until we merge that. * We forgot to bump cache version to `6`. This is trivial however, functions now have a flag indicating if they have a type comment. * I found a pretty bad bug in native parser: parse errors cannot be type-ignored even if they are non-blocking. Again, it is a one line change, so not making a separate PR for this. Finally, there is one thing that bothers me a bit: we have few places where we mention words `type: ignore` in errors and notes. However, it would take a significant effort to reliably change them to `mypy: ignore` when needed. Essentially we would need to record provenance for each error code, and thread it through to many places (including to cache metas, for some technical reasons). So we have three options: 1. Do nothing about it. `mypy: ignore` really means mypy-specific type ignore, so it is fine to use `type: ignore` in error notes, if we actually have `mypy: ignore` written in code. 2. Actually implement all the work needed (not a fan of this option TBH). 3. Change the wording in those few errors/notes so that it is more neutral: i.e. say `Unused "ignore" comment` instead of `Unused "type: ignore" comment` etc. I am leaning towards option 3, but wanted to double-check before actually doing this.
1 parent 2a8d6ab commit 45b9b1d

9 files changed

Lines changed: 261 additions & 22 deletions

File tree

‎docs/source/error_codes.rst‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,43 @@ codes prefixed with ``mypy:`` will be validated:
5353
# OK: error code may be used by other type checker.
5454
y = 2 # type: ignore[something-else]
5555
56+
An alternative way to specify mypy-specific type ignores is to use
57+
``# mypy: ignore[one-code, other-code]``. Such form is equivalent to writing
58+
``# type: ignore[mypy:one-code, mypy:other-code]``. You can have both
59+
``# type: ignore[...]`` and ``# mypy: ignore[...]`` on the same line, in this
60+
case the error code lists will be merged.
61+
62+
Note that to allow better interoperability with other tools, mypy recognizes
63+
``# type: ignore`` and ``mypy: ignore`` in any position in a comment, including
64+
after another "magic" comments used by other tools. To temporarily disable
65+
an ignore, place it after an empty (i.e. whitespace only) comment. For example:
66+
67+
.. code-block:: python
68+
69+
from typing import Any
70+
71+
# No error, type ignore is active.
72+
x: str = 1 # noqa # pragma: no cover # type: ignore[assignment]
73+
74+
# Error: Incompatible types in assignment (expression has type "int", variable has type "str")
75+
y: str = 1 ## type: ignore[assignment]
76+
77+
# Error: Incompatible types in assignment (expression has type "int", variable has type "str")
78+
z: str = 1 # # type: ignore[assignment]
79+
80+
The only exception to this logic, is if there is no other code on a line
81+
except for the comment. In this case type ignores preceded by another comment
82+
are disabled. This is done to avoid spurious errors about unused ignores in
83+
a commented out block of code:
84+
85+
.. code-block:: python
86+
87+
# mypy: warn-unused-ignores
88+
89+
x: int = 1
90+
# OK, no unused-ignore error below.
91+
# x = "no way" # type: ignore[assignment]
92+
5693
Enabling/disabling specific error codes globally
5794
------------------------------------------------
5895

‎mypy/message_registry.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,9 @@ def with_additional_msg(self, info: str) -> ErrorMessage:
311311
codes.SYNTAX,
312312
)
313313
INVALID_TYPE_IGNORE: Final = ErrorMessage('Invalid "type: ignore" comment', codes.SYNTAX)
314+
NO_PREFIX_IN_MYPY_IGNORE: Final = (
315+
'Error codes in "mypy: ignore" should not be prefixed with "mypy:"'
316+
)
314317
TYPE_COMMENT_SYNTAX_ERROR_VALUE: Final = ErrorMessage(
315318
'Syntax error in type comment "{}"', codes.SYNTAX
316319
)

‎mypy/nativeparse.py‎

Lines changed: 101 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
)
4949
from mypy.errorcodes import error_codes
5050
from mypy.errors import CompileError
51+
from mypy.message_registry import NO_PREFIX_IN_MYPY_IGNORE
5152
from mypy.nodes import (
5253
ARG_KINDS,
5354
ARG_POS,
@@ -235,14 +236,25 @@ def native_parse(
235236
b,
236237
errors,
237238
ignores,
239+
mypy_ignores,
238240
import_bytes,
239241
is_partial_package,
240242
uses_template_strings,
241243
source_hash,
242244
mypy_comments,
243245
) = parse_to_binary_ast(filename, options, source, skip_function_bodies)
246+
244247
ignores, extra_errors = normalize_error_codes(ignores)
245248
errors.extend(extra_errors)
249+
mypy_ignores, extra_errors = normalize_error_codes(
250+
mypy_ignores, verify_all=True, prohibit_prefix=True
251+
)
252+
errors.extend(extra_errors)
253+
254+
ignores = merge_type_ignores(ignores, mypy_ignores)
255+
ignores, extra_errors = remove_duplicate_codes(ignores)
256+
errors.extend(extra_errors)
257+
246258
node = MypyFile([], [])
247259
node.path = filename
248260
node.raw_data = FileRawData(
@@ -258,29 +270,89 @@ def native_parse(
258270
return node, errors, ignores
259271

260272

261-
def normalize_error_codes(ignores: TypeIgnores) -> tuple[TypeIgnores, list[ParseError]]:
273+
def normalize_error_codes(
274+
ignores: TypeIgnores, verify_all: bool = False, prohibit_prefix: bool = False
275+
) -> tuple[TypeIgnores, list[ParseError]]:
262276
"""Remove mypy: prefix from error codes (if present)."""
263277
normalized = []
264278
extra_errors: list[ParseError] = []
265279
for line, codes in ignores:
266280
new_codes = []
267281
for c in codes:
268-
if not c.startswith("mypy:"):
269-
new_codes.append(c)
270-
continue
282+
prefixed = c.startswith("mypy:")
271283
c = c.removeprefix("mypy:")
272-
if c in error_codes:
273-
new_codes.append(c)
284+
# Always record unknown codes, so that type: ignore[unknown]
285+
# will not become blanket ignore.
286+
new_codes.append(c)
287+
if (prefixed or verify_all) and c not in error_codes:
288+
extra_errors.append(
289+
{
290+
"line": line,
291+
"column": -1,
292+
"message": f'Unrecognized error code "{c}"',
293+
"blocker": False,
294+
"code": "misc",
295+
}
296+
)
297+
if prefixed and prohibit_prefix:
298+
extra_errors.append(
299+
{
300+
"line": line,
301+
"column": -1,
302+
"message": NO_PREFIX_IN_MYPY_IGNORE,
303+
"blocker": False,
304+
"code": "misc",
305+
}
306+
)
307+
normalized.append((line, new_codes))
308+
return normalized, extra_errors
309+
310+
311+
def merge_type_ignores(ignores: TypeIgnores, mypy_ignores: TypeIgnores) -> TypeIgnores:
312+
"""Merge error codes specified in # type: ignore[...] and in # mypy: ignore[...]."""
313+
merged = []
314+
ignores_by_line = dict(ignores)
315+
mypy_ignores_by_line = dict(mypy_ignores)
316+
for line in sorted(ignores_by_line.keys() | mypy_ignores_by_line.keys()):
317+
codes = ignores_by_line.get(line)
318+
mypy_codes = mypy_ignores_by_line.get(line)
319+
if codes is not None and mypy_codes is None:
320+
new_codes = codes
321+
elif codes is None and mypy_codes is not None:
322+
new_codes = mypy_codes
323+
else:
324+
assert codes is not None and mypy_codes is not None
325+
if not codes or not mypy_codes:
326+
# If at least one ignore is blanket, make the merged ignore blanket.
327+
# We are not overly strict here, in case someone wants to quickly
328+
# silence everything.
329+
new_codes = []
330+
else:
331+
new_codes = codes + mypy_codes
332+
merged.append((line, new_codes))
333+
return merged
334+
335+
336+
def remove_duplicate_codes(ignores: TypeIgnores) -> tuple[TypeIgnores, list[ParseError]]:
337+
normalized = []
338+
extra_errors: list[ParseError] = []
339+
for line, codes in ignores:
340+
seen = set()
341+
new_codes = []
342+
for c in codes:
343+
if c in seen:
344+
extra_errors.append(
345+
{
346+
"line": line,
347+
"column": -1,
348+
"message": f'Duplicate error code "{c}"',
349+
"blocker": False,
350+
"code": "misc",
351+
}
352+
)
274353
continue
275-
extra_errors.append(
276-
{
277-
"line": line,
278-
"column": -1,
279-
"message": f'Unrecognized error code "{c}"',
280-
"blocker": False,
281-
"code": "misc",
282-
}
283-
)
354+
seen.add(c)
355+
new_codes.append(c)
284356
normalized.append((line, new_codes))
285357
return normalized, extra_errors
286358

@@ -327,7 +399,17 @@ def parse_to_binary_ast(
327399
options: Options,
328400
source: str | bytes | None = None,
329401
skip_function_bodies: bool = False,
330-
) -> tuple[bytes, list[ParseError], TypeIgnores, bytes, bool, bool, str, list[tuple[int, str]]]:
402+
) -> tuple[
403+
bytes,
404+
list[ParseError],
405+
TypeIgnores,
406+
TypeIgnores,
407+
bytes,
408+
bool,
409+
bool,
410+
str,
411+
list[tuple[int, str]],
412+
]:
331413
try:
332414
ast_bytes, errors, ignores, import_bytes, ast_data = ast_serialize.parse(
333415
filename,
@@ -337,7 +419,7 @@ def parse_to_binary_ast(
337419
platform=options.platform,
338420
always_true=options.always_true,
339421
always_false=options.always_false,
340-
cache_version=5,
422+
cache_version=7,
341423
)
342424
except ValueError as exc:
343425
if source is None:
@@ -366,6 +448,7 @@ def parse_to_binary_ast(
366448
ast_bytes,
367449
errors,
368450
ignores,
451+
ast_data["mypy_ignores"],
369452
import_bytes,
370453
ast_data["is_partial_package"],
371454
ast_data["uses_template_strings"],
@@ -796,6 +879,7 @@ def read_func_def(state: State, data: ReadBuffer) -> FuncDef:
796879
func_def = FuncDef(name, arguments, body, typ=typ, type_args=type_params)
797880
if is_async:
798881
func_def.is_coroutine = True
882+
func_def.has_type_comments = read_bool(data)
799883
read_loc(data, func_def)
800884
if type_params:
801885
state.check_min_version(

‎mypy/nodes.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,6 +1114,7 @@ class FuncDef(FuncItem, SymbolNode, Statement):
11141114
"docstring",
11151115
"deprecated",
11161116
"original_first_arg",
1117+
"has_type_comments",
11171118
)
11181119

11191120
__match_args__ = ("name", "arguments", "type", "body")
@@ -1156,6 +1157,9 @@ def __init__(
11561157
# We record this status to avoid multiple (similar but different) errors in case
11571158
# of partial types etc.
11581159
self.is_invalid_redefinition = False
1160+
# Record if the function definition has any type comments (including per-argument).
1161+
# This is only available with the native parser (and always False otherwise).
1162+
self.has_type_comments = False
11591163

11601164
@property
11611165
def name(self) -> str:

‎mypy/parse.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ def load_from_raw(
9595
# Report parse errors, this replicates the logic in parse().
9696
all_errors = raw_data.raw_errors + state.errors
9797
errors.set_file(fnam, module, options=options)
98+
errors.set_file_ignored_lines(fnam, tree.ignored_lines, options.ignore_errors)
9899
for error in all_errors:
99100
# Note we never raise in this function, so it should not be called in coordinator.
100101
report_parse_error(error, errors)

‎mypy/test/test_nativeparse.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,15 +268,15 @@ def locs(start_line: int, start_column: int, end_line: int, end_column: int) ->
268268

269269
def test_trivial_binary_data_from_file(self) -> None:
270270
with temp_source("print('hello')") as fnam:
271-
b, _, _, _, _, _, _, _ = parse_to_binary_ast(fnam, Options())
271+
b, _, _, _, _, _, _, _, _ = parse_to_binary_ast(fnam, Options())
272272
self._assert_trivial_binary_data(b)
273273

274274
def test_trivial_binary_data_from_string_source(self) -> None:
275-
b, _, _, _, _, _, _, _ = parse_to_binary_ast("", Options(), "print('hello')")
275+
b, _, _, _, _, _, _, _, _ = parse_to_binary_ast("", Options(), "print('hello')")
276276
self._assert_trivial_binary_data(b)
277277

278278
def test_trivial_binary_data_from_bytes_source(self) -> None:
279-
b, _, _, _, _, _, _, _ = parse_to_binary_ast("", Options(), b"print('hello')")
279+
b, _, _, _, _, _, _, _, _ = parse_to_binary_ast("", Options(), b"print('hello')")
280280
self._assert_trivial_binary_data(b)
281281

282282
def test_invalid_bytes_raises(self) -> None:

‎test-data/unit/check-deprecated.test‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ from typing_extensions import deprecated
4949
@deprecated("use f2 instead")
5050
def f() -> None: ...
5151

52-
f # E: function __main__.f is deprecated: use f2 instead # type: ignore[deprecated]
52+
f # E: function __main__.f is deprecated: use f2 instead
5353
f(1) # E: function __main__.f is deprecated: use f2 instead \
5454
# E: Too many arguments for "f"
5555
f[1] # E: function __main__.f is deprecated: use f2 instead \

‎test-data/unit/check-fastparse.test‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ for i in (1, 2, 3, 100): # type: str # type: ignore
7777
pass
7878
[builtins fixtures/tuple.pyi]
7979

80-
[case testFastParseVariableCommentThenIgnore]
80+
-- New parser has a different stance on this.
81+
[case testFastParseVariableCommentThenIgnore_old_parser]
8182
a="test" # type: int #comment # type: ignore # E: Incompatible types in assignment (expression has type "str", variable has type "int")
8283

8384
[case testFastParseProperty]

0 commit comments

Comments
 (0)