Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not show unused-ignore errors in unreachable code, and make it a real error code #15164

Merged
merged 7 commits into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -347,3 +347,47 @@ silence the error:

async def g() -> None:
_ = asyncio.create_task(f()) # No error

Check that ``# type: ignore`` comment is used [unused-ignore]
-------------------------------------------------------------

If you use :option:`--enable-error-code unused-ignore <mypy --enable-error-code>`,
or :option:`--warn-unused-ignores <mypy --warn-unused-ignores>`
mypy generates an error if you don't use a ``# type: ignore`` comment, i.e. if
there is a comment, but there would be no error generated by mypy on this line
anyway.

Example:

.. code-block:: python

# Use "mypy --warn-unused-ignores ..."

def add(a: int, b: int) -> int:
# Error: unused "type: ignore" comment
return a + b # type: ignore

Note that due to a specific nature of this comment, the only way to selectively
silence it, is to include the error code explicitly. Also note that this error is
not shown if the ``# type: ignore`` is not used due to code being statically
unreachable (e.g. due to platform or version checks).

Example:

.. code-block:: python

# Use "mypy --warn-unused-ignores ..."

import sys

try:
# The "[unused-ignore]" is needed to get a clean mypy run
# on both Python 3.8, and 3.9 where this module was added
import graphlib # type: ignore[import,unused-ignore]
except ImportError:
pass

if sys.version_info >= (3, 9):
# The following will not generate an error on either
# Python 3.8, or Python 3.9
42 + "testing..." # type: ignore
6 changes: 5 additions & 1 deletion mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -2238,6 +2238,7 @@ def semantic_analysis_pass1(self) -> None:
analyzer = SemanticAnalyzerPreAnalysis()
with self.wrap_context():
analyzer.visit_file(self.tree, self.xpath, self.id, options)
self.manager.errors.set_unreachable_lines(self.xpath, self.tree.unreachable_lines)
# TODO: Do this while constructing the AST?
self.tree.names = SymbolTable()
if not self.tree.is_stub:
Expand Down Expand Up @@ -2572,7 +2573,10 @@ def dependency_lines(self) -> list[int]:
return [self.dep_line_map.get(dep, 1) for dep in self.dependencies + self.suppressed]

def generate_unused_ignore_notes(self) -> None:
if self.options.warn_unused_ignores:
if (
self.options.warn_unused_ignores
or codes.UNUSED_IGNORE in self.options.enabled_error_codes
) and codes.UNUSED_IGNORE not in self.options.disabled_error_codes:
# If this file was initially loaded from the cache, it may have suppressed
# dependencies due to imports with ignores on them. We need to generate
# those errors to avoid spuriously flagging them as unused ignores.
Expand Down
3 changes: 3 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ def __str__(self) -> str:
USED_BEFORE_DEF: Final[ErrorCode] = ErrorCode(
"used-before-def", "Warn about variables that are used before they are defined", "General"
)
UNUSED_IGNORE: Final = ErrorCode(
"unused-ignore", "Ensure that all type ignores are used", "General", default_enabled=False
)


# Syntax errors are often blocking.
Expand Down
15 changes: 13 additions & 2 deletions mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ class Errors:
# (path -> line -> error-codes)
ignored_lines: dict[str, dict[int, list[str]]]

# Lines that are statically unreachable (e.g. due to platform/version check).
unreachable_lines: dict[str, set[int]]

# Lines on which an error was actually ignored.
used_ignored_lines: dict[str, dict[int, list[str]]]

Expand Down Expand Up @@ -277,6 +280,7 @@ def initialize(self) -> None:
self.import_ctx = []
self.function_or_member = [None]
self.ignored_lines = {}
self.unreachable_lines = {}
self.used_ignored_lines = defaultdict(lambda: defaultdict(list))
self.ignored_files = set()
self.only_once_messages = set()
Expand Down Expand Up @@ -325,6 +329,9 @@ def set_file_ignored_lines(
if ignore_all:
self.ignored_files.add(file)

def set_unreachable_lines(self, file: str, unreachable_lines: set[int]) -> None:
self.unreachable_lines[file] = unreachable_lines

def current_target(self) -> str | None:
"""Retrieves the current target from the associated scope.

Expand Down Expand Up @@ -623,6 +630,10 @@ def generate_unused_ignore_errors(self, file: str) -> None:
ignored_lines = self.ignored_lines[file]
used_ignored_lines = self.used_ignored_lines[file]
for line, ignored_codes in ignored_lines.items():
if line in self.unreachable_lines[file]:
continue
if codes.UNUSED_IGNORE.code in ignored_codes:
continue
used_ignored_codes = used_ignored_lines[line]
unused_ignored_codes = set(ignored_codes) - set(used_ignored_codes)
# `ignore` is used
Expand All @@ -639,7 +650,7 @@ def generate_unused_ignore_errors(self, file: str) -> None:
for unused in unused_ignored_codes:
narrower = set(used_ignored_codes) & codes.sub_code_map[unused]
if narrower:
message += f", use narrower [{', '.join(narrower)}] instead of [{unused}]"
message += f", use narrower [{', '.join(narrower)}] instead of [{unused}] code"
# Don't use report since add_error_info will ignore the error!
info = ErrorInfo(
self.import_context(),
Expand All @@ -653,7 +664,7 @@ def generate_unused_ignore_errors(self, file: str) -> None:
-1,
"error",
message,
None,
codes.UNUSED_IGNORE,
False,
False,
False,
Expand Down
81 changes: 40 additions & 41 deletions mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ def translate_stmt_list(
codes.FILE.code
)
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
self.set_block_lines(block, stmts)
mark_block_unreachable(block)
return [block]

Expand Down Expand Up @@ -611,30 +612,38 @@ def from_comp_operator(self, op: ast3.cmpop) -> str:
else:
return op_name

def as_block(self, stmts: list[ast3.stmt], lineno: int) -> Block | None:
def set_block_lines(self, b: Block, stmts: Sequence[ast3.stmt]) -> None:
first, last = stmts[0], stmts[-1]
b.line = first.lineno
b.column = first.col_offset
b.end_line = getattr(last, "end_lineno", None)
b.end_column = getattr(last, "end_col_offset", None)
if not b.body:
return
new_first = b.body[0]
if isinstance(new_first, (Decorator, OverloadedFuncDef)):
# Decorated function lines are different between Python versions.
# copy the normalization we do for them to block first lines.
b.line = new_first.line
b.column = new_first.column

def as_block(self, stmts: list[ast3.stmt]) -> Block | None:
b = None
if stmts:
b = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
b.set_line(lineno)
self.set_block_lines(b, stmts)
return b

def as_required_block(
self,
stmts: list[ast3.stmt],
lineno: int,
*,
can_strip: bool = False,
is_coroutine: bool = False,
self, stmts: list[ast3.stmt], *, can_strip: bool = False, is_coroutine: bool = False
) -> Block:
assert stmts # must be non-empty
b = Block(
self.fix_function_overloads(
self.translate_stmt_list(stmts, can_strip=can_strip, is_coroutine=is_coroutine)
)
)
# TODO: in most call sites line is wrong (includes first line of enclosing statement)
# TODO: also we need to set the column, and the end position here.
b.set_line(lineno)
self.set_block_lines(b, stmts)
return b

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

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

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

cdef = ClassDef(
n.name,
self.as_required_block(n.body, n.lineno),
self.as_required_block(n.body),
None,
self.translate_expr_list(n.bases),
metaclass=dict(keywords).get("metaclass"),
Expand Down Expand Up @@ -1237,8 +1243,8 @@ def visit_For(self, n: ast3.For) -> ForStmt:
node = ForStmt(
self.visit(n.target),
self.visit(n.iter),
self.as_required_block(n.body, n.lineno),
self.as_block(n.orelse, n.lineno),
self.as_required_block(n.body),
self.as_block(n.orelse),
target_type,
)
return self.set_line(node, n)
Expand All @@ -1249,8 +1255,8 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
node = ForStmt(
self.visit(n.target),
self.visit(n.iter),
self.as_required_block(n.body, n.lineno),
self.as_block(n.orelse, n.lineno),
self.as_required_block(n.body),
self.as_block(n.orelse),
target_type,
)
node.is_async = True
Expand All @@ -1259,19 +1265,14 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
# While(expr test, stmt* body, stmt* orelse)
def visit_While(self, n: ast3.While) -> WhileStmt:
node = WhileStmt(
self.visit(n.test),
self.as_required_block(n.body, n.lineno),
self.as_block(n.orelse, n.lineno),
self.visit(n.test), self.as_required_block(n.body), self.as_block(n.orelse)
)
return self.set_line(node, n)

# If(expr test, stmt* body, stmt* orelse)
def visit_If(self, n: ast3.If) -> IfStmt:
lineno = n.lineno
node = IfStmt(
[self.visit(n.test)],
[self.as_required_block(n.body, lineno)],
self.as_block(n.orelse, lineno),
[self.visit(n.test)], [self.as_required_block(n.body)], self.as_block(n.orelse)
)
return self.set_line(node, n)

Expand All @@ -1281,7 +1282,7 @@ def visit_With(self, n: ast3.With) -> WithStmt:
node = WithStmt(
[self.visit(i.context_expr) for i in n.items],
[self.visit(i.optional_vars) for i in n.items],
self.as_required_block(n.body, n.lineno),
self.as_required_block(n.body),
target_type,
)
return self.set_line(node, n)
Expand All @@ -1292,7 +1293,7 @@ def visit_AsyncWith(self, n: ast3.AsyncWith) -> WithStmt:
s = WithStmt(
[self.visit(i.context_expr) for i in n.items],
[self.visit(i.optional_vars) for i in n.items],
self.as_required_block(n.body, n.lineno),
self.as_required_block(n.body),
target_type,
)
s.is_async = True
Expand All @@ -1309,15 +1310,15 @@ def visit_Try(self, n: ast3.Try) -> TryStmt:
self.set_line(NameExpr(h.name), h) if h.name is not None else None for h in n.handlers
]
types = [self.visit(h.type) for h in n.handlers]
handlers = [self.as_required_block(h.body, h.lineno) for h in n.handlers]
handlers = [self.as_required_block(h.body) for h in n.handlers]

node = TryStmt(
self.as_required_block(n.body, n.lineno),
self.as_required_block(n.body),
vs,
types,
handlers,
self.as_block(n.orelse, n.lineno),
self.as_block(n.finalbody, n.lineno),
self.as_block(n.orelse),
self.as_block(n.finalbody),
)
return self.set_line(node, n)

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

node = TryStmt(
self.as_required_block(n.body, n.lineno),
self.as_required_block(n.body),
vs,
types,
handlers,
self.as_block(n.orelse, n.lineno),
self.as_block(n.finalbody, n.lineno),
self.as_block(n.orelse),
self.as_block(n.finalbody),
)
node.is_star = True
return self.set_line(node, n)
Expand Down Expand Up @@ -1469,9 +1470,7 @@ def visit_Lambda(self, n: ast3.Lambda) -> LambdaExpr:
body.col_offset = n.body.col_offset

self.class_and_function_stack.append("L")
e = LambdaExpr(
self.transform_args(n.args, n.lineno), self.as_required_block([body], n.lineno)
)
e = LambdaExpr(self.transform_args(n.args, n.lineno), self.as_required_block([body]))
self.class_and_function_stack.pop()
e.set_line(n.lineno, n.col_offset) # Overrides set_line -- can't use self.set_line
return e
Expand Down Expand Up @@ -1743,7 +1742,7 @@ def visit_Match(self, n: Match) -> MatchStmt:
self.visit(n.subject),
[self.visit(c.pattern) for c in n.cases],
[self.visit(c.guard) for c in n.cases],
[self.as_required_block(c.body, n.lineno) for c in n.cases],
[self.as_required_block(c.body) for c in n.cases],
)
return self.set_line(node, n)

Expand Down
4 changes: 4 additions & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ class MypyFile(SymbolNode):
"names",
"imports",
"ignored_lines",
"unreachable_lines",
"is_stub",
"is_cache_skeleton",
"is_partial_stub_package",
Expand All @@ -313,6 +314,8 @@ class MypyFile(SymbolNode):
# If the value is empty, ignore all errors; otherwise, the list contains all
# error codes to ignore.
ignored_lines: dict[int, list[str]]
# Lines that are statically unreachable (e.g. due to platform/version check).
unreachable_lines: set[int]
# Is this file represented by a stub file (.pyi)?
is_stub: bool
# Is this loaded from the cache and thus missing the actual body of the file?
Expand Down Expand Up @@ -345,6 +348,7 @@ def __init__(
self.ignored_lines = ignored_lines
else:
self.ignored_lines = {}
self.unreachable_lines = set()

self.path = ""
self.is_stub = False
Expand Down
Loading