Skip to content

Allow disabling the Sorbet detection via config #3468

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michaelherold
Copy link

Motivation

When configuring editors other than VS Code (e.g. Zed or Emacs), sometimes running multiple LSPs isn't an option. This means that Sorbet-enabled projects aren't able to run both Ruby LSP and Sorbet LSP.

In those cases, it's convenient to be able to disable the deference for some features to the type checker LSP. Prior to this change, you could accomplish this by setting the RUBY_LSP_BYPASS_TYPECHECKER environment variable, which works but isn't the easiest thing to do in all cases (e.g. you can't launch Zed from Spotlight unless you globally set this value, which then disables Sorbet deference in all projects).

Instead of hiding this behavior behind an environment variable, this change makes it configurable via an initialization option.

Implementation

This change introduces a new initialization option for disabling the automatic detection of a type checker. This behavior was initially implemented through an environment variable (which this change maintains for backwards-compatibility), but that isn't the easiest thing to configure.

By making it a part of initialization, we can now declaratively set the flag with all other configuration for the server.

I went with the naming of "defer" and "bypass" because the default behavior is to defer some functionality to the type checker and the alternative is to bypass with it. While these are communicative, I'm unsure if they're discoverable.

Automated Tests

I added all cases for this behavior to the GlobalState test suite.

Manual Tests

Use this configuration in a Sorbet-enabled project:

{
  "initializationOptions": {
    "featuresConfiguration": {
      "typeCheckerIntegration": "bypass"
    }
  }
}

and observe that Ruby LSP handles go-to-symbol and workspace symbols.

This change introduces a new initialization option for disabling the
automatic detection of a type checker. This behavior was initially
implemented through an environment variable (which this change maintains
for backwards-compatibility), but that isn't the easiest thing to
configure.

By making it a part of initialization, we can now declaratively set the
flag with all other configuration for the server.
@michaelherold michaelherold requested a review from a team as a code owner May 8, 2025 20:38
Copy link

graphite-app bot commented May 8, 2025

How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@amomchilov amomchilov requested a review from vinistock May 9, 2025 14:48
@amomchilov
Copy link
Contributor

Looks good to me, but I'll run it by @vinistock

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels May 9, 2025
Copy link
Member

@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.

Thanks for explaining the use case. I think it makes sense to move to an initialization option.

However, let's not have two ways to turn it on. Let's get rid of the environment variable.

To make that work, we need to update two places:

  • We need to stop mutating the environment here
  • And should instead pass the expected value based on the setting here

No need to change the user facing VS Code setting name, let's just make sure VS Code is also using the initialization opts.

@michaelherold
Copy link
Author

Roger, I missed that you can enable this through VS Code.

Should we add a deprecation warning to rubyLsp.bypassTypechecker to nudge people to migrate? It is used in several Github repos and, like you said, it would be good to have one way to do the thing.

@vinistock
Copy link
Member

We don't have to change the setting itself. We can keep the rubyLsp.bypassTypechecker setting and just change how it gets passed to the server. That should be fine.

The server will probably need to accept both at least for a few weeks until the upgrades rollout to not break people's setups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants