Skip to content

Refactor so Pylinter do not need to be a BaseChecker anymore #5156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Pierre-Sassoulas opened this issue Oct 14, 2021 · 2 comments
Open

Refactor so Pylinter do not need to be a BaseChecker anymore #5156

Pierre-Sassoulas opened this issue Oct 14, 2021 · 2 comments
Labels
Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow

Comments

@Pierre-Sassoulas
Copy link
Member

Current problem

The Pylinter class is very big and does a lot of thing. There's also some meta checks that need information from Pylinter and other management class so they can't be a BaseChecker as they need meta-information not available outside of Pylinter. Another design could separate the coupling between those meta-warning and Pylinter.

Desired solution

Use composition instead of inheritance ? Create a MetaChecker class for those checker ? Find a proper design to decouple this.

Additional context

Meta warning are:

all message that are added with add_message in Pylinter, MessageHandlerMixin or OptionManagerMixin instead of being their own Checker class (ie exhaustive list if you search for add_message in those classes's files).

See discussion.

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow labels Oct 14, 2021
@DanielNoord
Copy link
Collaborator

For reference #3312 (comment).

That would already remove the need for any linting of PyLinter. We would then only need to find a way to register PyLinter's options. That could very well also be done on a potential _MessageStateHandler as that would then be the class that is responsible for emitting messages instead of the PyLinter base class.

@DanielNoord
Copy link
Collaborator

I have just opened the PRs necessary to create _MessageStateHandler and make PyLinter a BaseChecker.

After that two main issues remain:

  1. Where to define the options generated by Run and PyLinter? Do we want to handle them differently, perhaps by letting PyLinter subclass _ArgumentsProvider?
  2. Where to define the messages generated by PyLinter? It might make sense to define some on _MessageStateHandler as that is the class that is now responsible for emitting messages such as deprecated-pragma but I think it might be better to not make _MessageStateHandler a Checker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

No branches or pull requests

2 participants