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

Revert returning error responses for non existing locations #3026

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions lib/ruby_lsp/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ 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 @@ -200,15 +199,7 @@ 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]
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 until LINE_BREAK == @source[@pos]
@pos += 1
@current_line += 1
end
Expand Down
11 changes: 0 additions & 11 deletions lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,6 @@ def process_message(message)
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: 0 additions & 23 deletions test/ruby_document_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -782,29 +782,6 @@ 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, global_state: @global_state)
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_match(/Requested position: {(:)?line[\s:=>]+3, (:)?character[\s:=>]+2}/, error.message)
assert_match(<<~MESSAGE.chomp, error.message)
Source:

class Foo
end
MESSAGE
end

def test_document_tracks_latest_edit_context
document = RubyLsp::RubyDocument.new(source: +<<~RUBY, version: 1, uri: @uri, global_state: @global_state)
class Foo
Expand Down
30 changes: 0 additions & 30 deletions test/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -721,36 +721,6 @@ 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

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

Expand Down
Loading