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

Ensure requests are not assuming the presence of file paths for entries #3030

Merged
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
9 changes: 7 additions & 2 deletions lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,8 @@ def handle_method_definition(message, receiver_type, inherited_only: false)

methods.each do |target_method|
uri = target_method.uri
next if sorbet_level_true_or_higher?(@sorbet_level) && not_in_dependencies?(T.must(uri.full_path))
full_path = uri.full_path
next if sorbet_level_true_or_higher?(@sorbet_level) && (!full_path || not_in_dependencies?(full_path))

@response_builder << Interface::LocationLink.new(
target_uri: uri.to_s,
Expand Down Expand Up @@ -403,7 +404,11 @@ def find_in_index(value)
# additional behavior on top of jumping to RBIs. The only sigil where Sorbet cannot handle constants is typed
# ignore
uri = entry.uri
next if @sorbet_level != RubyDocument::SorbetLevel::Ignore && not_in_dependencies?(T.must(uri.full_path))
full_path = uri.full_path

if @sorbet_level != RubyDocument::SorbetLevel::Ignore && (!full_path || not_in_dependencies?(full_path))
next
end

@response_builder << Interface::LocationLink.new(
target_uri: uri.to_s,
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_lsp/requests/rename.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ def collect_file_renames(fully_qualified_name, document_changes)
T.must(@global_state.index[fully_qualified_name]).each do |entry|
# Do not rename files that are not part of the workspace
uri = entry.uri
file_path = T.must(uri.full_path)
next unless file_path.start_with?(@global_state.workspace_path)
file_path = uri.full_path
next unless file_path&.start_with?(@global_state.workspace_path)

case entry
when RubyIndexer::Entry::Class, RubyIndexer::Entry::Module, RubyIndexer::Entry::Constant,
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_lsp/requests/workspace_symbol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ def initialize(global_state, query)
def perform
@index.fuzzy_search(@query).filter_map do |entry|
uri = entry.uri
file_path = T.must(uri.full_path)
file_path = uri.full_path

# We only show symbols declared in the workspace
in_dependencies = !not_in_dependencies?(file_path)
in_dependencies = file_path && !not_in_dependencies?(file_path)
next if in_dependencies

# We should never show private symbols when searching the entire workspace
Expand Down
43 changes: 43 additions & 0 deletions test/requests/definition_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,49 @@ def with_foo(foo)
end
end

def test_definition_does_proper_dependency_checking_for_unsaved_files_for_methods
source = <<~RUBY
# typed: true
class Foo
def bar
end
end

Foo.new.bar
RUBY

with_server(source, URI("untitled:Untitled-1")) do |server, uri|
# On the `foo` receiver, we should not show any results
server.process_message(
id: 1,
method: "textDocument/definition",
params: { textDocument: { uri: uri }, position: { character: 8, line: 6 } },
)
assert_empty(server.pop_response.response)
end
end

def test_definition_does_proper_dependency_checking_for_unsaved_files_for_constants
source = <<~RUBY
class Foo
def bar
end
end

Foo
RUBY

with_server(source, URI("untitled:Untitled-1")) do |server, uri|
# On the `foo` receiver, we should not show any results
server.process_message(
id: 1,
method: "textDocument/definition",
params: { textDocument: { uri: uri }, position: { character: 8, line: 0 } },
)
assert_empty(server.pop_response.response)
end
end

private

def create_definition_addon
Expand Down
48 changes: 48 additions & 0 deletions test/requests/rename_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,54 @@ class Conflicting
end
end

def test_renaming_an_unsaved_symbol
fixture_path = "test/fixtures/rename_me.rb"
source = File.read(fixture_path)
global_state = RubyLsp::GlobalState.new
global_state.apply_options({
capabilities: {
workspace: {
workspaceEdit: {
resourceOperations: ["rename"],
},
},
},
})

store = RubyLsp::Store.new(global_state)

path = File.expand_path(fixture_path)
global_state.index.index_single(URI::Generic.from_path(path: path), source)

untitled_uri = URI("untitled:Untitled-1")
untitled_source = <<~RUBY
class RenameMe
end
RUBY
global_state.index.index_single(untitled_uri, untitled_source)
store.set(uri: untitled_uri, source: untitled_source, version: 1, language_id: RubyLsp::Document::LanguageId::Ruby)

document = RubyLsp::RubyDocument.new(
source: source,
version: 1,
uri: URI::Generic.from_path(path: path),
global_state: global_state,
)

response = T.must(
RubyLsp::Requests::Rename.new(
global_state,
store,
document,
{ position: { line: 3, character: 7 }, newName: "NewMe" },
).perform,
)

untitled_change = response.document_changes[1]
assert_equal("untitled:Untitled-1", untitled_change.text_document.uri)
assert_equal("NewMe", untitled_change.edits[0].new_text)
end

private

def expect_renames(fixture_path, new_fixture_path, expected, position, new_name)
Expand Down
10 changes: 10 additions & 0 deletions test/requests/workspace_symbol_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,14 @@ def bar; end
assert_equal("baz", T.must(result).name)
assert_equal(RubyLsp::Constant::SymbolKind::PROPERTY, T.must(result).kind)
end

def test_returns_symbols_from_unsaved_files
@index.index_single(URI("untitled:Untitled-1"), <<~RUBY)
class Foo; end
RUBY

result = RubyLsp::Requests::WorkspaceSymbol.new(@global_state, "Foo").perform.first
assert_equal("Foo", T.must(result).name)
assert_equal(RubyLsp::Constant::SymbolKind::CLASS, T.must(result).kind)
end
end
Loading