Skip to content

Commit

Permalink
Add an option to require ignore comments have error codes (#11633)
Browse files Browse the repository at this point in the history
Fixes #11509
  • Loading branch information
PeterJCLaw authored Feb 9, 2022
1 parent 6ff8091 commit 84696ce
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 2 deletions.
31 changes: 31 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,34 @@ no error in practice. In such case, it might be prudent to annotate ``items: Seq
This is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``Sized``),
except that attempting to invoke an undefined method (e.g. ``__len__``) results in an error,
while attempting to evaluate an object in boolean context without a concrete implementation results in a truthy value.


Check that ``# type: ignore`` include an error code [ignore-without-code]
-------------------------------------------------------------------------

Warn when a ``# type: ignore`` comment does not specify any error codes.
This clarifies the intent of the ignore and ensures that only the
expected errors are silenced.

Example:

.. code-block:: python
# mypy: enable-error-code ignore-without-code
class Foo:
def __init__(self, name: str) -> None:
self.name = name
f = Foo('foo')
# This line has a typo that mypy can't help with as both:
# - the expected error 'assignment', and
# - the unexpected error 'attr-defined'
# are silenced.
# Error: "type: ignore" comment without error code (currently ignored: [attr-defined])
f.nme = 42 # type: ignore
# This line warns correctly about the typo in the attribute name
# Error: "Foo" has no attribute "nme"; maybe "name"?
f.nme = 42 # type: ignore[assignment]
8 changes: 8 additions & 0 deletions mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,13 @@ def generate_unused_ignore_notes(self) -> None:
self.verify_dependencies(suppressed_only=True)
self.manager.errors.generate_unused_ignore_errors(self.xpath)

def generate_ignore_without_code_notes(self) -> None:
if self.manager.errors.is_error_code_enabled(codes.IGNORE_WITHOUT_CODE):
self.manager.errors.generate_ignore_without_code_errors(
self.xpath,
self.options.warn_unused_ignores,
)


# Module import and diagnostic glue

Expand Down Expand Up @@ -3168,6 +3175,7 @@ def process_stale_scc(graph: Graph, scc: List[str], manager: BuildManager) -> No
graph[id].finish_passes()
for id in stale:
graph[id].generate_unused_ignore_notes()
graph[id].generate_ignore_without_code_notes()
if any(manager.errors.is_errors_for_file(graph[id].xpath) for id in stale):
for id in stale:
graph[id].transitive_error = True
Expand Down
11 changes: 11 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,21 @@ def __str__(self) -> str:
"Check that overloaded functions outside stub files have an implementation",
"General",
)
IGNORE_WITHOUT_CODE: Final = ErrorCode(
"ignore-without-code",
"Warn about '# type: ignore' comments which do not have error codes",
"General",
default_enabled=False,
)


# Syntax errors are often blocking.
SYNTAX: Final = ErrorCode("syntax", "Report syntax errors", "General")

# This is an internal marker code for a whole-file ignore. It is not intended to
# be user-visible.
FILE: Final = ErrorCode("file", "Internal marker for a whole file being ignored", "General")
del error_codes[FILE.code]

# This is a catch-all for remaining uncategorized errors.
MISC: Final = ErrorCode("misc", "Miscellaneous other checks", "General")
35 changes: 35 additions & 0 deletions mypy/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,41 @@ def generate_unused_ignore_errors(self, file: str) -> None:
None, False, False, False)
self._add_error_info(file, info)

def generate_ignore_without_code_errors(self,
file: str,
is_warning_unused_ignores: bool) -> None:
if is_typeshed_file(file) or file in self.ignored_files:
return

used_ignored_lines = self.used_ignored_lines[file]

# If the whole file is ignored, ignore it.
if used_ignored_lines:
_, used_codes = min(used_ignored_lines.items())
if codes.FILE.code in used_codes:
return

for line, ignored_codes in self.ignored_lines[file].items():
if ignored_codes:
continue

# If the ignore is itself unused and that would be warned about, let
# that error stand alone
if is_warning_unused_ignores and not used_ignored_lines[line]:
continue

codes_hint = ''
ignored_codes = used_ignored_lines[line]
if ignored_codes:
codes_hint = f' (currently ignored: [{", ".join(ignored_codes)}])'

message = f'"type: ignore" comment without error code{codes_hint}'
# Don't use report since add_error_info will ignore the error!
info = ErrorInfo(self.import_context(), file, self.current_module(), None,
None, line, -1, 'error', message, codes.IGNORE_WITHOUT_CODE,
False, False, False)
self._add_error_info(file, info)

def num_messages(self) -> int:
"""Return the number of generated messages."""
return sum(len(x) for x in self.error_info_map.values())
Expand Down
2 changes: 1 addition & 1 deletion mypy/fastparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ def translate_stmt_list(self,
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][min(self.type_ignores)].append(
codes.MISC.code)
codes.FILE.code)
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
mark_block_unreachable(block)
return [block]
Expand Down
2 changes: 1 addition & 1 deletion mypy/fastparse2.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ def translate_stmt_list(self,
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][min(self.type_ignores)].append(
codes.MISC.code)
codes.FILE.code)
block = Block(self.fix_function_overloads(self.translate_stmt_list(stmts)))
mark_block_unreachable(block)
return [block]
Expand Down
19 changes: 19 additions & 0 deletions test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,25 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi
# flags: --warn-unused-ignores
"x" # type: ignore[name-defined] # E: Unused "type: ignore" comment

[case testErrorCodeMissingWhenRequired]
# flags: --enable-error-code ignore-without-code
"x" # type: ignore # E: "type: ignore" comment without error code [ignore-without-code]
y # type: ignore # E: "type: ignore" comment without error code (currently ignored: [name-defined]) [ignore-without-code]
z # type: ignore[name-defined]
"a" # type: ignore[ignore-without-code]

[case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning]
# flags: --enable-error-code ignore-without-code --warn-unused-ignores
"x" # type: ignore # E: Unused "type: ignore" comment
"y" # type: ignore[ignore-without-code] # E: Unused "type: ignore" comment
z # type: ignore[ignore-without-code] # E: Unused "type: ignore" comment # E: Name "z" is not defined [name-defined] # N: Error code "name-defined" not covered by "type: ignore" comment

[case testErrorCodeMissingWholeFileIgnores]
# flags: --enable-error-code ignore-without-code
# type: ignore # whole file ignore
x
y # type: ignore # ignore the lack of error code since we're ignore the whole file

[case testErrorCodeIgnoreWithExtraSpace]
x # type: ignore [name-defined]
x2 # type: ignore [ name-defined ]
Expand Down

0 comments on commit 84696ce

Please sign in to comment.