Skip to content

Commit

Permalink
Check if RuboCop is available before searching for add-on (#3071)
Browse files Browse the repository at this point in the history
### Motivation

This is a fix for the bug I just introduced in #3067 🤦‍♂️. We cannot assume that the `RuboCop` constant will be defined because projects may use a different formatter. We need to check first.

### Implementation

Started checking if the constant we need is defined as part of the check.

### Automated Tests

Part of why this wasn't caught is because our integration tests were ignoring the response return for the initialize request.

That makes the tests extremely weak because the LSP will actually rescue anything that happens during initialize and then simply return an error back to the client, which means that tests passed despite initialization failing.

I started properly checking for what response was returned, so that we can fail tests if initialization was not successful. Without the fix, tests now properly fail.
  • Loading branch information
vinistock authored Jan 16, 2025
1 parent 542e896 commit bbd0c84
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/ruby_lsp/global_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ def apply_options(options)
@workspace_uri = URI(workspace_uri) if workspace_uri

specified_formatter = options.dig(:initializationOptions, :formatter)
rubocop_has_addon = Gem::Requirement.new(">= 1.70.0").satisfied_by?(Gem::Version.new(::RuboCop::Version::STRING))
rubocop_has_addon = defined?(::RuboCop::Version::STRING) &&
Gem::Requirement.new(">= 1.70.0").satisfied_by?(Gem::Version.new(::RuboCop::Version::STRING))

if specified_formatter
@formatter = specified_formatter
Expand Down
13 changes: 13 additions & 0 deletions test/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ def launch(workspace_path, exec = "ruby-lsp-launcher", extra_env = {})
workspaceFolders: [{ uri: URI::Generic.from_path(path: workspace_path).to_s }],
},
})

# First message is the log of initializing Ruby LSP
read_message(stdout)
# Verify that initialization didn't fail
initialize_response = read_message(stdout)
refute(initialize_response[:error], initialize_response.dig(:error, :message))

send_message(stdin, { id: 2, method: "shutdown" })
send_message(stdin, { method: "exit" })

Expand Down Expand Up @@ -253,6 +260,12 @@ def send_message(stdin, message)
stdin.flush
end

def read_message(stdout)
headers = stdout.gets("\r\n\r\n")
length = headers[/Content-Length: (\d+)/i, 1].to_i
JSON.parse(stdout.read(length), symbolize_names: true)
end

def in_temp_dir
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
Expand Down

0 comments on commit bbd0c84

Please sign in to comment.