feat: pyproject whitelist #659
Conversation
|
What do you think about this type of implementation? I first thought about adding |
|
Thanks! I'm traveling until next weekend so I may be slow to give a full review. |
| @@ -371,6 +375,11 @@ def parse_config_file( | |||
| ) | |||
|
|
|||
|
|
|||
| @functools.lru_cache | |||
| def get_all_error_codes() -> FrozenSet[str]: | |||
There was a problem hiding this comment.
I do not really like this function. I think that, ideally, this should be done by using the registry option populated within __init_subclass__ created by ConfigOption. However, all the error codes are deriving from BooleanOptions. But not only error codes, but also some other boolean based options.
A solution could be creating a new class deriving from BooleanOptions that would only contain all the error codes in registry. This way, the logic splits the boolean options between error-based and pure config based. And therefore, registry would be populated with our error codes. I am not sure if this makes sense, but it is also a bigger rework that might not be interesting. Maybe you know better ways of accessing all the error codes without using this function that I wrote :)
There was a problem hiding this comment.
I think that's actually a good idea and shouldn't be too difficult to implement: add a new subclass of BooleanOption that is only for error codes. Might also play better with pyanalyze's functionality for registering new error codes from plugins.
There was a problem hiding this comment.
Thinking about it more that feels like unnecessary extra complexity. I'll push a few changes and then merge this.
pyanalyze/options.py
Outdated
|
|
||
| if disable_all_default_error_codes: | ||
| if not enabled_error_codes: | ||
| warnings.warn( |
There was a problem hiding this comment.
This would be the only place we use warnings.warn in pyanalyze. I can see a use case: disable all error codes in the whole project, but enable some specific error codes only for some submodules. So I think we should just remove the warning.
| @@ -371,6 +375,11 @@ def parse_config_file( | |||
| ) | |||
|
|
|||
|
|
|||
| @functools.lru_cache | |||
| def get_all_error_codes() -> FrozenSet[str]: | |||
There was a problem hiding this comment.
I think that's actually a good idea and shouldn't be too difficult to implement: add a new subclass of BooleanOption that is only for error codes. Might also play better with pyanalyze's functionality for registering new error codes from plugins.
This PR implements the "white-list" feature for
pyproject.toml. Addingdisable-all = truewill automatically disable all rules but the ones indicated in thepyproject.toml. While I was doing it, I thought that maybedisable-all-default-ruleswould be more explicit, but I left thedisable_allto be more consistent with the CLI implementation.Regarding the tests, tell me if it is needed to write some.
About the implementation itself, maybe there might be better ways of doing. Specially for the function that gets all error codes. Maybe there is a util-based function that already does that.
Linked to #648