From 57311e0c4df29acb9bab63fa438be03d683fdae2 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sun, 28 Nov 2021 15:10:32 +0000 Subject: [PATCH 01/17] Add an option to require ignore comments have error codes Fixes https://github.com/python/mypy/issues/11509 --- docs/source/command_line.rst | 9 +++++++++ mypy/build.py | 8 ++++++++ mypy/errors.py | 22 ++++++++++++++++++++++ mypy/main.py | 4 ++++ mypy/options.py | 3 +++ test-data/unit/check-errorcodes.test | 9 +++++++++ 6 files changed, 55 insertions(+) diff --git a/docs/source/command_line.rst b/docs/source/command_line.rst index 64c2f8f03b3e..cfc52385fff3 100644 --- a/docs/source/command_line.rst +++ b/docs/source/command_line.rst @@ -680,6 +680,15 @@ in error messages. See :ref:`error-codes` for more information. +.. option:: --disallow-ignore-without-code + + This flag will disallow ``type: ignore`` comments which do not have + error codes:: + + prog.py:1: error: "type: ignore" comment without error code + + See :ref:`error-codes` for more information. + .. option:: --pretty Use visually nicer output in error messages: use soft word wrap, diff --git a/mypy/build.py b/mypy/build.py index 4398b0281d99..a9dd35f33c16 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.options.disallow_ignore_without_code: + self.manager.errors.generate_ignore_without_code_errors( + self.xpath, + self.options.warn_unused_ignores, + ) + # Module import and diagnostic glue @@ -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 diff --git a/mypy/errors.py b/mypy/errors.py index c711456468ed..c5cf2752f4f6 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -485,6 +485,28 @@ 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] + 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 + + # 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', '"type: ignore" comment without error code', + None, 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/main.py b/mypy/main.py index d765781838cf..536ed0eb1f48 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -677,6 +677,10 @@ def add_invertible_flag(flag: str, " non-overlapping types", group=strictness_group) + add_invertible_flag('--disallow-ignore-without-code', default=False, strict_flag=True, + help="Disallow 'type: ignore' comments which do not have error codes", + group=strictness_group) + strict_help = "Strict mode; enables the following flags: {}".format( ", ".join(strict_flag_names)) strictness_group.add_argument( diff --git a/mypy/options.py b/mypy/options.py index 58278b1580e8..d60dc48eb9a8 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -143,6 +143,9 @@ def __init__(self) -> None: # Warn about unused '# type: ignore' comments self.warn_unused_ignores = False + # Warn about '# type: ignore' comments without error codes + self.disallow_ignore_without_code = False + # Warn about unused '[mypy-]' or '[[tool.mypy.overrides]]' config sections self.warn_unused_configs = False diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 04a895d95638..e860b5ffe25c 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -135,6 +135,15 @@ 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: --disallow-ignore-without-code +"x" # type: ignore # E: "type: ignore" comment without error code +y # type: ignore # E: "type: ignore" comment without error code + +[case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning] +# flags: --disallow-ignore-without-code --warn-unused-ignores +"x" # type: ignore # E: Unused "type: ignore" comment + [case testErrorCodeIgnoreWithExtraSpace] x # type: ignore [name-defined] x2 # type: ignore [ name-defined ] From a99fd9bb4117e5ea7b2d3edb4a65aeaf5fdfc851 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sun, 28 Nov 2021 16:59:20 +0000 Subject: [PATCH 02/17] Ensure wholly ignored files don't get errors about missing codes --- mypy/errorcodes.py | 5 +++++ mypy/errors.py | 7 +++++++ mypy/fastparse.py | 2 +- mypy/fastparse2.py | 2 +- test-data/unit/check-errorcodes.test | 6 ++++++ 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 2a07bbb8597b..c6f35b7ddb92 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -138,5 +138,10 @@ def __str__(self) -> str: # 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 c5cf2752f4f6..dd9b2004f90b 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -492,6 +492,13 @@ def generate_ignore_without_code_errors(self, 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 diff --git a/mypy/fastparse.py b/mypy/fastparse.py index a0d0ec8e34b0..2ec532d27a74 100644 --- a/mypy/fastparse.py +++ b/mypy/fastparse.py @@ -360,7 +360,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 33f34082cf83..6e862374845f 100644 --- a/mypy/fastparse2.py +++ b/mypy/fastparse2.py @@ -216,7 +216,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 e860b5ffe25c..cba7a6074b8c 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -144,6 +144,12 @@ y # type: ignore # E: "type: ignore" comment without error code # flags: --disallow-ignore-without-code --warn-unused-ignores "x" # type: ignore # E: Unused "type: ignore" comment +[case testErrorCodeMissingWholeFileIgnores] +# flags: --disallow-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 ] From b678dd4b403659d1d772bc60a0674eed1657dd59 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Mon, 29 Nov 2021 21:53:44 +0000 Subject: [PATCH 03/17] Tell the user which codes they need to add --- mypy/errors.py | 8 +++++++- test-data/unit/check-errorcodes.test | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/mypy/errors.py b/mypy/errors.py index dd9b2004f90b..78977e3c6125 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -508,9 +508,15 @@ def generate_ignore_without_code_errors(self, 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' (hint: add [{", ".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', '"type: ignore" comment without error code', + None, line, -1, 'error', message, None, False, False, False) self._add_error_info(file, info) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index cba7a6074b8c..9a3028f528b1 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -138,7 +138,7 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi [case testErrorCodeMissingWhenRequired] # flags: --disallow-ignore-without-code "x" # type: ignore # E: "type: ignore" comment without error code -y # type: ignore # E: "type: ignore" comment without error code +y # type: ignore # E: "type: ignore" comment without error code (hint: add [name-defined]) [case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning] # flags: --disallow-ignore-without-code --warn-unused-ignores From 091c0211929a04cd110c99c3c3f5b2a253219f55 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Mon, 29 Nov 2021 22:00:42 +0000 Subject: [PATCH 04/17] Group --disallow-ignore-without-code with --warn-unused-ignores Also make it non-strict for now. It may become part of --strict in the future, though work may be needed to reduce the number of 'misc' errors before then. --- docs/source/command_line.rst | 18 +++++++++--------- mypy/main.py | 7 +++---- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/docs/source/command_line.rst b/docs/source/command_line.rst index cfc52385fff3..9cb374366193 100644 --- a/docs/source/command_line.rst +++ b/docs/source/command_line.rst @@ -440,6 +440,15 @@ potentially problematic or redundant in some way. These two flags let you discover cases where either workarounds are no longer necessary. +.. option:: --disallow-ignore-without-code + + This flag will disallow ``type: ignore`` comments which do not have + error codes:: + + prog.py:1: error: "type: ignore" comment without error code + + See :ref:`error-codes` for more information. + .. option:: --no-warn-no-return By default, mypy will generate errors when a function is missing @@ -680,15 +689,6 @@ in error messages. See :ref:`error-codes` for more information. -.. option:: --disallow-ignore-without-code - - This flag will disallow ``type: ignore`` comments which do not have - error codes:: - - prog.py:1: error: "type: ignore" comment without error code - - See :ref:`error-codes` for more information. - .. option:: --pretty Use visually nicer output in error messages: use soft word wrap, diff --git a/mypy/main.py b/mypy/main.py index 536ed0eb1f48..e53d02467d15 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -637,6 +637,9 @@ def add_invertible_flag(flag: str, add_invertible_flag('--warn-unused-ignores', default=False, strict_flag=True, help="Warn about unneeded '# type: ignore' comments", group=lint_group) + add_invertible_flag('--disallow-ignore-without-code', default=False, + help="Disallow '# type: ignore' comments which do not have error codes", + group=lint_group) add_invertible_flag('--no-warn-no-return', dest='warn_no_return', default=True, help="Do not warn about functions that end without returning", group=lint_group) @@ -677,10 +680,6 @@ def add_invertible_flag(flag: str, " non-overlapping types", group=strictness_group) - add_invertible_flag('--disallow-ignore-without-code', default=False, strict_flag=True, - help="Disallow 'type: ignore' comments which do not have error codes", - group=strictness_group) - strict_help = "Strict mode; enables the following flags: {}".format( ", ".join(strict_flag_names)) strictness_group.add_argument( From 2b65ab27509f604e6be42219db6356898571064f Mon Sep 17 00:00:00 2001 From: Peter Law Date: Mon, 29 Nov 2021 22:04:20 +0000 Subject: [PATCH 05/17] Clarify that this is a comment Stylistically matching --warn-unused-ignores' docs. --- docs/source/command_line.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/command_line.rst b/docs/source/command_line.rst index 9cb374366193..5f5d329e9724 100644 --- a/docs/source/command_line.rst +++ b/docs/source/command_line.rst @@ -442,7 +442,7 @@ potentially problematic or redundant in some way. .. option:: --disallow-ignore-without-code - This flag will disallow ``type: ignore`` comments which do not have + This flag will disallow ``# type: ignore`` comments which do not have error codes:: prog.py:1: error: "type: ignore" comment without error code From 8569dc6d406b593c547fd04578a1ebd4a93872c7 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Thu, 2 Dec 2021 20:49:19 +0000 Subject: [PATCH 06/17] Also document as a config setting --- docs/source/config_file.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/source/config_file.rst b/docs/source/config_file.rst index c34f23d9e169..1ef19b0c2d70 100644 --- a/docs/source/config_file.rst +++ b/docs/source/config_file.rst @@ -507,6 +507,13 @@ section of the command line docs. Warns about unneeded ``# type: ignore`` comments. +.. confval:: disallow_ignore_without_code + + :type: boolean + :default: False + + Disallow ``# type: ignore`` comments which do not have error codes. + .. confval:: warn_no_return :type: boolean From c811aa7fb13ef49c9f4677e6d925d10ca8e44e74 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Thu, 2 Dec 2021 20:49:55 +0000 Subject: [PATCH 07/17] Note that this can be set on a per-file basis This also includes it in the OPTIONS_AFFECTING_CACHE, which fixes the caching issue I'd been seeing. --- mypy/options.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mypy/options.py b/mypy/options.py index d60dc48eb9a8..9ffcc8ed4c79 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -32,6 +32,7 @@ class BuildType: "disallow_any_expr", "disallow_any_generics", "disallow_any_unimported", + "disallow_ignore_without_code", "disallow_incomplete_defs", "disallow_subclassing_any", "disallow_untyped_calls", From 765e1c5014b3be7d70999082fceb6fec5b83066f Mon Sep 17 00:00:00 2001 From: Peter Law Date: Thu, 2 Dec 2021 20:56:41 +0000 Subject: [PATCH 08/17] Rephrase flag to more closely match similar flags "Disallow" didn't feel right here since mypy was still obeying the ignore comment. "Warn" is more precise since this is configuring a new warning to be emitted. This also adjusts the pluralisation in a way I think is better. --- docs/source/command_line.rst | 6 +++--- docs/source/config_file.rst | 4 ++-- mypy/build.py | 2 +- mypy/main.py | 2 +- mypy/options.py | 6 +++--- test-data/unit/check-errorcodes.test | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/source/command_line.rst b/docs/source/command_line.rst index 5f5d329e9724..cadb527b84f6 100644 --- a/docs/source/command_line.rst +++ b/docs/source/command_line.rst @@ -440,10 +440,10 @@ potentially problematic or redundant in some way. These two flags let you discover cases where either workarounds are no longer necessary. -.. option:: --disallow-ignore-without-code +.. option:: --warn-ignores-without-codes - This flag will disallow ``# type: ignore`` comments which do not have - error codes:: + This flag will make mypy report an error for any ``# type: ignore`` comments + which do not have error codes:: prog.py:1: error: "type: ignore" comment without error code diff --git a/docs/source/config_file.rst b/docs/source/config_file.rst index 1ef19b0c2d70..46a10fa457fb 100644 --- a/docs/source/config_file.rst +++ b/docs/source/config_file.rst @@ -507,12 +507,12 @@ section of the command line docs. Warns about unneeded ``# type: ignore`` comments. -.. confval:: disallow_ignore_without_code +.. confval:: warn_ignores_without_codes :type: boolean :default: False - Disallow ``# type: ignore`` comments which do not have error codes. + Warn about '# type: ignore' comments which do not have error codes. .. confval:: warn_no_return diff --git a/mypy/build.py b/mypy/build.py index a9dd35f33c16..0410cd635b9c 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -2370,7 +2370,7 @@ def generate_unused_ignore_notes(self) -> None: self.manager.errors.generate_unused_ignore_errors(self.xpath) def generate_ignore_without_code_notes(self) -> None: - if self.options.disallow_ignore_without_code: + if self.options.warn_ignores_without_codes: self.manager.errors.generate_ignore_without_code_errors( self.xpath, self.options.warn_unused_ignores, diff --git a/mypy/main.py b/mypy/main.py index e53d02467d15..37e10745de3e 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -637,7 +637,7 @@ def add_invertible_flag(flag: str, add_invertible_flag('--warn-unused-ignores', default=False, strict_flag=True, help="Warn about unneeded '# type: ignore' comments", group=lint_group) - add_invertible_flag('--disallow-ignore-without-code', default=False, + add_invertible_flag('--warn-ignores-without-codes', default=False, help="Disallow '# type: ignore' comments which do not have error codes", group=lint_group) add_invertible_flag('--no-warn-no-return', dest='warn_no_return', default=True, diff --git a/mypy/options.py b/mypy/options.py index 9ffcc8ed4c79..12b5930e032a 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -32,7 +32,6 @@ class BuildType: "disallow_any_expr", "disallow_any_generics", "disallow_any_unimported", - "disallow_ignore_without_code", "disallow_incomplete_defs", "disallow_subclassing_any", "disallow_untyped_calls", @@ -50,6 +49,7 @@ class BuildType: "strict_equality", "strict_optional", "strict_optional_whitelist", + "warn_ignores_without_codes", "warn_no_return", "warn_return_any", "warn_unreachable", @@ -144,8 +144,8 @@ def __init__(self) -> None: # Warn about unused '# type: ignore' comments self.warn_unused_ignores = False - # Warn about '# type: ignore' comments without error codes - self.disallow_ignore_without_code = False + # Warn about '# type: ignore' comments which do not have error codes + self.warn_ignores_without_codes = False # Warn about unused '[mypy-]' or '[[tool.mypy.overrides]]' config sections self.warn_unused_configs = False diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 9a3028f528b1..49837b309f2b 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -136,16 +136,16 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi "x" # type: ignore[name-defined] # E: Unused "type: ignore" comment [case testErrorCodeMissingWhenRequired] -# flags: --disallow-ignore-without-code +# flags: --warn-ignores-without-codes "x" # type: ignore # E: "type: ignore" comment without error code y # type: ignore # E: "type: ignore" comment without error code (hint: add [name-defined]) [case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning] -# flags: --disallow-ignore-without-code --warn-unused-ignores +# flags: --warn-ignores-without-codes --warn-unused-ignores "x" # type: ignore # E: Unused "type: ignore" comment [case testErrorCodeMissingWholeFileIgnores] -# flags: --disallow-ignore-without-code +# flags: --warn-ignores-without-codes # type: ignore # whole file ignore x y # type: ignore # ignore the lack of error code since we're ignore the whole file From f113c4ab0b3e29a148ee558ebd771ab2917d2cfe Mon Sep 17 00:00:00 2001 From: Peter Law Date: Fri, 3 Dec 2021 18:49:39 +0000 Subject: [PATCH 09/17] Add an error code for this new warning --- mypy/errorcodes.py | 6 ++++++ mypy/errors.py | 4 ++-- test-data/unit/check-errorcodes.test | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index c6f35b7ddb92..73316886d692 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -133,6 +133,12 @@ def __str__(self) -> str: NAME_MATCH: Final = ErrorCode( "name-match", "Check that type definition has consistent naming", "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. diff --git a/mypy/errors.py b/mypy/errors.py index 78977e3c6125..995ba10c6458 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -516,8 +516,8 @@ def generate_ignore_without_code_errors(self, 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, - None, False, False, False) + None, line, -1, 'error', message, codes.IGNORE_WITHOUT_CODE, + False, False, False) self._add_error_info(file, info) def num_messages(self) -> int: diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 49837b309f2b..c74e8200ec31 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -137,8 +137,8 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi [case testErrorCodeMissingWhenRequired] # flags: --warn-ignores-without-codes -"x" # type: ignore # E: "type: ignore" comment without error code -y # type: ignore # E: "type: ignore" comment without error code (hint: add [name-defined]) +"x" # type: ignore # E: "type: ignore" comment without error code [ignore-without-code] +y # type: ignore # E: "type: ignore" comment without error code (hint: add [name-defined]) [ignore-without-code] [case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning] # flags: --warn-ignores-without-codes --warn-unused-ignores From 19e5ccf3d19dc999899e1a1c94a4ee133beb6827 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 18 Dec 2021 12:21:30 +0000 Subject: [PATCH 10/17] Add testing of positive cases --- test-data/unit/check-errorcodes.test | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index c74e8200ec31..9f0f745e5dd9 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -139,10 +139,14 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi # flags: --warn-ignores-without-codes "x" # type: ignore # E: "type: ignore" comment without error code [ignore-without-code] y # type: ignore # E: "type: ignore" comment without error code (hint: add [name-defined]) [ignore-without-code] +z # type: ignore[name-defined] +"a" # type: ignore[ignore-without-code] [case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning] # flags: --warn-ignores-without-codes --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: --warn-ignores-without-codes From 223da4b8e5b49689f75cdf68b67fe4e1d02b70ce Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 18 Dec 2021 13:03:03 +0000 Subject: [PATCH 11/17] Switch to ignore-without-code control via error-code enablement In preference to adding another specific command line flag. --- docs/source/config_file.rst | 7 ------- docs/source/error_code_list2.rst | 28 ++++++++++++++++++++++++++++ mypy/build.py | 2 +- mypy/options.py | 4 ---- test-data/unit/check-errorcodes.test | 6 +++--- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/docs/source/config_file.rst b/docs/source/config_file.rst index 46a10fa457fb..c34f23d9e169 100644 --- a/docs/source/config_file.rst +++ b/docs/source/config_file.rst @@ -507,13 +507,6 @@ section of the command line docs. Warns about unneeded ``# type: ignore`` comments. -.. confval:: warn_ignores_without_codes - - :type: boolean - :default: False - - Warn about '# type: ignore' comments which do not have error codes. - .. confval:: warn_no_return :type: boolean diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index 1e035fcf7f69..d82d0b6cb5d0 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -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 +--------------------------------------------------- + +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 +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 + # 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 0410cd635b9c..e5c0ec9cfdb8 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -2370,7 +2370,7 @@ def generate_unused_ignore_notes(self) -> None: self.manager.errors.generate_unused_ignore_errors(self.xpath) def generate_ignore_without_code_notes(self) -> None: - if self.options.warn_ignores_without_codes: + 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, diff --git a/mypy/options.py b/mypy/options.py index 12b5930e032a..58278b1580e8 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -49,7 +49,6 @@ class BuildType: "strict_equality", "strict_optional", "strict_optional_whitelist", - "warn_ignores_without_codes", "warn_no_return", "warn_return_any", "warn_unreachable", @@ -144,9 +143,6 @@ def __init__(self) -> None: # Warn about unused '# type: ignore' comments self.warn_unused_ignores = False - # Warn about '# type: ignore' comments which do not have error codes - self.warn_ignores_without_codes = False - # Warn about unused '[mypy-]' or '[[tool.mypy.overrides]]' config sections self.warn_unused_configs = False diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 9f0f745e5dd9..1afcbab3621f 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -136,20 +136,20 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi "x" # type: ignore[name-defined] # E: Unused "type: ignore" comment [case testErrorCodeMissingWhenRequired] -# flags: --warn-ignores-without-codes +# 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 (hint: add [name-defined]) [ignore-without-code] z # type: ignore[name-defined] "a" # type: ignore[ignore-without-code] [case testErrorCodeMissingDoesntTrampleUnusedIgnoresWarning] -# flags: --warn-ignores-without-codes --warn-unused-ignores +# 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: --warn-ignores-without-codes +# 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 From 6f39906ba06d2ad48c0dca241ac0d06d6d0faa30 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 18 Dec 2021 13:03:47 +0000 Subject: [PATCH 12/17] Tweak error message to avoid misleading the user Users who turn ignore-without-code on are likely at first to encounter cases where their existing ignores are too broad. The previous hint spelling was useful, however could encourage users to just ignore everything which was already ignored rather than actually reviewing the code. This change removes the implication that the correct course is to blindly update the ignore comment while still containing the useful information about what is ignored. --- mypy/errors.py | 2 +- test-data/unit/check-errorcodes.test | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/errors.py b/mypy/errors.py index 995ba10c6458..506e57df823e 100644 --- a/mypy/errors.py +++ b/mypy/errors.py @@ -511,7 +511,7 @@ def generate_ignore_without_code_errors(self, codes_hint = '' ignored_codes = used_ignored_lines[line] if ignored_codes: - codes_hint = f' (hint: add [{", ".join(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! diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 1afcbab3621f..36fb19e46537 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -138,7 +138,7 @@ x # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[attr-defi [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 (hint: add [name-defined]) [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] From 34a906f318bab952a90f17e2f16922f28ae22ae3 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Sat, 18 Dec 2021 13:43:57 +0000 Subject: [PATCH 13/17] Remove missed CLI arg spelling of this new option It's been moved to a pure error code style option. --- docs/source/command_line.rst | 9 --------- mypy/main.py | 3 --- 2 files changed, 12 deletions(-) diff --git a/docs/source/command_line.rst b/docs/source/command_line.rst index cadb527b84f6..64c2f8f03b3e 100644 --- a/docs/source/command_line.rst +++ b/docs/source/command_line.rst @@ -440,15 +440,6 @@ potentially problematic or redundant in some way. These two flags let you discover cases where either workarounds are no longer necessary. -.. option:: --warn-ignores-without-codes - - This flag will make mypy report an error for any ``# type: ignore`` comments - which do not have error codes:: - - prog.py:1: error: "type: ignore" comment without error code - - See :ref:`error-codes` for more information. - .. option:: --no-warn-no-return By default, mypy will generate errors when a function is missing diff --git a/mypy/main.py b/mypy/main.py index 37e10745de3e..d765781838cf 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -637,9 +637,6 @@ def add_invertible_flag(flag: str, add_invertible_flag('--warn-unused-ignores', default=False, strict_flag=True, help="Warn about unneeded '# type: ignore' comments", group=lint_group) - add_invertible_flag('--warn-ignores-without-codes', default=False, - help="Disallow '# type: ignore' comments which do not have error codes", - group=lint_group) add_invertible_flag('--no-warn-no-return', dest='warn_no_return', default=True, help="Do not warn about functions that end without returning", group=lint_group) From 14a6fce185ef4afcfca06e51ee3006c3b7cfa763 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Wed, 9 Feb 2022 22:17:59 +0000 Subject: [PATCH 14/17] Add recently defined additional message picked up by merge This appears to have originated in https://github.com/python/mypy/pull/12067 which was picked up by 61e9589f2. TODO: Work out if we should minimise the number of errors that users end up with in this scenario. --- test-data/unit/check-errorcodes.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index 4ba89176d0b4..642828085b53 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -156,7 +156,7 @@ z # type: ignore[name-defined] # 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] +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 From f9e60f9b36eda3443a69481bbffed9e293c87833 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Wed, 9 Feb 2022 22:26:29 +0000 Subject: [PATCH 15/17] Include error code in the heading --- docs/source/error_code_list2.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index d82d0b6cb5d0..1df43548e621 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -257,8 +257,8 @@ except that attempting to invoke an undefined method (e.g. ``__len__``) results 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 ---------------------------------------------------- +Check that ``# type: ignore`` include an error code [ignore-without-code] +------------------------------------------------------------------------- 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 From 7798cbd902b3c6c9a781eba1e5ac857ef5dbe355 Mon Sep 17 00:00:00 2001 From: Peter Law Date: Wed, 9 Feb 2022 22:33:50 +0000 Subject: [PATCH 16/17] Clarify this example --- docs/source/error_code_list2.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index 1df43548e621..566ff53db9ae 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -276,7 +276,10 @@ Example: f = Foo('foo') - # This line has a typo that mypy can't help with as both the expected 'assignment' and 'attr-defined' are silenced + # 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 From ed580e34d3f66c75b1d6e8f4e39552f126aaf222 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 9 Feb 2022 15:08:07 -0800 Subject: [PATCH 17/17] Update docs/source/error_code_list2.rst --- docs/source/error_code_list2.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index 566ff53db9ae..ba5b67a5aeb7 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -260,8 +260,8 @@ while attempting to evaluate an object in boolean context without a concrete imp Check that ``# type: ignore`` include an error code [ignore-without-code] ------------------------------------------------------------------------- -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 +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: