From 86d94e4609ae02c360de1988bb60b47813d8dd96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=B6kdemir?= Date: Fri, 9 Oct 2026 02:03:43 +0300 Subject: [PATCH 1/2] Fix require_relative paths when renaming classes --- lib/ruby_lsp/requests/rename.rb | 90 +++++++++++++++++++++++++++++---- test/requests/rename_test.rb | 57 +++++++++++++++++++++ 2 files changed, 137 insertions(+), 10 deletions(-) diff --git a/lib/ruby_lsp/requests/rename.rb b/lib/ruby_lsp/requests/rename.rb index a8e8276410..aac469d8b9 100644 --- a/lib/ruby_lsp/requests/rename.rb +++ b/lib/ruby_lsp/requests/rename.rb @@ -66,11 +66,14 @@ def perform fully_qualified_name = entries.first #: as !nil .name reference_target = RubyIndexer::ReferenceFinder::ConstTarget.new(fully_qualified_name) - changes = collect_text_edits(reference_target, name) + supports_rename = @global_state.client_capabilities.supports_rename? + file_renames = [] #: Array[[String, String]] + file_renames = collect_file_renames(fully_qualified_name) if supports_rename + changes = collect_text_edits(reference_target, name, file_renames) # If the client doesn't support resource operations, such as renaming files, then we can only return the basic # text changes - unless @global_state.client_capabilities.supports_rename? + unless supports_rename return Interface::WorkspaceEdit.new(changes: changes) end @@ -81,16 +84,25 @@ def perform text_document: Interface::VersionedTextDocumentIdentifier.new(uri: uri, version: nil), edits: edits, ) + end #: Array[(Interface::RenameFile | Interface::TextDocumentEdit)] + + file_renames.each do |old_path, new_path| + document_changes << Interface::RenameFile.new( + kind: "rename", + old_uri: URI::Generic.from_path(path: old_path).to_s, + new_uri: URI::Generic.from_path(path: new_path).to_s, + ) end - collect_file_renames(fully_qualified_name, 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) + #: (String fully_qualified_name) -> Array[[String, String]] + def collect_file_renames(fully_qualified_name) + file_renames = [] #: Array[[String, String]] + # 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. # @@ -114,20 +126,23 @@ def collect_file_renames(fully_qualified_name, document_changes) @new_name.split("::").last, #: as !nil ) - new_uri = URI::Generic.from_path(path: File.join( + new_file_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) + file_renames << [file_path, new_file_path] end end end + + file_renames end - #: (RubyIndexer::ReferenceFinder::Target target, String name) -> Hash[String, Array[Interface::TextEdit]] - def collect_text_edits(target, name) + #: (RubyIndexer::ReferenceFinder::Target target, String name, Array[[String, String]] file_renames) -> Hash[String, Array[Interface::TextEdit]] + def collect_text_edits(target, name, file_renames) changes = {} + file_renames_by_path = file_renames.to_h Dir.glob(File.join(@global_state.workspace_path, "**/*.rb")).each do |path| uri = URI::Generic.from_path(path: path) @@ -137,6 +152,7 @@ def collect_text_edits(target, name) parse_result = Prism.parse_file(path) edits = collect_changes(target, parse_result.value, name, uri) + edits.concat(collect_require_relative_edits(parse_result.value, path, file_renames_by_path)) 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. @@ -146,12 +162,27 @@ def collect_text_edits(target, name) next unless document.is_a?(RubyDocument) || document.is_a?(ERBDocument) edits = collect_changes(target, document.ast, name, document.uri) + if (path = document.uri.full_path) + edits.concat(collect_require_relative_edits(document.ast, path, file_renames_by_path)) + end changes[uri] = edits unless edits.empty? end changes end + #: (Prism::Node ast, String source_path, Hash[String, String] file_renames) -> Array[Interface::TextEdit] + def collect_require_relative_edits(ast, source_path, file_renames) + workspace_path = @global_state.workspace_path + return [] if file_renames.empty? || !source_path.start_with?("#{workspace_path}#{File::SEPARATOR}") + + visitor = RequireRelativeVisitor.new(source_path, file_renames) + visitor.visit(ast) + visitor.edits.map do |location, new_path| + Interface::TextEdit.new(range: range_from_location(location), new_text: new_path) + end + 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 @@ -179,6 +210,45 @@ def file_from_constant_name(constant_name) .gsub(/([a-z])([A-Z])|([A-Z])([A-Z][a-z])/, '\1\3_\2\4') .downcase end + + class RequireRelativeVisitor < Prism::Visitor + #: Array[[Prism::Location, String]] + attr_reader :edits + + #: (String source_path, Hash[String, String] file_renames) -> void + def initialize(source_path, file_renames) + super() + @source_path = source_path + @file_renames = file_renames + @edits = [] #: Array[[Prism::Location, String]] + end + + #: (Prism::CallNode node) -> void + def visit_call_node(node) + return super unless node.receiver.nil? && node.name == :require_relative + + arguments = node.arguments&.arguments + return super unless arguments && arguments.length == 1 + + argument = arguments.first + return super unless argument.is_a?(Prism::StringNode) + + relative_path = argument.unescaped + required_path = File.expand_path(relative_path, File.dirname(@source_path)) + required_path = "#{required_path}.rb" unless required_path.end_with?(".rb") + new_file_path = @file_renames[required_path] + + if new_file_path + new_relative_path = Pathname.new(new_file_path) + .relative_path_from(Pathname.new(File.dirname(@source_path))) + .to_s + new_relative_path = new_relative_path.delete_suffix(".rb") unless relative_path.end_with?(".rb") + @edits << [argument.content_loc, new_relative_path] + end + + super + end + end end end end diff --git a/test/requests/rename_test.rb b/test/requests/rename_test.rb index d9bf7c01ec..1fde9708d3 100644 --- a/test/requests/rename_test.rb +++ b/test/requests/rename_test.rb @@ -4,6 +4,63 @@ require "test_helper" class RenameTest < Minitest::Test + def test_renaming_a_class_updates_require_relative_paths + Dir.mktmpdir do |workspace| + lib_dir = File.join(workspace, "lib") + FileUtils.mkdir_p(lib_dir) + + store_path = File.join(lib_dir, "store.rb") + store_source = "class Store; end\n" + File.write(store_path, store_source) + + use_path = File.join(lib_dir, "use.rb") + use_source = <<~RUBY + require_relative "store" + Store.new + "require_relative 'store'" + RUBY + File.write(use_path, use_source) + + global_state = RubyLsp::GlobalState.new + global_state.apply_options({ + workspaceFolders: [{ uri: URI::Generic.from_path(path: workspace).to_s }], + capabilities: { + workspace: { + workspaceEdit: { + resourceOperations: ["rename"], + }, + }, + }, + }) + + store_uri = URI::Generic.from_path(path: store_path) + use_uri = URI::Generic.from_path(path: use_path) + global_state.index.index_single(store_uri, store_source) + global_state.index.index_single(use_uri, use_source) + + document = RubyLsp::RubyDocument.new( + source: store_source, + version: 1, + uri: store_uri, + global_state: global_state, + ) + + workspace_edit = RubyLsp::Requests::Rename.new( + global_state, + RubyLsp::Store.new(global_state), + document, + { position: { line: 0, character: 7 }, newName: "Shop" }, + ).perform #: as !nil + + use_edit = workspace_edit.document_changes.find do |change| + change.is_a?(RubyLsp::Interface::TextDocumentEdit) && change.text_document.uri == use_uri.to_s + end + + assert_includes(use_edit.edits.map(&:new_text), "Shop") + assert_equal(1, use_edit.edits.count { |edit| edit.new_text == "shop" }) + end + end + def test_empty_diagnostics_for_ignored_file expected = <<~RUBY class Article From 7b860df4e78f500a228cc832fd0d1427b47532a6 Mon Sep 17 00:00:00 2001 From: Efe Date: Tue, 3 Nov 2026 03:34:32 +0300 Subject: [PATCH 2/2] Preserve Ruby string escaping in require_relative renames --- lib/ruby_lsp/requests/rename.rb | 15 ++++++- test/requests/rename_test.rb | 78 +++++++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/lib/ruby_lsp/requests/rename.rb b/lib/ruby_lsp/requests/rename.rb index aac469d8b9..c466419c6f 100644 --- a/lib/ruby_lsp/requests/rename.rb +++ b/lib/ruby_lsp/requests/rename.rb @@ -233,6 +233,10 @@ def visit_call_node(node) argument = arguments.first return super unless argument.is_a?(Prism::StringNode) + closing = argument.closing_loc + return super unless closing + return super if argument.opening_loc&.slice&.start_with?("<<") + relative_path = argument.unescaped required_path = File.expand_path(relative_path, File.dirname(@source_path)) required_path = "#{required_path}.rb" unless required_path.end_with?(".rb") @@ -243,7 +247,16 @@ def visit_call_node(node) .relative_path_from(Pathname.new(File.dirname(@source_path))) .to_s new_relative_path = new_relative_path.delete_suffix(".rb") unless relative_path.end_with?(".rb") - @edits << [argument.content_loc, new_relative_path] + opening = argument.opening_loc&.slice + case opening + when "'" + escaped_path = new_relative_path.gsub(/[\\']/) { |character| "\\#{character}" } + @edits << [argument.content_loc, escaped_path] + when '"' + @edits << [argument.content_loc, new_relative_path.dump.delete_prefix('"').delete_suffix('"')] + else + @edits << [argument.location, new_relative_path.dump] + end end super diff --git a/test/requests/rename_test.rb b/test/requests/rename_test.rb index 1fde9708d3..4955dd8af4 100644 --- a/test/requests/rename_test.rb +++ b/test/requests/rename_test.rb @@ -5,17 +5,30 @@ class RenameTest < Minitest::Test def test_renaming_a_class_updates_require_relative_paths + assert_rename_updates_require_relative("lib", '"store"', "shop", same_directory: true) + end + + def test_renaming_a_class_updates_nested_require_relative_paths + assert_rename_updates_require_relative("lib", '"lib/store"', "lib/shop") + end + + def test_renaming_a_class_preserves_escaped_require_relative_paths + assert_rename_updates_require_relative("dir's", "'dir\\'s/store'", "dir\\'s/shop") + end + + #: (String directory, String literal, String expected_content, ?same_directory: bool) -> void + def assert_rename_updates_require_relative(directory, literal, expected_content, same_directory: false) Dir.mktmpdir do |workspace| - lib_dir = File.join(workspace, "lib") + lib_dir = File.join(workspace, directory) FileUtils.mkdir_p(lib_dir) store_path = File.join(lib_dir, "store.rb") store_source = "class Store; end\n" File.write(store_path, store_source) - use_path = File.join(lib_dir, "use.rb") + use_path = File.join(same_directory ? lib_dir : workspace, "use.rb") use_source = <<~RUBY - require_relative "store" + require_relative #{literal} Store.new "require_relative 'store'" RUBY @@ -57,7 +70,64 @@ def test_renaming_a_class_updates_require_relative_paths end assert_includes(use_edit.edits.map(&:new_text), "Shop") - assert_equal(1, use_edit.edits.count { |edit| edit.new_text == "shop" }) + assert_equal(1, use_edit.edits.count { |edit| edit.new_text == expected_content }) + use_document = RubyLsp::RubyDocument.new( + source: use_source, version: 1, uri: use_uri, global_state: global_state, + ) + use_document.push_edits( + use_edit.edits.sort_by { |edit| [-edit.range.start.line, -edit.range.start.character] }.map do |edit| + { range: edit.range.to_hash.transform_values(&:to_hash), text: edit.new_text } + end, + version: 2, + ) + parsed = Prism.parse(use_document.source) + assert_empty(parsed.errors) + expected_path = same_directory ? "shop" : "#{directory}/shop" + assert_equal(expected_path, parsed.value.statements.body.first.arguments.arguments.first.unescaped) + end + end + + def test_require_relative_edits_preserve_static_string_values + paths_and_literals = [ + ["dir's/store", %q{'dir\'s/store'}], + ['dir"s/store', %q{"dir\"s/store"}], + ['dir\\name/store', "'dir\\\\name/store'"], + ['dir#{name}/store', %q{'dir#{name}/store'}], + ["dir}/store", "%q{dir\\}/store}"], + ['dir#{name}/store', '%Q{dir\\#{name}/store}'], + ["日本語/store", "'日本語/store'"], + ] + + paths_and_literals.each do |path, literal| + source = "require_relative #{literal}" + old_path = File.expand_path(path, "/workspace") + ".rb" + new_path = old_path.sub(/store\.rb\z/, "shop.rb") + visitor = RubyLsp::Requests::Rename::RequireRelativeVisitor.new( + "/workspace/use.rb", { old_path => new_path } + ) + parsed = Prism.parse(source) + assert_empty(parsed.errors, source) + visitor.visit(parsed.value) + refute_empty(visitor.edits, source) + visitor.edits.sort_by { |location, _| -location.start_offset }.each do |location, replacement| + source.bytesplice(location.start_offset, location.length, replacement) + end + parsed = Prism.parse(source) + assert_empty(parsed.errors, source) + argument = parsed.value.statements.body.first.arguments.arguments.first + assert_equal(path.sub(/store\z/, "shop"), argument.unescaped) + end + end + + def test_require_relative_edits_leave_heredocs_and_dynamic_paths_unchanged + ["require_relative <<~PATH\n store\nPATH\n", 'require_relative "#{directory}/store"'].each do |source| + visitor = RubyLsp::Requests::Rename::RequireRelativeVisitor.new( + "/workspace/use.rb", { "/workspace/store.rb" => "/workspace/shop.rb" } + ) + parsed = Prism.parse(source) + assert_empty(parsed.errors) + visitor.visit(parsed.value) + assert_empty(visitor.edits) end end