Feat code actions

[betaboon]

resolves #3

i tried my hand at surfacing the fixes that ruff proposes as code-actions.

i didn't yet add a fixAll action, as i was unsure about the semantics:

• should "fixall" apply only to all the fixes on the currently selected line or on the whole file?

[betaboon]

on the topic of code-action vs formatting:

I'm coming from pyls-isort which implements import-sorting as textDocument/formatting and i got very used to having an autocmd that runs vim.lsp.buf.format on save.

I'm wondering if python-lsp-ruff should also implement a textDocument/formatting but only apply a limited subset of all of ruffs proposed fixes, eg import-sorting due to I001.

any opinions on that ?

[jhossbach]

Nice changes!
On the semantics: "Fixall" is supposed to format the whole document, not just the current line.

On the formatting vs. code action part:
I had asked a similar question in #3, I like the idea of using formatting to e.g. sort imports.
Ruff-LSP has the specific code action "Fix imports" for that, but I think we can do one better and let the user specify a set of fixable errors that should be fixed during formatting.
We could call this option ruff.formatCodes.
We also need to think about the default codes, either we completely let the user decide or we select a set of codes. What do you think?

[betaboon]

thanks for the quick response.

(sorry in advance for the amount of text)

i would like to keep the scope of this PR as it is (that is, exposing the individual fixes as proposed by ruff).

i would be willing to send some more PRs, some ideas are described further down.

this PR

this is what i want to do in the scope of this PR:

• some tests wouldnt hurt
• the ci should pass

fixAll

when having fixAll fix all the problems in the current file, i guess it would be best to let ruff do that itself, instead of trying to do that with WorkspaceEdit.

reason being: ruff already respects fixable coming from the config.

formatting

i like the idea of formatCodes.
maybe calling it format would keep it consistent with ignore and select tho?

regarding the default-codes, i think it makes sense to define a default for the formatCodes that can then be overridden by the user. (eg just [ "I" ]?)

repo "improvements"

I'm proposing the following changes to the repo itself:

• replace isort in pre-commit config with ruff
• replace setup.py+setuptools with pdm for dependency-pinning and building
• setup ci to automatically build+release to pypi after merge to main (including changelog generation)

code improvements

Since mypy is a already setup in pre-commit, I'm proposing:

• setting mypy to strict via pyproject.toml
• getting the codebase mypy-happy
• using dataclasses or typeddicts for all the json structures.

[jhossbach]

Nice ideas in all, let's create new issues from your ideas to discuss. Specifically for the code base improvements I think we should discuss this to be done in python-lsp-server as well rather than just python-lsp-ruff.
Same for using pdm which I am unfamiliar with.

[betaboon]

i do agree, that it would be best if python-lsp-server itself is typed, but i guess that's a whole different kind of beast.

what are your opinions on what i wrote regarding fixall and formatting?

[jhossbach]

You're right about the "Fix All" part, but we should nevertheless respect the user pylsp config settings, e.g. extendIgnore.
As for the formatting I have nothing to add except that we can disable formatting from ruff if the list is set to be empty.

[jhossbach]

I'll start creating some issues.

[betaboon]

fyi: i just found out about lsprotocol which is coming from upstream (microsoft), also uses cattrs for serialization and has all the classes for the payloads defined.

so i changed this PR accordingly

[jhossbach]

Yeah, ruff-lsp uses the same package. I wanted to play around with it for a while now, and I'd love to see pylsp being written with that in the future

[betaboon]

ok. this now has some more code-actions:

• fix code
• disable code
• organize imports
• fix all

[jhossbach]

Hmm, for some reason I can't reproduce the issue that mypy is reporting even when using 3.8. Not sure whether this poses a problem, but we should fix testing Found out how to change the python-version in mypy properly (I don't use mypy that often). We can use typing.List in plugin.py:291, I just tested locally and cattrs is unimpressed whether we use list or typing.List. What is left to do is fixing the pytest tests and writing additional checks for the code actions

[betaboon]

sorry for the long delay, i was caught up in things.
the typing.List should actually not be required due to the from __future__ import annotations oO

I'll try to get the tests done sometime within the next week.

i was trying to get the formatting stuff in here also. but then i figured out that seems to require some more refactoring. so that's out of scope here now and will come as a separate PR.

[jhossbach]

Yeah , good idea moving the code format to another PR.
I fixed the tests on my side, but the Github runners seem not to agree with me. The problem before was that run_ruff returns a str that is not understood by the cattrs converter. json.loads works, but it expects the string not to be empty, thus requiring the check before parsing.
Can you see if you find the problem?

[betaboon]

I will try to get this sorted.

quick question tho: is it possible to change the configuration so that the pipeline run doesnt need your approval everytime?
otherwise we're time coupled, which doesnt help :P

[jhossbach]

Found the issue... ruff v0.0.260 introduced a regression. I suspect a different syntax in the json format output but I'll look into it now.

[jhossbach]

I reverted from using __future__.annotations as this created the issues.

[jhossbach]

I added some tests for the code actions, PTAL if you see anything else @betaboon

[betaboon]

I'm just coming back to this seeing that you got pretty much everything done. awesome. thanks a bunch.

I'm still curious why from __future__ import annotations doesnt seem to work.
I'm using that in all my projects and that just works :/

edit:
now i know, I'm always only supporting >=3.9. that's why i never ran into this issue...

[jhossbach]

Sorry, I forgot to push the new tests. Please review the unit tests if anything is missing, otherwise let's get this pushed!

[betaboon]

i cleaned up the branch.
i think it's ready now.

[Shane-XB-Qian]

seems it did not support some kinds of err, right?
e.g:

def f():
    pass


ff()

would not prompt code action for ff().

[jhossbach]

@Shane-XB-Qian That's because ruff doesn't provide any fixes for ff(), see https://beta.ruff.rs/docs/rules/ for a list of rules that are fixable