PR: pylint: Add support for multiple linters#25885
PR: pylint: Add support for multiple linters#25885rear1019 wants to merge 3 commits intospyder-ide:masterfrom
Conversation
Add support for multiple linters in the pylint ("Code Analysis") plugin:
- Add API to define how to execute a linter and parse its output
- Add support for ruff and mypy, in addition to the previously used
pylint
- The linters to be executed can be selected in Preferences
- "Run code analysis" executes all selected linters and displays their
results in a tree widget (grouped by linter)
- The plugin is now tied to the currently selected file in the editor:
The ability to select a file for analysis has been removed. This
(greatly) simplifies code and state handling.
- The plugin doesn't save results anymore (see above for reasoning).
|
|
||
|
|
||
| # NOTE Update spyder/config/main.py when the list is changed | ||
| LINTERS = (PylintLinter, RuffLinter, MypyLinter) |
There was a problem hiding this comment.
We could add a @register() decorator, however, changes to spyder/config/main.py still need to be performed manually.
| # end_column: int | None = None | ||
|
|
||
|
|
||
| class Linter: |
There was a problem hiding this comment.
Instead of defining multiple getters for command, environment and working directory, we could define a single get_qprocess(). The former approach has the benefit that we can define sensible defaults.
Docstrings will be added after review.
| return code not in (0, 1) | ||
|
|
||
|
|
||
| class PylintLinter(Linter): |
There was a problem hiding this comment.
All code to to run pylint has been copied unchanged from main_widget.py
| return env | ||
|
|
||
| @classmethod | ||
| def get_working_dir(cls, file: str) -> str: |
There was a problem hiding this comment.
In the future, it might be sensible to also pass project_dir if some tool requires it.
|
|
||
| @classmethod | ||
| def get_environment(cls) -> QProcessEnvironment: | ||
| env = QProcessEnvironment() |
There was a problem hiding this comment.
Using an empty environment is based on existing code for pylint, though I am not sure if it's the "right" thing to do. Users might expect that settings are picked up from the environment (e.g. XDG_*_DIR on Unix). There might also be variables which are required for a linter to run (see APPDATA and USERPROFILE for pylint).
On the other hand, using the system's environment might break things as well (especially any PYTHON* variable).
| vlayout.addStretch(1) | ||
| self.setLayout(vlayout) | ||
| def setup_page(self) -> None: | ||
| linters_group = QGroupBox(_("Tool selection")) |
There was a problem hiding this comment.
I am not sure if it should say "Tool" or "Linter". Likewise in the tooltip below, and in DESCRIPTION_WHEN_EMPTY in main_window.py.
|
Forgot to mention: Tests are not adapted yet 😃 Will be done after initial review. |
|
@ccordoba12 I just wanted to query if there is any blocker. Please let me know if you have any questions or suggestions. |
1 participant
Description of Changes
This PR adds support for multiple linters in the pylint ("Code Analysis") plugin. See commit message for more details.
The first screenshot below shows how the plugin looks in the initial version of the PR. An alternative is shown in the second screenshot. A table instead of a tree view could be used as well.
@ccordoba12 Note: I removed the ability to select a file in the plugin. I don't think this feature is sensible given that we want to move LSP results from the
Source > Show warning/error listmenu to the plugin (suggested in #25620 (comment)). This also (greatly) simplifies code.Issue(s) Resolved
Fixes #25620
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct:
rear1019