8000 Do not show unused-ignore errors in unreachable code, and make it a r… · python/mypy@61f6358 · GitHub
[go: up one dir, main page]
Skip to content

Commit 61f6358

Browse files
ilevkivskyiJukkaL
ilevkivskyi
and
JukkaL
authored
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>
1 parent 66602fc commit 61f6358

22 files changed

+407
-299
lines changed

‎docs/source/error_code_list2.rst

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,47 @@ silence the error:
347347
348348
async def g() -> None:
349349
_ = asyncio.create_task(f()) # No error
350+
351+
Check that ``# type: ignore`` comment is used [unused-ignore]
352+
-------------------------------------------------------------
353+
354+
If you use :option:`--enable-error-code unused-ignore <mypy --enable-error-code>`,
355+
or :option:`--warn-unused-ignores <mypy --warn-unused-ignores>`
356+
mypy generates an error if you don't use a ``# type: ignore`` comment, i.e. if
357+
there is a comment, but there would be no error generated by mypy on this line
358+
anyway.
359+
360+
Example:
361+
362+
.. code-block:: python
363+
364+
# Use "mypy --warn-unused-ignores ..."
365+
366+
def add(a: int, b: int) -> int:
367+
# Error: unused "type: ignore" comment
368+
return a + b # type: ignore
369+
370+
Note that due to a specific nature of this comment, the only way to selectively
371+
silence it, is to include the error code explicitly. Also note that this error is
372+
not shown if the ``# type: ignore`` is not used due to code being statically
373+
unreachable (e.g. due to platform or version checks).
374+
375+
Example:
376+
377+
.. code-block:: python
378+
379+
# Use "mypy --warn-unused-ignores ..."
380+
381+
import sys
382+
383+
try:
384+
# The "[unused-ignore]" is needed to get a clean mypy run
385+
# on both Python 3.8, and 3.9 where this module was added
386+
import graphlib # type: ignore[import,unused-ignore]
387+
except ImportError:
388+
pass
389+
390+
if sys.version_info >= (3, 9):
391+
# The following will not generate an error on either
392+
# Python 3.8, or Python 3.9
393+
42 + "testing..." # type: ignore

‎mypy/build.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2238,6 +2238,7 @@ def semantic_analysis_pass1(self) -> None:
22382238
analyzer = SemanticAnalyzerPreAnalysis()
22392239
with self.wrap_context():
22402240
analyzer.visit_file(self.tree, self.xpath, self.id, options)
2241+
self.manager.errors.set_unreachable_lines(self.xpath, self.tree.unreachable_lines)
22412242
# TODO: Do this while constructing the AST?
22422243
self.tree.names = SymbolTable()
22432244
if not self.tree.is_stub:
@@ -2572,7 +2573,10 @@ def dependency_lines(self) -> list[int]:
25722573
return [self.dep_line_map.get(dep, 1) for dep in self.dependencies + self.suppressed]
25732574

25742575
def generate_unused_ignore_notes(self) -> None:
2575-
if self.options.warn_unused_ignores:
2576+
if (
2577+
self.options.warn_unused_ignores
2578+
or codes.UNUSED_IGNORE in self.options.enabled_error_codes
2579+
) and codes.UNUSED_IGNORE not in self.options.disabled_error_codes:
25762580
# If this file was initially loaded from the cache, it may have suppressed
25772581
# dependencies due to imports with ignores on them. We need to generate
25782582
# those errors to avoid spuriously flagging them as unused ignores.

‎mypy/errorcodes.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,9 @@ def __str__(self) -> str:
221221
USED_BEFORE_DEF: Final[ErrorCode] = ErrorCode(
222222
"used-before-def", "Warn about variables that are used before they are defined", "General"
223223
)
224+
UNUSED_IGNORE: Final = ErrorCode(
225+
"unused-ignore", "Ensure that all type ignores are used", "General", default_enabled=False
226+
)
224227

225228

226229
# Syntax errors are often blocking.

‎mypy/errors.py

Lines changed: 13 additions & 2 deletions
1CF5
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ class Errors:
222222
# (path -> line -> error-codes)
223223
ignored_lines: dict[str, dict[int, list[str]]]
224224

225+
# Lines that are statically unreachable (e.g. due to platform/version check).
226+
unreachable_lines: dict[str, set[int]]
227+
225228
# Lines on which an error was actually ignored.
226229
used_ignored_lines: dict[str, dict[int, list[str]]]
227230

@@ -277,6 +280,7 @@ def initialize(self) -> None:
277280
self.import_ctx = []
278281
self.function_or_member = [None]
279282
self.ignored_lines = {}
283+
self.unreachable_lines = {}
280284
self.used_ignored_lines = defaultdict(lambda: defaultdict(list))
281285
self.ignored_files = set()
282286
self.only_once_messages = set()
@@ -325,6 +329,9 @@ def set_file_ignored_lines(
325329
if ignore_all:
326330
self.ignored_files.add(file)
327331

332+
def set_unreachable_lines(self, file: str, unreachable_lines: set[int]) -> None:
333+
self.unreachable_lines[file] = unreachable_lines
334+
328335
def current_target(self) -> str | None:
329336
"""Retrieves the current target from the associated scope.
330337
@@ -623,6 +630,10 @@ def generate_unused_ignore_errors(self, file: str) -> None:
623630
ignored_lines = self.ignored_lines[file]
624631
used_ignored_lines = self.used_ignored_lines[file]
625632
for line, ignored_codes in ignored_lines.items():
633+
if line in self.unreachable_lines[file]:
634+
continue
635+
if codes.UNUSED_IGNORE.code in ignored_codes:
636+
continue
626637
used_ignored_codes = used_ignored_lines[line]
627638
unused_ignored_codes = set(ignored_codes) - set(used_ignored_codes)
628639
# `ignore` is used
@@ -639,7 +650,7 @@ def generate_unused_ignore_errors(self, file: str) -> None:
639650
for unused in unused_ignored_codes:
640651
narrower = set(used_ignored_codes) & codes.sub_code_map[unused]
641652
if narrower:
642-
message += f", use narrower [{', '.join(narrower)}] instead of [{unused}]"
653+
message += f", use narrower [{', '.join(narrower)}] instead of [{unused}] code"
643654
# Don't use report since add_error_info will ignore the error!
644655
info = ErrorInfo(
645656
self.import_context(),
@@ -653,7 +664,7 @@ def generate_unused_ignore_errors(self, file: str) -> None:
653664
-1,
654665
"error",
655666
message,
656-
None,
667+
codes.UNUSED_IGNORE,
657668
False,
658669
False,
659670
False,

‎mypy/fastparse.py

Lines changed: 40 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,7 @@ def translate_stmt_list(
516516
codes.FILE.code
517517
)
518518
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
519+
self.set_block_lines(block, stmts)
519520
mark_block_unreachable(block)
520521
return [block]
521522

@@ -611,30 +612,38 @@ def from_comp_operator(self, op: ast3.cmpop) -> str:
611612
else:
612613
return op_name
613614

614-
def as_block(self, stmts: list[ast3.stmt], lineno: int) -> Block | None:
615+
def set_block_lines(self, b: Block, stmts: Sequence[ast3.stmt]) -> None:
616+
first, last = stmts[0], stmts[-1]
617+
b.line = first.lineno
618+
b.column = first.col_offset
619+
b.end_line = getattr(last, "end_lineno", None)
620+
b.end_column = getattr(last, "end_col_offset", None)
621+
if not b.body:
622+
return
623+
new_first = b.body[0]
624+
if isinstance(new_first, (Decorator, OverloadedFuncDef)):
625+
# Decorated function lines are different between Python versions.
626+
# copy the normalization we do for them to block first lines.
627+
b.line = new_first.line
628+
b.column = new_first.column
629+
630+
def as_block(self, stmts: list[ast3.stmt]) -> Block | None:
615631
b = None
616632
if stmts:
617633
b = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
618-
b.set_line(lineno)
634+
self.set_block_lines(b, stmts)
619635
return b
620636

621637
def as_required_block(
622-
self,
623-
stmts: list[ast3.stmt],
624-
lineno: int,
625-
*,
626-
can_strip: bool = False,
627-
is_coroutine: bool = False,
638+
self, stmts: list[ast3.stmt], *, can_strip: bool = False, is_coroutine: bool = False
628639
) -> Block:
629640
assert stmts # must be non-empty
630641
b = Block(
631642
self.fix_function_overloads(
632643
self.translate_stmt_list(stmts, can_strip=can_strip, is_coroutine=is_coroutine)
633644
)
634645
)
635-
# TODO: in most call sites line is wrong (includes first line of enclosing statement)
636-
# TODO: also we need to set the column, and the end position here.
637-
b.set_line(lineno)
646+
self.set_block_lines(b, stmts)
638647
return b
639648

640649
def fix_function_overloads(self, stmts: list[Statement]) -> list[Statement]:
@@ -1023,7 +1032,7 @@ def do_func_def(
10231032

10241033
self.class_and_function_stack.pop()
10251034
self.class_and_function_stack.append("F")
1026-
body = self.as_required_block(n.body, lineno, can_strip=True, is_coroutine=is_coroutine)
1035+
body = self.as_required_block(n.body, can_strip=True, is_coroutine=is_coroutine)
10271036
func_def = FuncDef(n.name, args, body, func_type)
10281037
if isinstance(func_def.type, CallableType):
10291038
# semanal.py does some in-place modifications we want to avoid
@@ -1052,9 +1061,6 @@ def do_func_def(
10521061
func_def.is_decorated = True
10531062
func_def.deco_line = deco_line
10541063
func_def.set_line(lineno, n.col_offset, end_line, end_column)
1055-
# Set the line again after we updated it (to make value same in Python 3.7/3.8)
1056-
# Note that TODOs in as_required_block() apply here as well.
1057-
func_def.body.set_line(lineno)
10581064

10591065
deco = Decorator(func_def, self.translate_expr_list(n.decorator_list), var)
10601066
first = n.decorator_list[0]
@@ -1165,7 +1171,7 @@ def visit_ClassDef(self, n: ast3.ClassDef) -> ClassDef:
11651171

11661172
cdef = ClassDef(
11671173
n.name,
1168-
self.as_required_block(n.body, n.lineno),
1174+
self.as_required_block(n.body),
11691175
None,
11701176
self.translate_expr_list(n.bases),
11711177
metaclass=dict(keywords).get("metaclass"),
@@ -1237,8 +1243,8 @@ def visit_For(self, n: ast3.For) -> ForStmt:
12371243
node = ForStmt(
12381244
self.visit(n.target),
12391245
self.visit(n.iter),
1240-
self.as_required_block(n.body, n.lineno),
1241-
self.as_block(n.orelse, n.lineno),
1246+
self.as_required_block(n.body),
1247+
self.as_block(n.orelse),
12421248
target_type,
12431249
)
12441250
return self.set_line(node, n)
@@ -1249,8 +1255,8 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
12491255
node = ForStmt(
12501256
self.visit(n.target),
12511257
self.visit(n.iter),
1252-
self.as_required_block(n.body, n.lineno),
1253-
self.as_block(n.orelse, n.lineno),
1258+
self.as_required_block(n.body),
1259+
self.as_block(n.orelse),
12541260
target_type,
12551261
)
12561262
node.is_async = True
@@ -1259,19 +1265,14 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
12591265
# While(expr test, stmt* body, stmt* orelse)
12601266
def visit_While(self, n: ast3.While) -> WhileStmt:
12611267
node = WhileStmt(
1262-
self.visit(n.test),
1263-
self.as_required_block(n.body, n.lineno),
1264-
self.as_block(n.orelse, n.lineno),
1268+
self.visit(n.test), self.as_required_block(n.body), self.as_block(n.orelse)
12651269
)
12661270
return self.set_line(node, n)
12671271

12681272
# If(expr test, stmt* body, stmt* orelse)
12691273
def visit_If(self, n: ast3.If) -> IfStmt:
1270-
lineno = n.lineno
12711274
node = IfStmt(
1272-
[self.visit(n.test)],
1273-
[self.as_required_block(n.body, lineno)],
1274-
self.as_block(n.orelse, lineno),
1275+
[self.visit(n.test)], [self.as_required_block(n.body)], self.as_block(n.orelse)
12751276
)
12761277
return self.set_line(node, n)
12771278

@@ -1281,7 +1282,7 @@ def visit_With(self, n: ast3.With) -> WithStmt:
12811282
node = WithStmt(
12821283
[self.visit(i.context_expr) for i in n.items],
12831284
[self.visit(i.optional_vars) for i in n.items],
1284-
self.as_required_block(n.body, n.lineno),
1285+
self.as_required_block(n.body),
12851286
target_type,
12861287
)
12871288
return self.set_line(node, n)
@@ -1292,7 +1293,7 @@ def visit_AsyncWith(self, n: ast3.AsyncWith) -> WithStmt:
12921293
s = WithStmt(
12931294
[self.visit(i.context_expr) for i in n.items],
12941295
[self.visit(i.optional_vars) for i in n.items],
1295-
self.as_required_block(n.body, n.lineno),
1296+
self.as_required_block(n.body),
12961297
target_type,
12971298
)
12981299
s.is_async = True
@@ -1309,15 +1310,15 @@ def visit_Try(self, n: ast3.Try) -> TryStmt:
13091310
self.set_line(NameExpr(h.name), h) if h.name is not None else None for h in n.handlers
13101311
]
13111312
types = [self.visit(h.type) for h in n.handlers]
1312-
handlers = [self.as_required_block(h.body, h.lineno) for h in n.handlers]
1313+
handlers = [self.as_required_block(h.body) for h in n.handlers]
13131314

13141315
node = TryStmt(
1315-
self.as_required_block(n.body, n.lineno),
1316+
self.as_required_block(n.body),
13161317
vs,
13171318
types,
13181319
handlers,
1319-
self.as_block(n.orelse, n.lineno),
1320-
self.as_block(n.finalbody, n.lineno),
1320+
self.as_block(n.orelse),
1321+
self.as_block(n.finalbody),
13211322
)
13221323
return self.set_line(node, n)
13231324

@@ -1326,15 +1327,15 @@ def visit_TryStar(self, n: TryStar) -> TryStmt:
13261327
self.set_line(NameExpr(h.name), h) if h.name is not None else None for h in n.handlers
13271328
]
13281329
types = [self.visit(h.type) for h in n.handlers]
1329-
handlers = [self.as_required_block(h.body, h.lineno) for h in n.handlers]
1330+
handlers = [self.as_required_block(h.body) for h in n.handlers]
13301331

13311332
node = TryStmt(
1332-
self.as_required_block(n.body, n.lineno),
1333+
self.as_required_block(n.body),
13331334
vs,
13341335
types,
13351336
handlers,
1336-
self.as_block(n.orelse, n.lineno),
1337-
self.as_block(n.finalbody, n.lineno),
1337+
self.as_block(n.orelse),
1338+
self.as_block(n.finalbody),
13381339
)
13391340
node.is_star = True
13401341
return self.set_line(node, n)
@@ -1469,9 +1470,7 @@ def visit_Lambda(self, n: ast3.Lambda) -> LambdaExpr:
14691470
body.col_offset = n.body.col_offset
14701471

14711472
self.class_and_function_stack.append("L")
1472-
e = LambdaExpr(
1473-
self.transform_args(n.args, n.lineno), self.as_required_block([body], n.lineno)
1474-
)
1473+
e = LambdaExpr(self.transform_args(n.args, n.lineno), self.as_required_block([body]))
14751474
self.class_and_function_stack.pop()
14761475
e.set_line(n.lineno, n.col_offset) # Overrides set_line -- can't use self.set_line
14771476
return e
@@ -1743,7 +1742,7 @@ def visit_Match(self, n: Match) -> MatchStmt:
17431742
self.visit(n.subject),
17441743
[self.visit(c.pattern) for c in n.cases],
17451744
[self.visit(c.guard) for c in n.cases],
1746-
[self.as_required_block(c.body, n.lineno) for c in n.cases],
1745+
[self.as_required_block(c.body) for c in n.cases],
17471746
)
17481747
return self.set_line(node, n)
17491748

‎mypy/nodes.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ class MypyFile(SymbolNode):
287287
"names",
288288
"imports",
289289
"ignored_lines",
290+
"unreachable_lines",
290291
"is_stub",
291292
"is_cache_skeleton",
292293
"is_partial_stub_package",
@@ -313,6 +314,8 @@ class MypyFile(SymbolNode):
313314
# If the value is empty, ignore all errors; otherwise, the list contains all
314315
# error codes to ignore.
315316
ignored_lines: dict[int, list[str]]
317+
# Lines that are statically unreachable (e.g. due to platform/version check).
318+
unreachable_lines: set[int]
316319
# Is this file represented by a stub file (.pyi)?
317320
is_stub: bool
318321
# Is this loaded from the cache and thus missing the actual body of the file?
@@ -345,6 +348,7 @@ def __init__(
345348
self.ignored_lines = ignored_lines
346349
else:
347350
self.ignored_lines = {}
351+
self.unreachable_lines = set()
348352

349353
self.path = ""
350354
self.is_stub = False

0 commit comments

Comments
 (0)
0