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

Support ruff 0.1.0 #48

Closed

Conversation

kstrauser
Copy link
Contributor

ruff 0.1.0 has 2 breaking changes that prevent python-lsp-ruff from working as-is:

  • --output is now spelled --output-format
  • Unsafe fixes are no longer applied unless explicitly enabled with --unsafe-fixes

This supports both of those changes so that users can upgrade to the newest ruff. However, this is going to be a pain in the neck for users: those fixes aren't backward compatible with older versions of ruff, so they'll need to upgrade python-lsp-ruff and ruff at the same time. I'm not sure what the alternative here. Maybe add a new mechanism to run ruff --version to see what command line args can be passed in? Gut it out because the new semver policy won't make future changes like this without bumping the minor version (which, in fairness, they did today)? I'll leave those decisions to you, oh wise maintainers! For now, I offer my humble patch to keep the spice flowing.

In astral-sh/ruff#7984, ruff removed support for
the "--output" command line option, and now requires "--output-format"
instead.
Ruff 0.1.0 no longer applies unsafe fixes without explicit opt-in; see
astral-sh/ruff#7769 . This gives users a
mechanism to enable them.
@kstrauser
Copy link
Contributor Author

Hmm. On other reflection, I guess letting Poetry and friends handle that is the right decision. Maybe it'd make sense to release a version 1.5.3 that's identical to 1.5.2, but with pyproject.toml set to only work with ruff < 0.1.0, then another (1.5.4? 1.6.0?) that includes this change. Then future poetry add python-lsp-ruff calls in new environments will work.

It seems like resolvers like poetry would consider either 1.5.2 or 1.6.x (but not 1.5.3) to work with ruff 0.1.0, though. Ugh. I predict this is going to be a bit of a hassle for everyone, no matter how this is addressed.

@zanieb
Copy link

zanieb commented Oct 17, 2023

Sorry the format -> output-format changes broke your LSP! We try to have much longer deprecation periods but this one was causing problems and we didn't want to wait until v0.2.0. Thanks for putting up a fix so quickly :)

p.s. If you have any ideas for how to increase visibility of the deprecation warnings please let us know.

@jhossbach
Copy link
Member

jhossbach commented Oct 17, 2023

My idea would be to:

  • Release v1.5.3 that limits to ruff<0.1.0 for the old version, and
  • Release v1.6.0 with this PR with the breaking changes and ruff>=0.1.0, <0.2.0
    That would mostly solve the issue as anyone who does not want to update could use v1.5.3 to keep the old ruff, right?

@kstrauser
Copy link
Contributor Author

Makes sense to me. There’s still the case where someone will have the version of this project pinned, but not ruff’s (eg because they installed this project directly and ruff got pulled in as a dependency). The next time they upgrade, they’ll end up with a newer version of ruff than their python-lsp-ruff can handle. There’s not a great way to prevent that from this end, though.

@jhossbach
Copy link
Member

@kstrauser I think there might be a problem with this specific note in https://docs.astral.sh/ruff/configuration/#fix-safety saying that all fixes are shown when using --output-format=json, regardless of whether --unsafe-fixes was given or not. I will check if this will indeed be ignored when trying to format a document

@jhossbach
Copy link
Member

jhossbach commented Oct 18, 2023

I checked and this does not seem to be a problem since we explicitly set the list of fixable codes when formatting. However astral-sh/ruff#7838 introduced a regression since we use stderr to check if the process was succesfull

edit: It does however create a problem with the code actions since unsafe-fixes is ignored when asking for fixes, so all actions are shown even if they are unsafe and it was not enabled.

@zanieb
Copy link

zanieb commented Oct 18, 2023

However astral-sh/ruff#7838 introduced a regression since we use stderr to check if the process was successful

Just wondering, why are you using stderr instead of the return code?

It does however create a problem with the code actions since unsafe-fixes is ignored when asking for fixes, so all actions are shown even if they are unsafe and it was not enabled.

You can filter out the unsafe fixes by inspecting the fix.applicability field in the JSON output.

@jhossbach
Copy link
Member

I am working on a fix, we didn't have an issue before with stderr, that's the only reason.

@jhossbach
Copy link
Member

@kstrauser
Copy link
Contributor Author

@jhossbach The “Allow edits from maintainers.” box is checked. I haven’t used that feature before. Can you push to it now?

@kstrauser
Copy link
Contributor Author

To simplify things and make sure there aren't any permission problems, I've also invited you as a collaborator to my fork.

@jhossbach
Copy link
Member

jhossbach commented Oct 19, 2023

Ah, the issue was that we had different branch names. I accidentally pushed my branch name to your repo, but I will rebase and push to this branch right away rename the branch kstrauer/support-ruff-0.1.0 to just support-ruff-0.1.0 since the naming is getting confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants