From 6c583d98cf176c96674926e5db7b71bcd02411e3 Mon Sep 17 00:00:00 2001 From: Andriy Tyurnikov Date: Fri, 20 Mar 2026 02:29:48 +0200 Subject: [PATCH 1/2] Fix bracket/brace filenames breaking LSP features Files with `[`, `]`, `{`, or `}` in their names (e.g., `[id].rb`, `{slug}.rb`) broke multiple ruby-lsp features because brackets were not percent-encoded in URIs and are glob metacharacters in Dir.glob. - Remove `[]` from URI safe character set so they are percent-encoded, matching VS Code's vscode-uri encoding behavior - Use Dir.glob `base:` parameter in references, rename, addon discovery, test_style, and go_to_relevant_file to treat paths as literal - Escape glob metacharacters in user input for require_relative completion - Fix expectations test runner to use File.file? instead of Dir.glob for expectation lookup and sanitize test method names - Add URI tests for brackets, braces, parentheses, and whitespace --- lib/ruby_indexer/lib/ruby_indexer/uri.rb | 2 +- lib/ruby_indexer/test/uri_test.rb | 59 +++++++++++++++++++ lib/ruby_lsp/addon.rb | 4 +- lib/ruby_lsp/listeners/completion.rb | 5 +- lib/ruby_lsp/listeners/test_style.rb | 5 +- lib/ruby_lsp/requests/go_to_relevant_file.rb | 23 +++++--- lib/ruby_lsp/requests/references.rb | 3 +- lib/ruby_lsp/requests/rename.rb | 3 +- .../support/expectations_test_runner.rb | 17 ++++-- 9 files changed, 100 insertions(+), 21 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/uri.rb b/lib/ruby_indexer/lib/ruby_indexer/uri.rb index 6eeff8b478..a11ffa0c60 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/uri.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/uri.rb @@ -18,7 +18,7 @@ def from_path(path:, fragment: nil, scheme: "file", load_path_entry: nil) # This unsafe regex is the same one used in the URI::RFC2396_REGEXP class with the exception of the fact that we # do not include colon as a safe character. VS Code URIs always escape colons and we need to ensure we do the # same to avoid inconsistencies in our URIs, which are used to identify resources - unsafe_regex = %r{[^\-_.!~*'()a-zA-Z\d;/?@&=+$,\[\]]} + unsafe_regex = %r{[^\-_.!~*'()a-zA-Z\d;/?@&=+$,]} # On Windows, if the path begins with the disk name, we need to add a leading slash to make it a valid URI escaped_path = if /^[A-Z]:/i.match?(path) diff --git a/lib/ruby_indexer/test/uri_test.rb b/lib/ruby_indexer/test/uri_test.rb index 8c064a1a77..1672516c92 100644 --- a/lib/ruby_indexer/test/uri_test.rb +++ b/lib/ruby_indexer/test/uri_test.rb @@ -81,5 +81,64 @@ def test_from_path_with_unicode_characters assert_equal(path, uri.to_standardized_path) assert_equal("file:///path/with/unicode/%E6%96%87%E4%BB%B6.rb", uri.to_s) end + + def test_from_path_with_brackets + uri = URI::Generic.from_path(path: "/some/path/[id].rb") + assert_match("%5B", uri.path) + assert_match("%5D", uri.path) + assert_equal("file:///some/path/%5Bid%5D.rb", uri.to_s) + end + + def test_from_path_with_braces + uri = URI::Generic.from_path(path: "/some/path/{slug}.rb") + assert_match("%7B", uri.path) + assert_match("%7D", uri.path) + assert_equal("file:///some/path/%7Bslug%7D.rb", uri.to_s) + end + + def test_round_trip_with_brackets + path = "/some/path/[id].rb" + uri = URI::Generic.from_path(path: path) + assert_equal(path, uri.to_standardized_path) + end + + def test_round_trip_with_braces + path = "/some/path/{slug}.rb" + uri = URI::Generic.from_path(path: path) + assert_equal(path, uri.to_standardized_path) + end + + def test_from_path_with_parentheses + uri = URI::Generic.from_path(path: "/some/path/(id).rb") + assert_equal("/some/path/(id).rb", uri.path) + assert_equal("file:///some/path/(id).rb", uri.to_s) + end + + def test_round_trip_with_parentheses + path = "/some/path/(id).rb" + uri = URI::Generic.from_path(path: path) + assert_equal(path, uri.to_standardized_path) + end + + def test_round_trip_with_spaces_inside_brackets + path = "/some/path/[id page].rb" + uri = URI::Generic.from_path(path: path) + assert_equal(path, uri.to_standardized_path) + assert_equal("file:///some/path/%5Bid%20page%5D.rb", uri.to_s) + end + + def test_round_trip_with_spaces_inside_braces + path = "/some/path/{slug name}.rb" + uri = URI::Generic.from_path(path: path) + assert_equal(path, uri.to_standardized_path) + assert_equal("file:///some/path/%7Bslug%20name%7D.rb", uri.to_s) + end + + def test_round_trip_with_spaces_inside_parentheses + path = "/some/path/file (copy).rb" + uri = URI::Generic.from_path(path: path) + assert_equal(path, uri.to_standardized_path) + assert_equal("file:///some/path/file%20(copy).rb", uri.to_s) + end end end diff --git a/lib/ruby_lsp/addon.rb b/lib/ruby_lsp/addon.rb index c346d6ea61..90247d97b7 100644 --- a/lib/ruby_lsp/addon.rb +++ b/lib/ruby_lsp/addon.rb @@ -56,7 +56,9 @@ def load_addons(global_state, outgoing_queue, include_project_addons: true) addon_files = Gem.find_files("ruby_lsp/**/addon.rb") if include_project_addons - project_addons = Dir.glob("#{global_state.workspace_path}/**/ruby_lsp/**/addon.rb") + project_addons = Dir.glob("**/ruby_lsp/**/addon.rb", base: global_state.workspace_path).map! do |relative_path| + File.join(global_state.workspace_path, relative_path) + end bundle_path = Bundler.bundle_path.to_s gems_dir = Bundler.bundle_path.join("gems") diff --git a/lib/ruby_lsp/listeners/completion.rb b/lib/ruby_lsp/listeners/completion.rb index 8f3a13ae10..17d6439927 100644 --- a/lib/ruby_lsp/listeners/completion.rb +++ b/lib/ruby_lsp/listeners/completion.rb @@ -457,10 +457,11 @@ def complete_require_relative(node) # if the path is not a directory, glob all possible next characters # for example ../somethi| (where | is the cursor position) # should find files for ../somethi*/ + escaped_content = content.gsub(/[\[\]{}*?\\]/) { |c| "\\#{c}" } path_query = if content.end_with?("/") || content.empty? - "#{content}**/*.rb" + "#{escaped_content}**/*.rb" else - "{#{content}*/**/*.rb,**/#{content}*.rb}" + "{#{escaped_content}*/**/*.rb,**/#{escaped_content}*.rb}" end Dir.glob(path_query, File::FNM_PATHNAME | File::FNM_EXTGLOB, base: origin_dir).sort!.each do |path| diff --git a/lib/ruby_lsp/listeners/test_style.rb b/lib/ruby_lsp/listeners/test_style.rb index b18ed7d320..dbac9a17fb 100644 --- a/lib/ruby_lsp/listeners/test_style.rb +++ b/lib/ruby_lsp/listeners/test_style.rb @@ -35,9 +35,10 @@ def resolve_test_commands(items) if children.empty? full_files.concat( Dir.glob( - "#{path}/**/{*_test,test_*,*_spec}.rb", + "**/{*_test,test_*,*_spec}.rb", File::Constants::FNM_EXTGLOB | File::Constants::FNM_PATHNAME, - ).map! { |p| Shellwords.escape(p) }, + base: path, + ).map! { |p| Shellwords.escape(File.join(path, p)) }, ) end elsif tags.include?("test_file") diff --git a/lib/ruby_lsp/requests/go_to_relevant_file.rb b/lib/ruby_lsp/requests/go_to_relevant_file.rb index 954d613749..e48d54cdb4 100644 --- a/lib/ruby_lsp/requests/go_to_relevant_file.rb +++ b/lib/ruby_lsp/requests/go_to_relevant_file.rb @@ -36,9 +36,10 @@ def perform #: -> Array[String] def find_relevant_paths patterns = relevant_filename_patterns + root = search_root candidate_paths = patterns.flat_map do |pattern| - Dir.glob(File.join(search_root, "**", pattern)) + Dir.glob(File.join("**", pattern), base: root).map! { |p| File.join(root, p) } end return [] if candidate_paths.empty? @@ -89,26 +90,34 @@ def relevant_filename_patterns # Test file -> find implementation base = input_basename.gsub(TEST_PATTERN, "") parent_dir = File.basename(File.dirname(@path)) + escaped_base = escape_glob_metacharacters(base) + escaped_parent_dir = escape_glob_metacharacters(parent_dir) # If test file is in a directory matching the implementation name # (e.g., go_to_relevant_file/test_go_to_relevant_file_a.rb) # return patterns for both the base file name and the parent directory name if base.include?(parent_dir) && base != parent_dir - ["#{base}#{extension}", "#{parent_dir}#{extension}"] + ["#{escaped_base}#{extension}", "#{escaped_parent_dir}#{extension}"] else - ["#{base}#{extension}"] + ["#{escaped_base}#{extension}"] end else # Implementation file -> find tests (including in matching directory) + escaped_basename = escape_glob_metacharacters(input_basename) [ - "{#{TEST_PREFIX_GLOB}}#{input_basename}#{extension}", - "#{input_basename}{#{TEST_SUFFIX_GLOB}}#{extension}", - "#{input_basename}/{#{TEST_PREFIX_GLOB}}*#{extension}", - "#{input_basename}/*{#{TEST_SUFFIX_GLOB}}#{extension}", + "{#{TEST_PREFIX_GLOB}}#{escaped_basename}#{extension}", + "#{escaped_basename}{#{TEST_SUFFIX_GLOB}}#{extension}", + "#{escaped_basename}/{#{TEST_PREFIX_GLOB}}*#{extension}", + "#{escaped_basename}/*{#{TEST_SUFFIX_GLOB}}#{extension}", ] end end + #: (String str) -> String + def escape_glob_metacharacters(str) + str.gsub(/[\[\]{}*?\\]/) { |c| "\\#{c}" } + end + # Using the Jaccard algorithm to determine the similarity between the # input path and the candidate relevant file paths. # Ref: https://en.wikipedia.org/wiki/Jaccard_index diff --git a/lib/ruby_lsp/requests/references.rb b/lib/ruby_lsp/requests/references.rb index f400224a47..22cd8ca67d 100644 --- a/lib/ruby_lsp/requests/references.rb +++ b/lib/ruby_lsp/requests/references.rb @@ -60,7 +60,8 @@ def perform reference_target = create_reference_target(target, node_context) return @locations unless reference_target - Dir.glob(File.join(@global_state.workspace_path, "**/*.rb")).each do |path| + Dir.glob("**/*.rb", base: @global_state.workspace_path).each do |relative_path| + path = File.join(@global_state.workspace_path, relative_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 diff --git a/lib/ruby_lsp/requests/rename.rb b/lib/ruby_lsp/requests/rename.rb index a8e8276410..c24fc5e348 100644 --- a/lib/ruby_lsp/requests/rename.rb +++ b/lib/ruby_lsp/requests/rename.rb @@ -129,7 +129,8 @@ def collect_file_renames(fully_qualified_name, document_changes) def collect_text_edits(target, name) changes = {} - Dir.glob(File.join(@global_state.workspace_path, "**/*.rb")).each do |path| + Dir.glob("**/*.rb", base: @global_state.workspace_path).each do |relative_path| + path = File.join(@global_state.workspace_path, relative_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 diff --git a/test/requests/support/expectations_test_runner.rb b/test/requests/support/expectations_test_runner.rb index 1c64c72758..a7242a12d4 100644 --- a/test/requests/support/expectations_test_runner.rb +++ b/test/requests/support/expectations_test_runner.rb @@ -50,16 +50,21 @@ def default_args raise "Expectations directory #{expectations_dir} does not exist" end - expectation_glob = Dir.glob(File.join(expectations_dir, "#{test_name}.exp.{rb,json}")) - if expectation_glob.size == 1 - expectation_path = expectation_glob.first - elsif expectation_glob.size > 1 + json_path = File.join(expectations_dir, "#{test_name}.exp.json") + rb_path = File.join(expectations_dir, "#{test_name}.exp.rb") + if File.file?(json_path) && File.file?(rb_path) raise "multiple expectations for #{test_name}" + elsif File.file?(json_path) + expectation_path = json_path + elsif File.file?(rb_path) + expectation_path = rb_path end + sanitized_name = test_name.gsub(/[^\w]/, "_") + if expectation_path && File.file?(expectation_path) class_eval(<<~RB, __FILE__, __LINE__ + 1) - def test_#{expectation_suffix}__#{test_name} + def test_#{expectation_suffix}__#{sanitized_name} @_path = "#{path}" @_expectation_path = "#{expectation_path}" source = File.read(@_path) @@ -70,7 +75,7 @@ def test_#{expectation_suffix}__#{test_name} RB else class_eval(<<~RB, __FILE__, __LINE__ + 1) - def test_#{expectation_suffix}__#{test_name}__does_not_raise + def test_#{expectation_suffix}__#{sanitized_name}__does_not_raise @_path = "#{path}" source = File.read(@_path) run_expectations(source) From 8626f6cf713b4babe40776da8ab44094ef24beea Mon Sep 17 00:00:00 2001 From: Andriy Tyurnikov Date: Fri, 20 Mar 2026 02:38:52 +0200 Subject: [PATCH 2/2] Retrigger CI after CLA signing