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 fe13536
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 2 deletions.
10 changes: 9 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,14 @@ 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
raise LocationNotFoundError, "Requested position: #{position}\nSource:\n\n#{@source.pack("U*")}"
end
end

@pos += 1
@current_line += 1
end
Expand Down
12 changes: 11 additions & 1 deletion lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,22 @@ 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
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 fe13536

Please sign in to comment.