Skip to content

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

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.

Sign up for GitHub

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

Merged
gvanrossum 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 2 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions docs/source/common_issues.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
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

@gvanrossum gvanrossum May 15, 2019

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

@brandtbucher brandtbucher May 15, 2019

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

@JelleZijlstra JelleZijlstra May 15, 2019

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

@JelleZijlstra JelleZijlstra May 15, 2019

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

@gvanrossum gvanrossum May 15, 2019

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

@brandtbucher brandtbucher May 15, 2019

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

@JelleZijlstra JelleZijlstra May 15, 2019 •
edited
Loading

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—users 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

@JelleZijlstra JelleZijlstra May 15, 2019

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
55 changes: 27 additions & 28 deletions mypy/fastparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,49 +294,48 @@ def translate_expr_list(self, l: Sequence[AST]) -> List[Expression]:
res.append(exp)
return res

def get_line(self, n: ast3.stmt) -> int:
if (isinstance(n, (ast3.AsyncFunctionDef, ast3.ClassDef, ast3.FunctionDef))
and n.decorator_list):
return n.decorator_list[0].lineno
return n.lineno
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,
l: Sequence[ast3.stmt],
module: bool = False) -> List[Statement]:

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 module and l and self.type_ignores and min(self.type_ignores) < self.get_line(l[0]):
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))
b = Block(self.fix_function_overloads(self.translate_stmt_list(l)))
b.is_unreachable = True
return [b]
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
block.is_unreachable = True
return [block]

res = [] # type: List[Statement]
line = 0

for i, e in enumerate(l):
for i, stmt in enumerate(stmts):

if module: # and...
if ismodule: # This line needs to be split for mypy to branch on version:
if sys.version_info >= (3, 8):

# In Python 3.8, 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_line(e))) & self.type_ignores
# 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))
b = Block(self.fix_function_overloads(self.translate_stmt_list(l[i:])))
b.is_unreachable = True
res.append(b)
rest = self.fix_function_overloads(self.translate_stmt_list(stmts[i:]))
block = Block(rest)
block.is_unreachable = True
res.append(block)
return res

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

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

return res

Expand Down Expand Up @@ -443,11 +442,11 @@ def translate_module_id(self, id: str) -> str:

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

# --- stmt ---
Expand Down
31 changes: 15 additions & 16 deletions mypy/fastparse2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,29 +193,28 @@ def translate_expr_list(self, l: Sequence[AST]) -> List[Expression]:
res.append(exp)
return res

def get_line(self, n: ast27.stmt) -> int:
if isinstance(n, (ast27.ClassDef, ast27.FunctionDef)) and n.decorator_list:
return n.decorator_list[0].lineno
return n.lineno
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,
l: Sequence[ast27.stmt],
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 l and self.type_ignores and min(self.type_ignores) < self.get_line(l[0]):
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))
b = Block(self.fix_function_overloads(self.translate_stmt_list(l)))
b.is_unreachable = True
return [b]
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 @@ -326,7 +325,7 @@ def visit_Module(self, mod: ast27.Module) -> MypyFile:
return MypyFile(body,
self.imports,
False,
set(self.type_ignores),
self.type_ignores,
)

# --- stmt ---
Expand Down