Skip to content
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

[Feat] Generic Way to trigger a light bulb for a code action on PyLint issues #43

Closed
dciborow opened this issue Apr 11, 2022 · 10 comments · Fixed by #300
Closed

[Feat] Generic Way to trigger a light bulb for a code action on PyLint issues #43

dciborow opened this issue Apr 11, 2022 · 10 comments · Fixed by #300
Labels
feature-request Request for new features or functionality needs PR verification-needed Verification of issue is requested
Milestone

Comments

@dciborow
Copy link
Contributor

I would like a generic way to specify per linter per error code to trigger a light bulb for a code action that is provided in a configuration file. Before finding a truely global way to achieve, this since PyLint handles the majority of my Python static code analysis, I would like to begin here. A subset of PyLint's suggestions could contain a suggested fix. In easy cases, this can be done by triggering a preexisting function in VSCode.

Here are some example PyLint Warnings.

Code VS Action Issue Description
C0301 Format Document line-too-long Line too long (%s/%s)
C0303 Format Document trailing-whitespace Trailing whitespace
C0304 Format Document missing-final-newline Final newline missing
C0305 Format Document trailing-newlines Trailing newlines
C0321 Format Document multiple-statements More than one statement on a single line
C0410 Sort Imports multiple-imports Multiple imports on one line (%s)
C0411 Sort Imports wrong-import-order %s should be placed before %s
C0412 Sort Imports ungrouped-imports Imports from package %s are not grouped
C0413 Sort Imports wrong-import-position Import "%s" should be placed at the top of the module

Here is an example of some slightly more complex Fixes and Actions.

Code VS Action Issue Description
W0611 Delete Line unused-import Unused %s
W0612 Delete Line unused-variable Unused variable %r
W0613 Refactor Function Signature - Remove %r unused-argument The argument in the function definition is deleted, then, anywhere in the code base that the argument was present, it is also removed
W0621 Refactor Variable Name - %r to _%r redefined-outer-name This is a simple opinionated auto-fix suggestion, that should be further configurable by the user
W0622 Refactor Variable Name - %r to my_%r redefined-builtin This is a simple opinionated auto-fix suggestion, that should be further configurable by the user

The HOOK into PyLint, would read the configuration file containing the list of PyLint Codes, and descriptions of the actions in VS Code.

In the above examples the following actions would be used.

  1. Format Document
  2. Sort Imports
  3. Delete Line
  4. Refactor Function Signature
  5. Refactor Variable Name

Some may use a sed like REGEX to perform a transformation, use case camelCase -> snake_case.

  1. Refactor Variable Name /[.][^][.]/(0)_(1)(2)/

If there is interest in developing the hook, I will continue to generate maps between PyLint Codes and VS actions. But I leave no suggested way to format the file, or hook declaration, and look for advice for that from the project owners.

@github-actions github-actions bot added the triage-needed Issue is not triaged. label Apr 11, 2022
@karthiknadig karthiknadig added feature-request Request for new features or functionality needs community feedback and removed triage-needed Issue is not triaged. labels Apr 11, 2022
@brettcannon
Copy link
Member

And even if we don't decide to have some generic way for all linters, we should probably discuss whether we want to accept changes to this extension directly to provide quick fixes.

@dciborow
Copy link
Contributor Author

dciborow commented May 2, 2022

One option to consider would be supporting sarif. https://sarifweb.azurewebsites.net/
But, most Python Linters do not support this out of the box.

Or, even just the more generic "GitHub" logging sytle:
%f:%l:%c: %m

%f = filename
%l = line number
%c = column number
%m = message

And within the message, a suggestion can be placed between ``` like this

```suggestion
RUN conda install -y python=3.8 \
` ``

(space added to ` `` to display properly)

@DanielNoord
Copy link

If supporting a more commonly used standard is indeed preferred (instead of something only VS Code can use) it might make more sense to build this into pylint directly so others can also benefit from this.

@brettcannon
Copy link
Member

Standardized output formats isn't going to be what helps here. The critical piece would be some mechanism to make it so you can hook into the linter results that we provide to get the quick fixes. Basically quick fixes are given the diagnostics, so you're already passed caring about how we got the diagnostic.

@ketozhang
Copy link

ketozhang commented Sep 5, 2022

I'm suggesting an additional action: to ignore the pylint rule for that line by adding the inline comment # pylint: disable=<issue>.

@ketozhang
Copy link

Additionally a second code action option for #pylint: disable-next=<issue>. The use case often being adding a comment will cause line-too-long.

Thoughts? I can work on this. @karthiknadig @luabud

@gilmarGNJ
Copy link

Any news on this?

@karthiknadig
Copy link
Member

karthiknadig commented Feb 21, 2023

@gilmarGNJ The extension now supports code actions. The first set of code actions (based on existing commands) is already done. The one that needed text change, can also be implemented. This is unblocked for community contributions.

@dciborow
Copy link
Contributor Author

I'm able to create #300

@karthiknadig
Copy link
Member

Verification:

  1. Install the latest pylint extension.
  2. create a python file with following content:
class MyObject(object):
    """Test class"""
    pass
  1. In the problems window it should show: useless-object-inheritance
  2. Click on the quickfix provided for that problem.
  3. Code should be updated to:
class MyObject:
    """Test class"""
    pass

@karthiknadig karthiknadig added verification-needed Verification of issue is requested and removed needs community feedback labels Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality needs PR verification-needed Verification of issue is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants