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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignoring a file with a single # type: ignore comment. #6830

Merged
merged 7 commits into from May 16, 2019
Merged

Ignoring a file with a single # type: ignore comment. #6830

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7a51cf2
Stop checking at bare top-level "# type: ignore" comments.
brandtbucher May 14, 2019
aa11089
Test new "# type: ignore" behavior.
brandtbucher May 14, 2019
ce54ab4
Appease mypy.
brandtbucher May 14, 2019
4567661
Add docs for new # type: ignore usage.
brandtbucher May 14, 2019
c95b98d
Code cleanup from GvR's first review.
brandtbucher May 14, 2019
07cc315
Break out a new section for this feature.
brandtbucher May 15, 2019
9937b1b
Back out Python 3.8 feature.
brandtbucher May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions docs/source/common_issues.rst
View file
Open in desktop
Expand Up @@ -131,6 +131,30 @@ The second line is now fine, since the ignore comment causes the name
if we did have a stub available for ``frobnicate`` then mypy would
ignore the ``# type: ignore`` comment and typecheck the stub as usual.

A ``# type: ignore`` comment at the top of a module (before any statements,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hardly qualifies as "locally" silencing the checker, so I think it doesn't belong in this section.

I would make it a new section titled "Ignoring a whole file" to be inserted after this section (i.e. just before the section named "Unexpected errors about 'None' and/or 'Optional' types").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've broken it out, as requested.

including imports or docstrings) has the effect of ignoring the *entire* module.

.. code-block:: python

# type: ignore
import frobnicate
frobnicate.start()

When running mypy with Python 3.8 or later, a ``# type: ignore`` comment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like a very useful behavior to me; when would you actually want this?

It's also potentially error-prone. In the example below, a user could write the # type: ignore expecting to silence errors on the import frobnicate line, but then get confused when this actually silences all errors in the whole file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, pyre's behavior is for an ignore comment on a line by itself to suppress errors on only the next line: https://pyre-check.org/docs/error-suppression.html#explicitly-suppressing-errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expecting to silence errors on the import frobnicate line

Why would you expect that to work?

Also, the behavior implemented here is specified by PEP 484.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had assumed that there was some degree of consensus between discussion on #626 and the relevant section of PEP 484 that a # type: ignore on its own line should disable type checking "until the end of the current indented block". This PR is an attempt to bring mypy closer to that spec. I don't have a personal use case for it, but I assume the PEP's authors gave it some thought? Maybe it's worth adding a note or some other warning to the user to help avoid mistakes.

One scenario I can imagine is somebody working their way through an existing code base with several large modules, adding or correcting annotations as they go. Test code also comes to mind, where you may have several fixtures you want type-checked near the top of the module, but intentionally broken code after that.

If we decide that the ignore-rest-of-file-on-3.8 part of this PR should be removed or deferred, I'm fine with that, though. I added it because I could for 3.8, but I'm certainly not married to it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you expect that to work?

It's the intuitively more obvious behavior to me (which, granted, is not a very strong argument), and it's also what Pyre implemented. On the other hand, pytype's behavior is closer to what PEP 484 specifies, but with a way to turn type checking back on: https://github.com/google/pytype/blob/master/docs/user_guide.md#silencing-errors.

Also, the behavior implemented here is specified by PEP 484.

That's true, but this is a part of PEP 484 that has remained unimplemented in mypy for five years. As far as I can tell there have been few if any user requests for the full behavior specified in PEP 484鈥攗sers want to be able to ignore the whole file, but I don't see people asking for "ignore errors from an arbitrary point to the end of the file". So I think it's worth asking if the PEP 484 behavior really is what we want. We can always change PEP 484 too.

If #2938 is implemented, even using # type: ignore to ignore the whole file could be a redundant feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm happy to take this to the python/typing tracker to discuss potentially changing the PEP. My concern on this PR is that we don't introduce this behavior into mypy too soon and produce backward compatibility problems if we ever decide to change it.

anywhere at the top indentation level of a module will skip type checking for
all remaining lines in the file.

.. code-block:: python

"""Docstring."""

import spam # These imports are still checked!
import eggs

# type: ignore
import frobnicate
frobnicate.start()

Another option is to explicitly annotate values with type ``Any`` --
mypy will let you perform arbitrary operations on ``Any``
values. Sometimes there is no more precise type you can use for a
Expand Down
69 changes: 53 additions & 16 deletions mypy/fastparse.py
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@
import sys

from typing import (
Tuple, Union, TypeVar, Callable, Sequence, Optional, Any, Dict, cast, List, overload
Tuple, Union, TypeVar, Callable, Sequence, Optional, Any, Dict, cast, List, overload, Set
)
MYPY = False
if MYPY:
Expand Down Expand Up @@ -258,7 +258,7 @@ def __init__(self,
self.is_stub = is_stub
self.errors = errors

self.extra_type_ignores = [] # type: List[int]
self.type_ignores = set() # type: Set[int]

# Cache of visit_X methods keyed by type of visited object
self.visitor_cache = {} # type: Dict[type, Callable[[Optional[AST]], Any]]
Expand Down Expand Up @@ -294,11 +294,49 @@ def translate_expr_list(self, l: Sequence[AST]) -> List[Expression]:
res.append(exp)
return res

def translate_stmt_list(self, l: Sequence[AST]) -> List[Statement]:
def get_lineno(self, node: Union[ast3.expr, ast3.stmt]) -> int:
if (isinstance(node, (ast3.AsyncFunctionDef, ast3.ClassDef, ast3.FunctionDef))
and node.decorator_list):
return node.decorator_list[0].lineno
return node.lineno

def translate_stmt_list(self,
stmts: Sequence[ast3.stmt],
ismodule: bool = False) -> List[Statement]:
# A "# type: ignore" comment before the first statement of a module
# ignores the whole module:
if (ismodule and stmts and self.type_ignores
and min(self.type_ignores) < self.get_lineno(stmts[0])):
self.errors.used_ignored_lines[self.errors.file].add(min(self.type_ignores))
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
block.is_unreachable = True
return [block]

res = [] # type: List[Statement]
for e in l:
stmt = self.visit(e)
res.append(stmt)
line = 0

for i, stmt in enumerate(stmts):

if ismodule: # This line needs to be split for mypy to branch on version:
if sys.version_info >= (3, 8):
# In Python 3.8+ (we need end_lineno), a "# type: ignore" comment
# between statements at the top level of a module skips checking
# for everything else:
ignores = set(range(line + 1, self.get_lineno(stmt))) & self.type_ignores

if ignores:
self.errors.used_ignored_lines[self.errors.file].add(min(ignores))
rest = self.fix_function_overloads(self.translate_stmt_list(stmts[i:]))
block = Block(rest)
block.is_unreachable = True
res.append(block)
return res

line = stmt.end_lineno if stmt.end_lineno is not None else stmt.lineno

node = self.visit(stmt)
res.append(node)

return res

op_map = {
Expand Down Expand Up @@ -403,13 +441,12 @@ def translate_module_id(self, id: str) -> str:
return id

def visit_Module(self, mod: ast3.Module) -> MypyFile:
body = self.fix_function_overloads(self.translate_stmt_list(mod.body))
ignores = [ti.lineno for ti in mod.type_ignores]
ignores.extend(self.extra_type_ignores)
self.type_ignores = {ti.lineno for ti in mod.type_ignores}
body = self.fix_function_overloads(self.translate_stmt_list(mod.body, ismodule=True))
return MypyFile(body,
self.imports,
False,
set(ignores),
self.type_ignores,
)

# --- stmt ---
Expand Down Expand Up @@ -615,7 +652,7 @@ def make_argument(self, arg: ast3.arg, default: Optional[ast3.expr], kind: int,
elif type_comment is not None:
extra_ignore, arg_type = parse_type_comment(type_comment, arg.lineno, self.errors)
if extra_ignore:
self.extra_type_ignores.append(arg.lineno)
self.type_ignores.add(arg.lineno)

return Argument(Var(arg.arg), arg_type, self.visit(default), kind)

Expand Down Expand Up @@ -673,7 +710,7 @@ def visit_Assign(self, n: ast3.Assign) -> AssignmentStmt:
if n.type_comment is not None:
extra_ignore, typ = parse_type_comment(n.type_comment, n.lineno, self.errors)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
typ = None
s = AssignmentStmt(lvalues, rvalue, type=typ, new_syntax=False)
Expand Down Expand Up @@ -707,7 +744,7 @@ def visit_For(self, n: ast3.For) -> ForStmt:
if n.type_comment is not None:
extra_ignore, target_type = parse_type_comment(n.type_comment, n.lineno, self.errors)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
target_type = None
node = ForStmt(self.visit(n.target),
Expand All @@ -722,7 +759,7 @@ def visit_AsyncFor(self, n: ast3.AsyncFor) -> ForStmt:
if n.type_comment is not None:
extra_ignore, target_type = parse_type_comment(n.type_comment, n.lineno, self.errors)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
target_type = None
node = ForStmt(self.visit(n.target),
Expand Down Expand Up @@ -753,7 +790,7 @@ def visit_With(self, n: ast3.With) -> WithStmt:
if n.type_comment is not None:
extra_ignore, target_type = parse_type_comment(n.type_comment, n.lineno, self.errors)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
target_type = None
node = WithStmt([self.visit(i.context_expr) for i in n.items],
Expand All @@ -767,7 +804,7 @@ def visit_AsyncWith(self, n: ast3.AsyncWith) -> WithStmt:
if n.type_comment is not None:
extra_ignore, target_type = parse_type_comment(n.type_comment, n.lineno, self.errors)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
target_type = None
s = WithStmt([self.visit(i.context_expr) for i in n.items],
Expand Down
41 changes: 28 additions & 13 deletions mypy/fastparse2.py
View file
Open in desktop
Expand Up @@ -16,7 +16,7 @@
"""
import sys

from typing import Tuple, Union, TypeVar, Callable, Sequence, Optional, Any, Dict, cast, List
from typing import Tuple, Union, TypeVar, Callable, Sequence, Optional, Any, Dict, cast, List, Set
MYPY = False
if MYPY:
import typing # for typing.Type, which conflicts with types.Type
Expand Down Expand Up @@ -163,7 +163,7 @@ def __init__(self,
# Cache of visit_X methods keyed by type of visited object
self.visitor_cache = {} # type: Dict[type, Callable[[Optional[AST]], Any]]

self.extra_type_ignores = [] # type: List[int]
self.type_ignores = set() # type: Set[int]

def fail(self, msg: str, line: int, column: int, blocker: bool = True) -> None:
if blocker or not self.options.ignore_errors:
Expand Down Expand Up @@ -193,12 +193,28 @@ def translate_expr_list(self, l: Sequence[AST]) -> List[Expression]:
res.append(exp)
return res

def translate_stmt_list(self, l: Sequence[AST]) -> List[Statement]:
def get_lineno(self, node: Union[ast27.expr, ast27.stmt]) -> int:
if isinstance(node, (ast27.ClassDef, ast27.FunctionDef)) and node.decorator_list:
return node.decorator_list[0].lineno
return node.lineno

def translate_stmt_list(self,
stmts: Sequence[ast27.stmt],
module: bool = False) -> List[Statement]:
# A "# type: ignore" comment before the first statement of a module
# ignores the whole module:
if (module and stmts and self.type_ignores
and min(self.type_ignores) < self.get_lineno(stmts[0])):
self.errors.used_ignored_lines[self.errors.file].add(min(self.type_ignores))
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
block.is_unreachable = True
return [block]

res = [] # type: List[Statement]
for e in l:
stmt = self.visit(e)
assert isinstance(stmt, Statement)
res.append(stmt)
for stmt in stmts:
node = self.visit(stmt)
assert isinstance(node, Statement)
res.append(node)
return res

op_map = {
Expand Down Expand Up @@ -304,13 +320,12 @@ def translate_module_id(self, id: str) -> str:
return id

def visit_Module(self, mod: ast27.Module) -> MypyFile:
self.type_ignores = {ti.lineno for ti in mod.type_ignores}
body = self.fix_function_overloads(self.translate_stmt_list(mod.body))
ignores = [ti.lineno for ti in mod.type_ignores]
ignores.extend(self.extra_type_ignores)
return MypyFile(body,
self.imports,
False,
set(ignores),
self.type_ignores,
)

# --- stmt ---
Expand Down Expand Up @@ -558,7 +573,7 @@ def visit_Assign(self, n: ast27.Assign) -> AssignmentStmt:
extra_ignore, typ = parse_type_comment(n.type_comment, n.lineno, self.errors,
assume_str_is_unicode=self.unicode_literals)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)

stmt = AssignmentStmt(self.translate_expr_list(n.targets),
self.visit(n.value),
Expand All @@ -578,7 +593,7 @@ def visit_For(self, n: ast27.For) -> ForStmt:
extra_ignore, typ = parse_type_comment(n.type_comment, n.lineno, self.errors,
assume_str_is_unicode=self.unicode_literals)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
typ = None
stmt = ForStmt(self.visit(n.target),
Expand Down Expand Up @@ -608,7 +623,7 @@ def visit_With(self, n: ast27.With) -> WithStmt:
extra_ignore, typ = parse_type_comment(n.type_comment, n.lineno, self.errors,
assume_str_is_unicode=self.unicode_literals)
if extra_ignore:
self.extra_type_ignores.append(n.lineno)
self.type_ignores.add(n.lineno)
else:
typ = None
stmt = WithStmt([self.visit(n.context_expr)],
Expand Down
5 changes: 5 additions & 0 deletions test-data/unit/check-38.test
View file
Open in desktop
Expand Up @@ -106,3 +106,8 @@ def g(x: int): ...
/
0 # type: ignore
) # type: ignore # E: unused 'type: ignore' comment

[case testIgnoreRestOfModule]
ERROR # E: Name 'ERROR' is not defined
# type: ignore
IGNORE
40 changes: 40 additions & 0 deletions test-data/unit/check-ignore.test
View file
Open in desktop
Expand Up @@ -218,3 +218,43 @@ def f() -> None: pass

[case testCannotIgnoreBlockingError]
yield # type: ignore # E: 'yield' outside function

[case testIgnoreWholeModule1]
# flags: --warn-unused-ignores
# type: ignore
IGNORE # type: ignore # E: unused 'type: ignore' comment

[case testIgnoreWholeModule2]
# type: ignore
if True:
IGNORE

[case testIgnoreWholeModule3]
# type: ignore
@d
class C: ...
IGNORE

[case testIgnoreWholeModule4]
# type: ignore
@d

def f(): ...
IGNORE

[case testDontIgnoreWholeModule1]
if True:
# type: ignore
ERROR # E: Name 'ERROR' is not defined
ERROR # E: Name 'ERROR' is not defined

[case testDontIgnoreWholeModule2]
@d # type: ignore
class C: ...
ERROR # E: Name 'ERROR' is not defined

[case testDontIgnoreWholeModule3]
@d # type: ignore

def f(): ...
ERROR # E: Name 'ERROR' is not defined