Skip to content

Commit

Permalink
Fail requests that are searching for a non existing position
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Nov 29, 2024
1 parent b0eb396 commit 57dc835
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 2 deletions.
11 changes: 10 additions & 1 deletion lib/ruby_lsp/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class LanguageId < T::Enum
extend T::Helpers
extend T::Generic

class LocationNotFoundError < StandardError; end
ParseResultType = type_member

# This maximum number of characters for providing expensive features, like semantic highlighting and diagnostics.
Expand Down Expand Up @@ -144,7 +145,15 @@ def initialize(source, encoding)
def find_char_position(position)
# Find the character index for the beginning of the requested line
until @current_line == position[:line]
@pos += 1 until LINE_BREAK == @source[@pos]
until LINE_BREAK == @source[@pos]
@pos += 1

if @pos >= @source.length
# Pack the code points back into the original string to provide context in the error message
raise LocationNotFoundError, "Requested position: #{position}\nSource:\n\n#{@source.pack("U*")}"
end
end

@pos += 1
@current_line += 1
end
Expand Down
14 changes: 13 additions & 1 deletion lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,24 @@ def process_message(message)
# If a document is deleted before we are able to process all of its enqueued requests, we will try to read it
# from disk and it raise this error. This is expected, so we don't include the `data` attribute to avoid
# reporting these to our telemetry
if e.is_a?(Store::NonExistingDocumentError)
case e
when Store::NonExistingDocumentError
send_message(Error.new(
id: message[:id],
code: Constant::ErrorCodes::INVALID_PARAMS,
message: e.full_message,
))
when Document::LocationNotFoundError
send_message(Error.new(
id: message[:id],
code: Constant::ErrorCodes::REQUEST_FAILED,
message: <<~MESSAGE,
Request #{message[:method]} failed to find the target position.
The file might have been modified while the server was in the middle of searching for the target.
If you experience this regularly, please report any findings and extra information on
https://github.com/Shopify/ruby-lsp/issues/2446
MESSAGE
))
else
send_message(Error.new(
id: message[:id],
Expand Down
23 changes: 23 additions & 0 deletions test/ruby_document_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,29 @@ class Foo
assert_nil(document.cache_get("textDocument/codeLens"))
end

def test_locating_a_non_existing_location_raises
document = RubyLsp::RubyDocument.new(source: <<~RUBY.chomp, version: 1, uri: URI("file:///foo/bar.rb"))
class Foo
end
RUBY

# Exactly at the last character doesn't raise
document.locate_node({ line: 1, character: 2 })

# Anything beyond does
error = assert_raises(RubyLsp::Document::LocationNotFoundError) do
document.locate_node({ line: 3, character: 2 })
end

assert_equal(<<~MESSAGE.chomp, error.message)
Requested position: {:line=>3, :character=>2}
Source:
class Foo
end
MESSAGE
end

private

def assert_error_edit(actual, error_range)
Expand Down
30 changes: 30 additions & 0 deletions test/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,36 @@ def handle_window_show_message_response(title)
end
end

def test_requests_to_a_non_existing_position_return_error
uri = URI("file:///foo.rb")

@server.process_message({
method: "textDocument/didOpen",
params: {
textDocument: {
uri: uri,
text: "class Foo\nend",
version: 1,
languageId: "ruby",
},
},
})

@server.process_message({
id: 1,
method: "textDocument/completion",
params: {
textDocument: {
uri: uri,
},
position: { line: 10, character: 0 },
},
})

error = find_message(RubyLsp::Error)
assert_match("Request textDocument/completion failed to find the target position.", error.message)
end

private

def with_uninstalled_rubocop(&block)
Expand Down

0 comments on commit 57dc835

Please sign in to comment.