Repository: python/mypy
Pull request: Add an option to require ignore comments have error codes
### Description

Fixes https://github.com/python/mypy/issues/11509

> **Pitch** `# type:ignore` comments risk suppressing an unintended error, which is why it's preferred to specify the exact error code you're suppressing

This introduces a new error code `ignore-without-code` which achieves that. ~I've gone with naming this `warn-` rather than `disallow-` since the ignores are still obeyed, just alerted about (and also as the most closely related options are also `warn-` style).~ This is disabled by default but can be controlled via the `--enable-error-code` machinery.

This new option is aware of `--warn-unused-ignores` (since they're likely to be used together) such that we don't emit warnings about a lack of error codes if the ignore is unused and that option is turned on.

~One thing I'm not sure about is what should happen for ignore comments that apply to a whole file. I've raised this on that issue, so discussion should probably happen there.~ Currently the new option obeys that instruction to ignore the file.

### Test Plan

Some manual testing, plus automated tests added.

### Code Review

~I'm fairly happy with the code approach here, though I'm not at all sure that I've put the command line option in the right places. I also suspect I've missed some places that would need updating in the docs. Guidance around the proper places to put the docs/CLI arg/etc. is most welcome.~ (I believe I've now caught everywhere that this needs to go)

Problem statement:
option to enforce error codes in `type: ignore` comments
**Feature**
it would be cool if there was an option like `--disallow-ignore-without-code` or something that enforces error codes in `type: ignore` comments

**Pitch**
`# type:ignore` comments risk suppressing an unintended error, which is why it's preferred to specify the exact error code you're suppressing

Hints from issue discussion:
https://github.com/KotlinIsland/basedmypy/issues/11
Would also be useful if we could use # type: ignore below comments in the python file.  Seems the # type: ignore is ignored if there is a license header in this format.  Might be a separate bug/issue here.

```python
# license copyright info
"""This is a file""
# type: ignore
```

This works.
```python
# type: ignore
# license copyright info
"""This is a file""
```

The above request seems more like a flake8 check?  Maybe in https://pypi.org/project/flake8-annotations/ it would be simpler to implement?
@asears What are you talking about? that code looks like a syntax error with an unterminated triple-quoted string.

There are no types in that snipped so how could there be a type error?
Just a typo in this github issue, in the full code there is a type error that isn't ignored with the first case and no typos/flake8 warnings.   The problem can be reproduced by introducing a type error and trying to silence it with this:

```python
# license copyright info
"""This is a file"""
# type: ignore
```

Adding the type: ignore to top of file works fine.
@asears this seems unrelated to the proposal. Please open a new issue if you'd like to see a change in mypy.
@asears are you asking for a whole file type ignore?

In Kotlin annotations can target the file `@file:Suppress("UNCHECKED_CAST")`

For mypy if that's what you want configure your pyproject.toml to specify the options you want, for eg:

pyproject.toml:
```toml
[[tool.mypy.overrides]]
module = "my.module"
ignore_errors = true
```
How should this option interact with file-level `# type: ignore` comments? Should those be exempt? (Currently it's not possible to specify error codes to ignore per-file, so it feels like it would be hard to require that those ignores have error codes)
@PeterJCLaw hmm, perhaps a separate option to prohibit file-level ignore comments?
I would not worry about those. If you're using a file-level ignore, you probably want to ignore everything.
Kotlin handles per code file wide suppression
`@file:Suppress("UNCHECKED_CAST")`, and I agree.

Commit message: Clarify this example

Patch to review:
diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst
index 1e035fcf7f69..566ff53db9ae 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 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 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 e636b606ed8f..32fc23500511 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
 
@@ -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/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 ]


Code review comments:
diff_hunk: @@ -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 an ``# type: ignore`` comment does not specify any error codes.
+This clarifies the intent of the ignore as well as ensuring that only the
path: docs/source/error_code_list2.rst
text: @user1:
```suggestion
Warn when a ``# type: ignore`` comment does not specify any error codes.
This clarifies the intent of the ignore and ensures that only the
```
Sorry, one more change