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

Author: andriytyurnikov

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)

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

lib/ruby_lsp/addon.rb

@@ -56,8 +56,10 @@ 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("**/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
-          project_addons = Dir.glob("#{global_state.workspace_path}/**/ruby_lsp/**/addon.rb")
           project_addons.reject! { |path| path.start_with?(bundle_path) || gem_installation_path?(path) }
 
           addon_files.concat(project_addons)

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|

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")

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

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

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

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)