Skip to content

Commit

Permalink
Use Entry#uri where possible instead of file path (#2927)
Browse files Browse the repository at this point in the history
### Motivation

Another step towards #1908

This PR starts using the entry's URI directly instead of rebuilding them from the file paths. This allows us to do less work, but is also an important step so that features will actually work once we index unsaved files.

### Implementation

Started using the entry's URI for building responses, like definition locations.
  • Loading branch information
vinistock authored Nov 28, 2024
1 parent 6172ba4 commit ce50f49
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 21 deletions.
16 changes: 8 additions & 8 deletions lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def handle_global_variable_definition(name)
location = entry.location

@response_builder << Interface::Location.new(
uri: URI::Generic.from_path(path: entry.file_path).to_s,
uri: entry.uri.to_s,
range: Interface::Range.new(
start: Interface::Position.new(line: location.start_line - 1, character: location.start_column),
end: Interface::Position.new(line: location.end_line - 1, character: location.end_column),
Expand All @@ -248,7 +248,7 @@ def handle_instance_variable_definition(name)
location = entry.location

@response_builder << Interface::Location.new(
uri: URI::Generic.from_path(path: entry.file_path).to_s,
uri: entry.uri.to_s,
range: Interface::Range.new(
start: Interface::Position.new(line: location.start_line - 1, character: location.start_column),
end: Interface::Position.new(line: location.end_line - 1, character: location.end_column),
Expand All @@ -275,11 +275,11 @@ def handle_method_definition(message, receiver_type, inherited_only: false)
return unless methods

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

@response_builder << Interface::LocationLink.new(
target_uri: URI::Generic.from_path(path: file_path).to_s,
target_uri: uri.to_s,
target_range: range_from_location(target_method.location),
target_selection_range: range_from_location(target_method.name_location),
)
Expand Down Expand Up @@ -348,11 +348,11 @@ def find_in_index(value)
# If the project has Sorbet, then we only want to handle go to definition for constants defined in gems, as an
# additional behavior on top of jumping to RBIs. The only sigil where Sorbet cannot handle constants is typed
# ignore
file_path = entry.file_path
next if @sorbet_level != RubyDocument::SorbetLevel::Ignore && not_in_dependencies?(file_path)
uri = entry.uri
next if @sorbet_level != RubyDocument::SorbetLevel::Ignore && not_in_dependencies?(T.must(uri.full_path))

@response_builder << Interface::LocationLink.new(
target_uri: URI::Generic.from_path(path: file_path).to_s,
target_uri: uri.to_s,
target_range: range_from_location(entry.location),
target_selection_range: range_from_location(entry.name_location),
)
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/requests/prepare_type_hierarchy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def perform
Interface::TypeHierarchyItem.new(
name: first_entry.name,
kind: kind_for_entry(first_entry),
uri: URI::Generic.from_path(path: first_entry.file_path).to_s,
uri: first_entry.uri.to_s,
range: range,
selection_range: range,
),
Expand Down
9 changes: 5 additions & 4 deletions lib/ruby_lsp/requests/rename.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ 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
next unless entry.file_path.start_with?(@global_state.workspace_path)
uri = entry.uri
file_path = T.must(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 All @@ -126,13 +128,12 @@ def collect_file_renames(fully_qualified_name, document_changes)
if "#{file_name}.rb" == entry.file_name
new_file_name = file_from_constant_name(T.must(@new_name.split("::").last))

old_uri = URI::Generic.from_path(path: entry.file_path).to_s
new_uri = URI::Generic.from_path(path: File.join(
File.dirname(entry.file_path),
File.dirname(file_path),
"#{new_file_name}.rb",
)).to_s

document_changes << Interface::RenameFile.new(kind: "rename", old_uri: old_uri, new_uri: new_uri)
document_changes << Interface::RenameFile.new(kind: "rename", old_uri: uri.to_s, new_uri: new_uri)
end
end
end
Expand Down
6 changes: 1 addition & 5 deletions lib/ruby_lsp/requests/support/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,7 @@ def categorized_markdown_from_index_entries(title, entries, max_entries = nil)
# based, which is why instead of the usual subtraction of 1 to line numbers, we are actually adding 1 to
# columns. The format for VS Code file URIs is
# `file:///path/to/file.rb#Lstart_line,start_column-end_line,end_column`
uri = URI::Generic.from_path(
path: entry.file_path,
fragment: "L#{loc.start_line},#{loc.start_column + 1}-#{loc.end_line},#{loc.end_column + 1}",
)

uri = "#{entry.uri}#L#{loc.start_line},#{loc.start_column + 1}-#{loc.end_line},#{loc.end_column + 1}"
definitions << "[#{entry.file_name}](#{uri})"
content << "\n\n#{entry.comments}" unless entry.comments.empty?
end
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/requests/type_hierarchy_supertypes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def hierarchy_item(entry)
Interface::TypeHierarchyItem.new(
name: entry.name,
kind: kind_for_entry(entry),
uri: URI::Generic.from_path(path: entry.file_path).to_s,
uri: entry.uri.to_s,
range: range_from_location(entry.location),
selection_range: range_from_location(entry.name_location),
detail: entry.file_name,
Expand Down
5 changes: 3 additions & 2 deletions lib/ruby_lsp/requests/workspace_symbol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ def initialize(global_state, query)
sig { override.returns(T::Array[Interface::WorkspaceSymbol]) }
def perform
@index.fuzzy_search(@query).filter_map do |entry|
file_path = entry.file_path
uri = entry.uri
file_path = T.must(uri.full_path)

# We only show symbols declared in the workspace
in_dependencies = !not_in_dependencies?(file_path)
Expand All @@ -43,7 +44,7 @@ def perform
container_name: container.join("::"),
kind: kind,
location: Interface::Location.new(
uri: URI::Generic.from_path(path: file_path).to_s,
uri: uri.to_s,
range: Interface::Range.new(
start: Interface::Position.new(line: loc.start_line - 1, character: loc.start_column),
end: Interface::Position.new(line: loc.end_line - 1, character: loc.end_column),
Expand Down

0 comments on commit ce50f49

Please sign in to comment.