diff --git a/lib/ruby_lsp/internal.rb b/lib/ruby_lsp/internal.rb index cbff9bb3c..1a2cbdf95 100644 --- a/lib/ruby_lsp/internal.rb +++ b/lib/ruby_lsp/internal.rb @@ -33,6 +33,7 @@ # Rubydex LSP additions require "ruby_lsp/rubydex/definition" +require "ruby_lsp/rubydex/reference" require "ruby-lsp" require "ruby_lsp/base_server" diff --git a/lib/ruby_lsp/requests/rename.rb b/lib/ruby_lsp/requests/rename.rb index a8e827641..db7b2d78d 100644 --- a/lib/ruby_lsp/requests/rename.rb +++ b/lib/ruby_lsp/requests/rename.rb @@ -22,6 +22,7 @@ def provider def initialize(global_state, store, document, params) super() @global_state = global_state + @graph = global_state.graph #: Rubydex::Graph @store = store @document = document @position = params[:position] #: Hash[Symbol, Integer] @@ -56,17 +57,14 @@ def perform name = RubyIndexer::Index.constant_name(target) return unless name - entries = @global_state.index.resolve(name, node_context.nesting) - return unless entries + declaration = @graph.resolve_constant(name, node_context.nesting) + return unless declaration - if (conflict_entries = @global_state.index.resolve(@new_name, node_context.nesting)) - raise InvalidNameError, "The new name is already in use by #{conflict_entries.first&.name}" + if (conflict = @graph.resolve_constant(@new_name, node_context.nesting)) + raise InvalidNameError, "The new name is already in use by #{conflict.name}" end - fully_qualified_name = entries.first #: as !nil - .name - reference_target = RubyIndexer::ReferenceFinder::ConstTarget.new(fully_qualified_name) - changes = collect_text_edits(reference_target, name) + changes = collect_text_edits(declaration, name) # If the client doesn't support resource operations, such as renaming files, then we can only return the basic # text changes @@ -78,99 +76,93 @@ def perform # renamed and then the URI associated to the text edit no longer exists, causing it to be dropped document_changes = changes.map do |uri, edits| Interface::TextDocumentEdit.new( - text_document: Interface::VersionedTextDocumentIdentifier.new(uri: uri, version: nil), + text_document: Interface::OptionalVersionedTextDocumentIdentifier.new(uri: uri, version: nil), edits: edits, ) end - collect_file_renames(fully_qualified_name, document_changes) + collect_file_renames(declaration, document_changes) Interface::WorkspaceEdit.new(document_changes: document_changes) end private - #: (String fully_qualified_name, Array[(Interface::RenameFile | Interface::TextDocumentEdit)] document_changes) -> void - def collect_file_renames(fully_qualified_name, document_changes) + #: (Rubydex::Declaration, Array[(Interface::RenameFile | Interface::TextDocumentEdit)]) -> void + def collect_file_renames(declaration, document_changes) # Check if the declarations of the symbol being renamed match the file name. In case they do, we automatically # rename the files for the user. # # We also look for an associated test file and rename it too - short_name = fully_qualified_name.split("::").last #: as !nil - @global_state.index[fully_qualified_name]&.each do |entry| + unless [ + Rubydex::Class, + Rubydex::Module, + Rubydex::Constant, + Rubydex::ConstantAlias, + ].any? { |type| declaration.is_a?(type) } + return + end + + short_name = declaration.unqualified_name + + declaration.definitions.each do |definition| # Do not rename files that are not part of the workspace - uri = entry.uri + uri = URI(definition.location.uri) 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, - RubyIndexer::Entry::ConstantAlias, RubyIndexer::Entry::UnresolvedConstantAlias - - file_name = file_from_constant_name(short_name) + file_name = file_from_constant_name(short_name) + next unless "#{file_name}.rb" == File.basename(file_path) - if "#{file_name}.rb" == entry.file_name - new_file_name = file_from_constant_name( - @new_name.split("::").last, #: as !nil - ) + new_file_name = file_from_constant_name( + @new_name.split("::").last, #: as !nil + ) - new_uri = URI::Generic.from_path(path: File.join( - File.dirname(file_path), - "#{new_file_name}.rb", - )).to_s + new_uri = URI::Generic.from_path(path: File.join( + File.dirname(file_path), + "#{new_file_name}.rb", + )).to_s - document_changes << Interface::RenameFile.new(kind: "rename", old_uri: uri.to_s, new_uri: new_uri) - end - end + document_changes << Interface::RenameFile.new(kind: "rename", old_uri: uri.to_s, new_uri: new_uri) end end - #: (RubyIndexer::ReferenceFinder::Target target, String name) -> Hash[String, Array[Interface::TextEdit]] - def collect_text_edits(target, name) - changes = {} - - Dir.glob(File.join(@global_state.workspace_path, "**/*.rb")).each do |path| - uri = URI::Generic.from_path(path: path) - # If the document is being managed by the client, then we should use whatever is present in the store instead - # of reading from disk - next if @store.key?(uri) - - parse_result = Prism.parse_file(path) - edits = collect_changes(target, parse_result.value, name, uri) - changes[uri.to_s] = edits unless edits.empty? - rescue Errno::EISDIR, Errno::ENOENT - # If `path` is a directory, just ignore it and continue. If the file doesn't exist, then we also ignore it. - end - - @store.each do |uri, document| - next unless document.is_a?(RubyDocument) || document.is_a?(ERBDocument) + #: (Rubydex::Declaration declaration, String name) -> Hash[String, Array[Interface::TextEdit]] + def collect_text_edits(declaration, name) + changes = {} #: Hash[String, Array[Interface::TextEdit]] + short_name = name.split("::").last #: as !nil + new_short_name = @new_name.split("::").last #: as !nil + + # Collect edits for definition sites (where the constant is declared) + declaration.definitions.each do |definition| + name_loc = definition.name_location + next unless name_loc + + uri_string = name_loc.uri + edits = (changes[uri_string] ||= []) + + # The name_location spans the constant name as written in the definition. + # We only replace the unqualified name portion (the last segment). + range = Interface::Range.new( + start: Interface::Position.new( + line: name_loc.end_line, + character: name_loc.end_column - short_name.length, + ), + end: Interface::Position.new(line: name_loc.end_line, character: name_loc.end_column), + ) - edits = collect_changes(target, document.ast, name, document.uri) - changes[uri] = edits unless edits.empty? + edits << Interface::TextEdit.new(range: range, new_text: new_short_name) end - changes - end - - #: (RubyIndexer::ReferenceFinder::Target target, Prism::Node ast, String name, URI::Generic uri) -> Array[Interface::TextEdit] - def collect_changes(target, ast, name, uri) - dispatcher = Prism::Dispatcher.new - finder = RubyIndexer::ReferenceFinder.new(target, @global_state.index, dispatcher, uri) - dispatcher.visit(ast) - - finder.references.map do |reference| - adjust_reference_for_edit(name, reference) + # Collect edits for reference sites (where the constant is used) + declaration.references.each do |reference| + ref = reference #: as Rubydex::ConstantReference + uri_string = ref.location.uri + edits = (changes[uri_string] ||= []) + edits << Interface::TextEdit.new(range: ref.to_lsp_range, new_text: new_short_name) end - end - - #: (String name, RubyIndexer::ReferenceFinder::Reference reference) -> Interface::TextEdit - def adjust_reference_for_edit(name, reference) - # The reference may include a namespace in front. We need to check if the rename new name includes namespaces - # and then adjust both the text and the location to produce the correct edit - location = reference.location - new_text = reference.name.sub(name, @new_name) - Interface::TextEdit.new(range: range_from_location(location), new_text: new_text) + changes end #: (String constant_name) -> String diff --git a/lib/ruby_lsp/rubydex/reference.rb b/lib/ruby_lsp/rubydex/reference.rb new file mode 100644 index 000000000..ae47aa0f7 --- /dev/null +++ b/lib/ruby_lsp/rubydex/reference.rb @@ -0,0 +1,16 @@ +# typed: strict +# frozen_string_literal: true + +module Rubydex + class ConstantReference + #: () -> RubyLsp::Interface::Range + def to_lsp_range + loc = location + + RubyLsp::Interface::Range.new( + start: RubyLsp::Interface::Position.new(line: loc.start_line, character: loc.start_column), + end: RubyLsp::Interface::Position.new(line: loc.end_line, character: loc.end_column), + ) + end + end +end diff --git a/test/requests/rename_test.rb b/test/requests/rename_test.rb index d9bf7c01e..e83435044 100644 --- a/test/requests/rename_test.rb +++ b/test/requests/rename_test.rb @@ -4,113 +4,167 @@ require "test_helper" class RenameTest < Minitest::Test - def test_empty_diagnostics_for_ignored_file - expected = <<~RUBY + def setup + @tmp_dir = Dir.mktmpdir + end + + def teardown + FileUtils.remove_entry(@tmp_dir) + end + + def test_renaming_a_constant + source = <<~RUBY + class RenameMe + end + + RenameMe + RUBY + + result, document = perform_rename( + source, + position: { line: 0, character: 7 }, + new_name: "Article", + file_name: "rename_me.rb", + ) + + apply_edits(result, document) + + assert_equal(<<~RUBY, document.source) class Article end Article RUBY - expect_renames( - "test/fixtures/rename_me.rb", - File.join("test", "fixtures", "article.rb"), - expected, - { line: 0, character: 7 }, - "Article", + assert_file_renamed(result, from: "rename_me.rb", to: "article.rb") + end + + def test_renaming_a_complex_compact_style_constant + source = <<~RUBY + module Foo + module Bar; end + end + + module Baz + include Foo + + class Bar::RenameMe + end + end + + Foo::Bar::RenameMe + RUBY + + result, document = perform_rename( + source, + position: { line: 6, character: 13 }, + new_name: "Article", ) + + apply_edits(result, document) + + assert_equal(<<~RUBY, document.source) + module Foo + module Bar; end + end + + module Baz + include Foo + + class Bar::Article + end + end + + Foo::Bar::Article + RUBY end - def test_renaming_conflict - 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"], - }, - }, - }, - }) - path = File.expand_path(fixture_path) - global_state.index.index_single(URI::Generic.from_path(path: path), source) - global_state.index.index_single(URI::Generic.from_path(path: "/fake.rb"), <<~RUBY) - class Conflicting + def test_renaming_a_method_receiver + source = <<~RUBY + class Foo + end + + class Bar + def Foo.qux + end end RUBY - store = RubyLsp::Store.new(global_state) - document = RubyLsp::RubyDocument.new( - source: source, - version: 1, - uri: URI::Generic.from_path(path: path), - global_state: global_state, + result, document = perform_rename( + source, + position: { line: 4, character: 6 }, + new_name: "Zip", ) + apply_edits(result, document) + + assert_equal(<<~RUBY, document.source) + class Zip + end + + class Bar + def Zip.qux + end + end + RUBY + end + + def test_renaming_conflict + source = <<~RUBY + class RenameMe + end + + RenameMe + RUBY + assert_raises(RubyLsp::Requests::Rename::InvalidNameError) do - RubyLsp::Requests::Rename.new( - global_state, - store, - document, - { position: { line: 3, character: 7 }, newName: "Conflicting" }, - ).perform + perform_rename(source, position: { line: 3, character: 0 }, new_name: "Conflicting") do |graph| + graph.index_source( + URI::Generic.from_path(path: File.join(@tmp_dir, "conflicting.rb")).to_s, + "class Conflicting\nend\n", + "ruby", + ) + end 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) + def test_renaming_across_unsaved_files + source = <<~RUBY + class RenameMe + end - path = File.expand_path(fixture_path) - global_state.index.index_single(URI::Generic.from_path(path: path), source) + RenameMe + RUBY 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: :ruby) - - document = RubyLsp::RubyDocument.new( - source: source, - version: 1, - uri: URI::Generic.from_path(path: path), - global_state: global_state, - ) - response = RubyLsp::Requests::Rename.new( - global_state, - store, - document, - { position: { line: 3, character: 7 }, newName: "NewMe" }, - ).perform #: as !nil + result, = perform_rename(source, position: { line: 3, character: 0 }, new_name: "NewMe") do |graph, store| + graph.index_source(untitled_uri.to_s, untitled_source, "ruby") + store.set(uri: untitled_uri, source: untitled_source, version: 1, language_id: :ruby) + end - untitled_change = response.document_changes[1] - assert_equal("untitled:Untitled-1", untitled_change.text_document.uri) + untitled_change = result.document_changes.find do |c| + c.is_a?(RubyLsp::Interface::TextDocumentEdit) && c.text_document.uri == untitled_uri.to_s + end + refute_nil(untitled_change) assert_equal("NewMe", untitled_change.edits[0].new_text) end private - def expect_renames(fixture_path, new_fixture_path, expected, position, new_name) - source = File.read(fixture_path) + #: (String, position: Hash[Symbol, Integer], new_name: String, ?file_name: String) ?{ (Rubydex::Graph, RubyLsp::Store) -> void } -> [RubyLsp::Interface::WorkspaceEdit, RubyLsp::RubyDocument] + def perform_rename(source, position:, new_name:, file_name: "test.rb", &block) + path = File.join(@tmp_dir, file_name) + File.write(path, source) + uri = URI::Generic.from_path(path: path) + global_state = RubyLsp::GlobalState.new global_state.apply_options({ + workspaceFolders: [{ uri: URI::Generic.from_path(path: @tmp_dir).to_s }], capabilities: { workspace: { workspaceEdit: { @@ -119,36 +173,52 @@ def expect_renames(fixture_path, new_fixture_path, expected, position, new_name) }, }, }) - path = File.expand_path(fixture_path) - global_state.index.index_single(URI::Generic.from_path(path: path), source) + graph = global_state.graph store = RubyLsp::Store.new(global_state) + graph.index_source(uri.to_s, source, "ruby") + + block&.call(graph, store) + + graph.resolve + document = RubyLsp::RubyDocument.new( - source: source, + source: source.dup, version: 1, - uri: URI::Generic.from_path(path: path), + uri: uri, global_state: global_state, ) - workspace_edit = RubyLsp::Requests::Rename.new( + + result = RubyLsp::Requests::Rename.new( global_state, store, document, { position: position, newName: new_name }, ).perform #: as !nil - file_renames = workspace_edit.document_changes.filter_map do |text_edit_or_rename| - next text_edit_or_rename unless text_edit_or_rename.is_a?(RubyLsp::Interface::TextDocumentEdit) + [result, document] + end + + #: (RubyLsp::Interface::WorkspaceEdit result, RubyLsp::RubyDocument document) -> void + def apply_edits(result, document) + result.document_changes.each do |change| + next unless change.is_a?(RubyLsp::Interface::TextDocumentEdit) + next unless change.text_document.uri == document.uri.to_s document.push_edits( - text_edit_or_rename.edits.map do |edit| + change.edits.map do |edit| { range: edit.range.to_hash.transform_values(&:to_hash), text: edit.new_text } end, version: 2, ) - nil end + end - assert_equal(expected, document.source) - assert_equal(File.expand_path(new_fixture_path), URI(file_renames.first.new_uri).to_standardized_path) + #: (RubyLsp::Interface::WorkspaceEdit result, from: String, to: String) -> void + def assert_file_renamed(result, from:, to:) + file_rename = result.document_changes.find { |c| c.is_a?(RubyLsp::Interface::RenameFile) } + refute_nil(file_rename, "Expected a file rename operation") + assert(file_rename.old_uri.end_with?(from), "Expected old_uri to end with '#{from}', got '#{file_rename.old_uri}'") + assert(file_rename.new_uri.end_with?(to), "Expected new_uri to end with '#{to}', got '#{file_rename.new_uri}'") end end