Repository navigation
Support error codes from plugins in options - #19719
ilevkivskyi merged 8 commits into
Conversation
Sorry, something went wrong.
This comment has been minimized.
Mypy has options for enabling or disabling specific error codes. These work fine, except that it is not possible to enable or disable error codes from plugins, only mypy's original error codes. The crux of the issue is that mypy validates and rejects unknown error codes passed in the options before it loads plugins and learns about the any error codes that might get registered. There are many ways to solve this. This commit tries to find a pragmatic solution where the relevant options parsing is deferred until after plugin loading. Error code validation in the config parser, where plugins are not loaded yet, is also skipped entirely, since the error code options are re-validated later anyway. This means that this commit introduces a small observable change in behavior when running with invalid error codes specified, as shown in the test test_config_file_error_codes_invalid. This fixes python#12987.
This comment has been minimized.
| # have loaded. | ||
| options.process_error_codes(error_callback=parser.error) | ||
|
|
||
| options._on_plugins_loaded = on_plugins_loaded |
There was a problem hiding this comment.
I don't like this pattern (callback that stores another callback). Instead you should simply use another way to create a blocker error, e.g. use raise CompileError(...) as a error_callback to process_error_codes() after the plugins are loaded.
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Nice! I added you as a collaborator on the fork, feel free to modify as you see fit!
Sorry, something went wrong.
There was a problem hiding this comment.
thank you-- committed callback refactor
Sorry, something went wrong.
There was a problem hiding this comment.
@ilevkivskyi raising CompileError rather than calling parser.error() has a subtly different result, since only the latter will output CLI usage with the error. Are you sure about this change?
Test expectations fail as follows:
Expected:
usage: mypy [-h] [-v] [-V] [more options; see below] (diff)
[-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...] (diff)
mypy: error: Invalid error code(s): YOLO (diff)
== Return code: 2
Actual:
Invalid error code(s): YOLO (diff)
== Return code: 2
Personally, I think parser.error() is the best for consistency with other parsing error cases. I wouldn't expect a different behavior only because a check needs to be deferred.
Sorry, something went wrong.
There was a problem hiding this comment.
It is fine (and probably even good) to not have the usage: ... part in this error. But I would add the mypy: error: prefix.
But also what you did is not what I wanted, the whole "double-callback" logic is pointless, you can simply call options.process_error_codes() directly in build.py.
Sorry, something went wrong.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Sorry, something went wrong.
Mypy has options for enabling or disabling specific error codes. These
work fine, except that it is not possible to enable or disable error
codes from plugins, only mypy's original error codes.
The crux of the issue is that mypy validates and rejects unknown error
codes passed in the options before it loads plugins and learns about the
any error codes that might get registered.
There are many ways to solve this. This commit tries to find a pragmatic
solution where the relevant options parsing is deferred until after
plugin loading. Error code validation in the config parser, where
plugins are not loaded yet, is also skipped entirely, since the error
code options are re-validated later anyway. This means that this commit
introduces a small observable change in behavior when running with
invalid error codes specified, as shown in the test
test_config_file_error_codes_invalid.
This fixes #12987.