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

Add an option to require ignore comments have error codes #11633

Merged
Merged
Show file tree
Hide file tree
Changes from 14 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
28 changes: 28 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,31 @@ 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
PeterJCLaw marked this conversation as resolved.
Show resolved Hide resolved
---------------------------------------------------

Warn when an ``# type: ignore`` comment does not specify any error codes.
This clarifies the intent of the ignore as well as ensuring that only the
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved
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 'assignment' and 'attr-defined' are silenced
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved
# 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 @@ -3155,6 +3162,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]

[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