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

Add support for Ruby LSP as a built-in add-on #630

Merged
merged 15 commits into from
Jun 23, 2024
Merged

Add support for Ruby LSP as a built-in add-on #630

merged 15 commits into from
Jun 23, 2024

Conversation

searls
Copy link
Contributor

@searls searls commented May 17, 2024

Release plan

  • Add a Ruby LSP addon as a formatter and a (pull) diagnostic provider
  • Add a test for the addon, using a modified test server helper that relies on ruby_lsp internals
  • Drop support for Ruby 2.7, so we can test the LSP plugin
  • Get CI passing again
  • Validate this all works in VS Code with the latest version of Ruby LSP
  • Cut a patch minor release

Background

After talking to @vinistock at Kaigi, I want to take a stab at advertising a working Ruby LSP addon from within the gem so that Ruby LSP users can get standard support without running the LSP themselves (or installing a custom extension, like our VS Code extension

I'm trying and failing to get this end-to-end working in an example app, so I'm trying to take a step back and just get this test passing. The result's response is nil, but (one assumes?) it shouldn't be. More importantly, WrapsBuiltinLspStandardizer#run_formatting isn't being invoked, so my attempts to set the formatter to `standard is probably not working, either.

To replicate the failure, you should be able to run:

$ bundle
$ bundle exec m test/ruby_lsp_addon_test.rb

@st0012
Copy link

st0012 commented May 17, 2024

It's failing because Ruby LSP doesn't format files outside of the workspace path. You can workaround that with:

-    with_server(source) do |server, uri|
+    with_server(source) do |server, _uri|
+      uri = URI(File.join(server.global_state.workspace_path, 'fake.rb'))
       server.process_message(
         id: 1,
-        method: "textDocument/formatting",
-        params: {textDocument: {uri: uri}, position: {line: 0, character: 0}}
+        method: 'textDocument/formatting',
+        params: { textDocument: { uri: uri },
+                  position: { line: 0, character: 0 } }
       )

However, it'd then actually try to run formatting with that file, which will result to another error since the file doesn't actually exist.

So you'll likely need additional setup to:

  • Create an actual file with the target source (maybe under path_to_standard/tmp/fake.rb)?
  • Run the test against that path
  • Remove the file / or tmp/ altogether

@searls
Copy link
Contributor Author

searls commented May 17, 2024

@st0012 thanks Stan! Got through that and have it formatting correctly now.

Question: can I safely rely on Dir.pwd being the workspace directory, as I do with my custom extension?

Question: this test is now failing b/c the range being returned ends on line 19, column 19, even though the source string is only 2 lines long and that second line is only 5 or 6 characters. Since the addon is not providing the range, is it a bug?

@searls
Copy link
Contributor Author

searls commented May 20, 2024

I'm unsure the cause of the test failure this introduces, but just looking at the addon stuff, does this look right? @vinistock, @st0012?

@searls
Copy link
Contributor Author

searls commented May 22, 2024

Looking into this failure, I have very few good answers.

AFAICT, if one executes require "ruby-lsp/internal" in our test suite, then the CLI test will start failing, with the odd behavior of suddenly being incapable of autofixing Lint/TrailingCommaInAttributeDeclaration

The test will print this to stderr:

tmp/cli_test/subject.rb:18:19: Lint/TrailingCommaInAttributeDeclaration: Avoid leaving a trailing comma in attribute declarations.

And then the test will fail because the shelled-out run will exit non-zero.

This is a bit of a mystery to me. Like, the best explanation I can imagine is that loading ruby_lsp/internal causes some Rubocop something-or-other to be loaded which in turn mutates the default configuration set of Lint/TrailingCommaInAttributeDeclaration so as to flag it as not autocorrectable?

Extremely odd failure

@searls searls changed the title Spike: Ruby LSP Addon Add support for Ruby LSP as a built-in add-on May 23, 2024
@searls
Copy link
Contributor Author

searls commented May 23, 2024

Hey @vinistock @st0012, I'm trying to validate this actually works by setting up a project pointing at my local copy using:

  gem "standard", path: "/path/to/standardrb/standard"

But Ruby LSP doesn't seem to pick it up as a formatter/linter option.

screenshot-2024-05-23-11h16m16s

(If I puts something at the end of the addon's activate method, I'll see it blow up in the Ruby LSP output tab, so I know it's being required, however)

Question: do you have any idea why this might not be working when the plugin is a local path-sourced gem?

Meanwhile, if I change my dependency to look at this github branch:

gem "standard", github: "standardrb/standard", branch: "ruby-lsp-spike"

And then I check the output tab of VS Code, I get a stack trace without an error message:

2024-05-23 11:30:02.677 [info] (grog) /Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/standard/plugin/creates_runner_context.rb:7:in `call'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/standard/plugin/merges_plugins_into_rubocop_config.rb:29:in `call'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/standard/plugin/combines_plugin_configs.rb:11:in `call'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/standard/creates_config_store.rb:22:in `block in call'

<internal:kernel>:90:in `tap'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/standard/creates_config_store.rb:19:in `call'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/standard/builds_config.rb:32:in `call'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/ruby_lsp/standard/wraps_built_in_lsp_standardizer.rb:15:in `init!'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/ruby_lsp/standard/wraps_built_in_lsp_standardizer.rb:11:in `initialize'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/ruby_lsp/standard/addon.rb:15:in `new'

/Users/justin/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/bundler/gems/standard-e7d5e49aafd2/lib/ruby_lsp/standard/addon.rb:15:in `activate'

So I'm kind of at a loss as to how to actually test the plugin locally, as a result.

@st0012
Copy link

st0012 commented May 23, 2024

It looks like the error was caused by Standard::VERSION not being defined when it's called here.
Adding require_relative "../../standard/version" makes everything works as expected.

Screenshot 2024-05-23 at 22 31 16

And for some reason the addon's error reporting mechanism didn't work as expected as the error message is not displayed. I'll give it a look when I'm back to work.

@andyw8
Copy link
Contributor

andyw8 commented May 23, 2024

And for some reason the addon's error reporting mechanism didn't work as expected as the error message is not displayed.

This should be solved by Shopify/ruby-lsp#2085

@searls
Copy link
Contributor Author

searls commented May 24, 2024

Thank you both for the above. I fixed the constant not defined error, but even after clearing it I'm seeing the same behavior where the plugin isn't recognized as a formatter

Setting value of "rubyLsp.formatter" to "standard" yields the problem:

> Value is not accepted. Valid values: "auto", "rubocop", "syntax_tree", "none".

Additionally, when I choose "format document" from the command pallete, I get "There is no formatter for 'ruby' files installed."

Here's a sample repo to demo the config:
https://github.com/searls/ruby-lsp-standard-example

@andyw8
Copy link
Contributor

andyw8 commented May 24, 2024

@searls You probably need to add standard to the enum here.

But the extension shouldn't really have to know about specific addons, so we probably need to re-think this approach.

@searls
Copy link
Contributor Author

searls commented May 25, 2024

@andyw8 ahh, yes, if it's just hard-coded now that won't really scale to third-party add-ons. I was assuming the extension was phoning home to the LSP and then updating the configuration schema dynamically (somehow? Think I was conflating it with WorkspaceConfiguration API, which I doubt can be used for that purpose.)

It might be necessary to just relax that configuration property in the schema to allow it to be set to any value?

@andyw8
Copy link
Contributor

andyw8 commented May 27, 2024

Looking into some options: Shopify/ruby-lsp#2092

Copy link
Contributor

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@andyw8
Copy link
Contributor

andyw8 commented Jun 21, 2024

@searls standard is now available as a formatter option: Shopify/ruby-lsp#2092

@searls
Copy link
Contributor Author

searls commented Jun 23, 2024

[UPDATE: Nevermind, got it running locally OK. Forgot I had to enable the format & diagnostic features on rubyLsp.enabledFeatures]

@andyw8 is there any way I ought to be able to test this now?

I tried pointing "rubyLsp.branch": "main" since the gem hasn't been released since you merged Shopify/ruby-lsp#2092, but of course that's based on the vscode extension's package.json, so then I cloned and ran that in a host extension but, try as I might, even though I can get the following config to clear warnings/errors:

{
  "[ruby]": {
    "editor.defaultFormatter": "Shopify.ruby-lsp"
  },
  "rubyLsp.formatter": "standard",
  "rubyLsp.linters": [
    "standard"
  ]
}

… I don't get any diagnostics and I still get "There is no formatter for 'ruby' files installed." when I try to manually invoke the Format Document command

@searls searls force-pushed the ruby-lsp-spike branch 2 times, most recently from 8ddcdc6 to 504a4e3 Compare June 23, 2024 15:22
@searls searls merged commit 710cb6f into main Jun 23, 2024
4 checks passed
@searls searls deleted the ruby-lsp-spike branch June 23, 2024 15:34
@searls
Copy link
Contributor Author

searls commented Jun 23, 2024

Just released this basic version (without code actions) as v1.38.0

@searls
Copy link
Contributor Author

searls commented Jun 23, 2024

@vinistock @andyw8 if you would be so kind, let me know when a version of the vscode extension has shipped with the standard enum present and I'll go to town testing this more thoroughly, adding it to our docs, and so forth

@andyw8
Copy link
Contributor

andyw8 commented Jun 24, 2024

@searls you can test it by checking out the main branch, selecting the vscode workspace and, then selecting Run and Debug:

https://github.com/Shopify/ruby-lsp/blob/main/CONTRIBUTING.md#live-debugging

Everyone on the team is at a company event this week, so there may be a little delay until the next release.

@searls
Copy link
Contributor Author

searls commented Jun 24, 2024

Heads up I also merged #636 which adds Code Action support (and duplicates half of Ruby LSP's rubocop integration in the process)

@andyw8
Copy link
Contributor

andyw8 commented Jul 6, 2024

@vinistock @andyw8 if you would be so kind, let me know when a version of the vscode extension has shipped with the standard enum present and I'll go to town testing this more thoroughly, adding it to our docs, and so forth

This shipped today: https://github.com/Shopify/ruby-lsp/releases/tag/vscode-ruby-lsp-v0.7.6

@searls
Copy link
Contributor Author

searls commented Jul 6, 2024

Thanks @andyw8. I've verified everything works, and so have updated these docs:

I also just published a blog post that I hope you all will appreciate as supporting the vision that @vinistock and others have laid out for the Ruby LSP project.

Additionally, I'd love if you highlights this addon on your repo's README along with the others, since the fact that this one is built-in will make discovery harder for Ruby LSP users (especially in the presence of our own bespoke LSP server and VS Code plugin).

@st0012
Copy link

st0012 commented Jul 6, 2024

I've created Shopify/ruby-lsp#2263 to include standard in the readme 👍

@vinistock
Copy link
Contributor

@searls thank you very much for the blog post and your work on this. Having your support for the vision is really important and I appreciate it 🙏.

koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Ruby LSP Add-on for
Standard Ruby into this RuboCop Add-on at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
koic added a commit to koic/rubocop that referenced this pull request Dec 26, 2024
## Summary

This PR adds support for the Ruby LSP Add-on:
https://shopify.github.io/ruby-lsp/add-ons.html

Ruby LSP is widely used among Ruby developers.

This Add-on resolves the current dependency of Ruby LSP on RuboCop's internal APIs
by allowing RuboCop itself to provide the features related to Ruby LSP,
marking the start of potentially removing these dependencies in the future.

It may also be possible to integrate the Standard Ruby's LSP implementation into
this RuboCop's LSP implementation at a later time.
A future is envisioned in which Ruby LSP, Standard Ruby, and RuboCop
don't each need to implement their own RuboCop runner for
LSP but instead unify that functionality.

An option is available so it can work with both RuboCop's built-in LSP and Ruby LSP.
RuboCop's built-in LSP will continue to serve as a lightweight LSP for users who only need RuboCop.

## References

- standardrb/standard#630
- https://github.com/standardrb/standard/wiki/IDE:-vscode#using-ruby-lsp

## NOTE

This PR is split into multiple commits around three core PRs related to Standard Ruby.
Please don't squash these commits, as the separation is intentional.

Documentation will likely be covered later in conjunction with Ruby LSP.
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.

4 participants