From 84696ceeec978e569ca702f4868bb54ba47b97f3 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Wed, 9 Feb 2022 23:44:24 +0000 Subject: [PATCH] Add an option to require ignore comments have error codes (#11633) Fixes #11509 --- docs/source/error_code_list2.rst | 31 ++++++++++++++++++++++++ mypy/build.py | 8 +++++++ mypy/errorcodes.py | 11 +++++++++ mypy/errors.py | 35 ++++++++++++++++++++++++++++ mypy/fastparse.py | 2 +- mypy/fastparse2.py | 2 +- test-data/unit/check-errorcodes.test | 19 +++++++++++++++ 7 files changed, 106 insertions(+), 2 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index 1e035fcf7f69..ba5b67a5aeb7 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -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] diff --git a/mypy/build.py b/mypy/build.py index 76dded73b2bf..24cef0b83bd3 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -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 @@ -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 diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index ba716608ae56..4407df47d596 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -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") diff --git a/mypy/errors.py b/mypy/errors.py index ec49541a15a5..9744f134ac16 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -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()) diff --git a/mypy/fastparse.py b/mypy/fastparse.py index 5f9ec87e544f..6de1196fcd81 100644 --- a/mypy/fastparse.py +++ b/mypy/fastparse.py @@ -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] diff --git a/mypy/fastparse2.py b/mypy/fastparse2.py index bf3c09453ec0..5162fbb5df45 100644 --- a/mypy/fastparse2.py +++ b/mypy/fastparse2.py @@ -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] diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 177612959354..642828085b53 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -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 ]