Add command line support to enable/disable error codes #9172

cph-w · 2020-07-19T12:49:38Z

Add the --disable-error-code flag to disable error codes globally.
Add the --enable-error-code flag to enable error codes globally

This is my first time working on this repo, so please let me know anything additional that needs completing.

cph-w · 2020-07-19T12:52:04Z

mypy/errorcodes.py

@@ -3,19 +3,25 @@
 These can be used for filtering specific errors.
 """

-from typing import List
+from typing import Dict, List, Set
 from typing_extensions import Final


 # All created error codes are implicitly stored in this list.
 all_error_codes = []  # type: List[ErrorCode]


This list seemed so far unused, and for my use case was not the ideal data structure, so I added the below dict. If it's preferred I can change to adding the error codes here and iterating through.

JukkaL · 2020-07-22T16:03:34Z

Thanks for the PR! It would be nice to also have --enable-error-code available.

This is a sketch of how it could work:

Add a boolean attribute to ErrorCode called enabled, for example, that controls whether the error code is enabled by default. It defaults to true. If it's false, the error code is disabled by default.
Initially error codes that are disabled by default are in the disabled set.
--enable-error-code will remove an error code from the disabled set.

Also, --disable-error-code X followed by --enable-error-code X on the command line results in X being enabled.

Initially we wouldn't have any error codes that are disabled by default, but you can test this manually by making some error code disabled by default. Tests can use --disable.. followed by --enable.. to test this.

cph-w · 2020-07-22T20:56:14Z

@JukkaL Thanks for getting back to me. Your implementation plan is clear and clean, I'll implement this tomorrow.

cph-w · 2020-07-23T09:30:38Z

@JukkaL I have what looks like a working implementation for this now, but I have failing tests. When I disable error codes in a test it looks like possibly that state is persisting between cases that run subsequently. Do you have an established/preferred way way to clean something like this up?

cph-w · 2020-07-27T17:07:57Z

Implemented a reset for the error codes that get changed, and --enable-error-code. Let me know any feedback or required changes :)

cph-w · 2020-07-29T14:42:06Z

Hey @JukkaL , no rush but looking for feedback on this PR when somebody is free or direction for the next steps to contributing this.

JukkaL

Thanks for the updates! This is almost there, but I'd like to get rid of error code object mutation.

Sorry for the slow response, I'll try to respond faster on the next iteration.

JukkaL · 2020-07-29T15:23:43Z

mypy/errorcodes.py

        self.code = code
        self.description = description
        self.category = category
+        self.default_enabled = default_enabled
+        self.enabled = default_enabled


I don't like having mutable state here. I think that it's better to filter error codes based on the enabled/disabled sets (my other comments contain additional context).

JukkaL · 2020-07-29T15:25:03Z

mypy/errors.py

@@ -351,7 +351,9 @@ def add_error_info(self, info: ErrorInfo) -> None:
        self._add_error_info(file, info)

    def is_ignored_error(self, line: int, info: ErrorInfo, ignores: Dict[int, List[str]]) -> bool:
-        if line not in ignores:
+        if info.code and info.code.enabled is False:
+            return True


You can pass the enabled and disabled error codes to Errors, and we can use them together with the default_enabled attribute to decide whether to ignore them or not.

This way there will be less global mutable state, and it will be easier to have per-file enabled/disabled sets in the future.

JukkaL · 2020-07-29T15:31:20Z

mypy/main.py

+        invalid_error_codes = process_error_codes(disabled_codes, enable=False)
+
+        if invalid_error_codes:
+            parser.error("Disabled error codes provided invalid values: (%s)" %


This error message is a bit verbose. What about "Invalid error code(s): %s"?

JukkaL · 2020-07-29T15:31:34Z

mypy/main.py

+        invalid_error_codes = process_error_codes(enabled_codes, enable=True)
+
+        if invalid_error_codes:
+            parser.error("Enabled error codes provided invalid values: (%s)" %


Similar to above.

JukkaL · 2020-07-29T15:32:23Z

mypy/test/testcheck.py

+                options.disable_error_code).union(set(options.enable_error_code))
+            for code in altered_error_codes:
+                error_code = errorcodes.error_codes[code]
+                error_code.enabled = error_code.default_enabled


Once error code objects aren't mutated, this can be removed.

JukkaL · 2020-07-29T15:32:42Z

test-data/unit/check-flags.test

@@ -1501,3 +1501,48 @@ class ShouldNotBeFine(x): ...  # E: Class cannot subclass 'x' (has type 'Any')
 disallow_subclassing_any = True
 \[mypy-m]
 disallow_subclassing_any = False
+
+[case testDisableErrorCode]


Add tests for invalid error codes?

JukkaL · 2020-07-29T15:34:50Z

docs/source/command_line.rst

+
+    This flag allows enabling one or multiple error codes globally.
+
+    NOTE: Will override disabled error codes from the --disable-error-code


Nist: For consistency, don't use all caps (above we use "Note: ..."). Use a full sentence, such as "This will ...".

JukkaL · 2020-07-29T15:36:43Z

mypy/main.py

@@ -984,6 +1012,20 @@ def process_cache_map(parser: argparse.ArgumentParser,
        options.cache_map[source] = (meta_file, data_file)


+def process_error_codes(codes: Iterable[str], enable: bool) -> List[str]:


When error code code objects aren't mutated, this can go away.

ofek · 2020-08-03T16:10:58Z

Need rebase btw

Add ability to enable error codes globally from the command line Add ability to disable error codes globally from the command line Enabling error codes will always override disabling error codes Update documentation to include new flags

cph-w · 2020-08-03T21:34:51Z

@JukkaL Switched back to passing sets of error codes to Errors, addressed your other comments. Let me know if there's anything else. Sorry for the short delay getting back to you, was moving house at the weekend and running without internet, will try to respond quickly to any further required iteration.

JukkaL

Thanks for the updates! Looks good. This will be a useful feature and let us add optional checks easily, without having to add a large number of individual command-line flags.

cph-w changed the title ~~[#8975] Add support for enable/disable error codes~~ [#8975] Add command line support to enable/disable error codes Jul 19, 2020

cph-w commented Jul 19, 2020

View reviewed changes

cph-w changed the title ~~[#8975] Add command line support to enable/disable error codes~~ [#8975] Add command line support to disable error codes Jul 19, 2020

JukkaL mentioned this pull request Jul 22, 2020

Distinguish redundant-expr warnings from unreachable warnings #9125

Merged

cph-w changed the title ~~[#8975] Add command line support to disable error codes~~ [#8975] Add command line support to enable/disable error codes Jul 28, 2020

JukkaL reviewed Jul 29, 2020

View reviewed changes

cph-w force-pushed the issue#8975_add-support-for-disabling-error-codes branch from e7333a2 to 1382f44 Compare August 3, 2020 20:18

gvanrossum changed the title ~~[#8975] Add command line support to enable/disable error codes~~ Add command line support to enable/disable error codes Aug 3, 2020

cph-w requested a review from JukkaL August 8, 2020 14:49

JukkaL approved these changes Aug 10, 2020

View reviewed changes

JukkaL merged commit d0711bd into python:master Aug 10, 2020

ofek mentioned this pull request Sep 9, 2020

warn_no_return = False: handle implicit "return None" (not ignoring return type) #7511

Closed

hauntsaninja mentioned this pull request Sep 13, 2020

Add support for per-module enabling/disabling of error codes #9440

Closed

JukkaL mentioned this pull request Jan 6, 2021

Consider adding strict-bool mode #8363

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add command line support to enable/disable error codes #9172

Add command line support to enable/disable error codes #9172

cph-w commented Jul 19, 2020 •

edited

cph-w Jul 19, 2020

JukkaL commented Jul 22, 2020

cph-w commented Jul 22, 2020

cph-w commented Jul 23, 2020

cph-w commented Jul 27, 2020 •

edited

cph-w commented Jul 29, 2020

JukkaL left a comment

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

JukkaL Jul 29, 2020

ofek commented Aug 3, 2020

cph-w commented Aug 3, 2020

JukkaL left a comment


		This flag allows enabling one or multiple error codes globally.

		NOTE: Will override disabled error codes from the --disable-error-code

		@@ -984,6 +1012,20 @@ def process_cache_map(parser: argparse.ArgumentParser,
		options.cache_map[source] = (meta_file, data_file)


		def process_error_codes(codes: Iterable[str], enable: bool) -> List[str]:

Add command line support to enable/disable error codes #9172

Add command line support to enable/disable error codes #9172

Conversation

cph-w commented Jul 19, 2020 • edited

Choose a reason for hiding this comment

JukkaL commented Jul 22, 2020

cph-w commented Jul 22, 2020

cph-w commented Jul 23, 2020

cph-w commented Jul 27, 2020 • edited

cph-w commented Jul 29, 2020

JukkaL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofek commented Aug 3, 2020

cph-w commented Aug 3, 2020

JukkaL left a comment

Choose a reason for hiding this comment

cph-w commented Jul 19, 2020 •

edited

cph-w commented Jul 27, 2020 •

edited