Merge pull request #848 from Shopify/vs/use_uri_objects

Use URI objects instead of strings

bin/benchmark

@@ -62,8 +62,8 @@ def avg_bench(method, params)
   [average, standard_deviation]
 end
 
-FILE_URI = "file://#{File.expand_path(__FILE__)}"
-base_params = { textDocument: { uri: FILE_URI } }
+FILE_URI = URI("file://#{File.expand_path(__FILE__)}")
+base_params = { textDocument: { uri: FILE_URI.to_s } }
 range = {
   start: { line: 50, character: 0 },
   end: { line: 75, character: 0 },
@@ -89,7 +89,7 @@ code_action_params = base_params.merge(
             edit: {
               documentChanges: [
                 {
-                  textDocument: { uri: FILE_URI, version: nil },
+                  textDocument: { uri: FILE_URI.to_s, version: nil },
                   edits: [
                     {
                       range: {
@@ -249,7 +249,7 @@ requests = {
   "textDocument/hover" => base_params.merge(position: position),
   "textDocument/codeAction" => code_action_params,
   "textDocument/onTypeFormatting" => base_params.merge(position: { line: 1, character: 31 }, ch: "\n"),
-  "codeAction/resolve" => base_params.merge(data: { range: range, uri: FILE_URI }),
+  "codeAction/resolve" => base_params.merge(data: { range: range, uri: FILE_URI.to_s }),
   "textDocument/completion" => base_params.merge(position: { line: 5, character: 24 }),
   "textDocument/codeLens" => base_params,
   "textDocument/definition" => base_params.merge(position: { line: 4, character: 20 }),

exe/ruby-lsp-check

@@ -30,13 +30,13 @@ message_queue = Thread::Queue.new
 executor = RubyLsp::Executor.new(store, message_queue)
 
 files.each_with_index do |file, index|
-  uri = "file://#{file}"
+  uri = URI("file://#{file}")
   store.set(uri: uri, source: File.read(file), version: 1)
 
   # Executing any of the automatic requests will execute all of them, so here we just pick one
   result = executor.execute({
     method: "textDocument/documentSymbol",
-    params: { textDocument: { uri: uri } },
+    params: { textDocument: { uri: uri.to_s } },
   })
 
   error = result.error

lib/ruby_lsp/document.rb

@@ -18,16 +18,16 @@ class Document
     sig { returns(Integer) }
     attr_reader :version
 
-    sig { returns(String) }
+    sig { returns(URI::Generic) }
     attr_reader :uri
 
-    sig { params(source: String, version: Integer, uri: String, encoding: String).void }
+    sig { params(source: String, version: Integer, uri: URI::Generic, encoding: String).void }
     def initialize(source:, version:, uri:, encoding: Constant::PositionEncodingKind::UTF8)
       @cache = T.let({}, T::Hash[String, T.untyped])
       @encoding = T.let(encoding, String)
       @source = T.let(source, String)
       @version = T.let(version, Integer)
-      @uri = T.let(uri, String)
+      @uri = T.let(uri, URI::Generic)
       @unparsed_edits = T.let([], T::Array[EditShape])
       @syntax_error = T.let(false, T::Boolean)
       @tree = T.let(SyntaxTree.parse(@source), T.nilable(SyntaxTree::Node))

lib/ruby_lsp/executor.rb

@@ -35,7 +35,7 @@ def execute(request)
 
     sig { params(request: T::Hash[Symbol, T.untyped]).returns(T.untyped) }
     def run(request)
-      uri = request.dig(:params, :textDocument, :uri)
+      uri = URI(request.dig(:params, :textDocument, :uri).to_s)
 
       case request[:method]
       when "initialize"
@@ -70,7 +70,7 @@ def run(request)
       when "textDocument/didClose"
         @message_queue << Notification.new(
           message: "textDocument/publishDiagnostics",
-          params: Interface::PublishDiagnosticsParams.new(uri: uri, diagnostics: []),
+          params: Interface::PublishDiagnosticsParams.new(uri: uri.to_s, diagnostics: []),
         )
 
         text_document_did_close(uri)
@@ -174,12 +174,12 @@ def run(request)
       end
     end
 
-    sig { params(uri: String, range: T.nilable(Document::RangeShape)).returns({ ast: String }) }
+    sig { params(uri: URI::Generic, range: T.nilable(Document::RangeShape)).returns({ ast: String }) }
     def show_syntax_tree(uri, range)
       { ast: Requests::ShowSyntaxTree.new(@store.get(uri), range).run }
     end
 
-    sig { params(uri: String, position: Document::PositionShape).returns(T.nilable(Interface::Location)) }
+    sig { params(uri: URI::Generic, position: Document::PositionShape).returns(T.nilable(Interface::Location)) }
     def definition(uri, position)
       document = @store.get(uri)
       return if document.syntax_error?
@@ -192,7 +192,7 @@ def definition(uri, position)
       base_listener.response
     end
 
-    sig { params(uri: String).returns(T::Array[Interface::FoldingRange]) }
+    sig { params(uri: URI::Generic).returns(T::Array[Interface::FoldingRange]) }
     def folding_range(uri)
       @store.cache_fetch(uri, "textDocument/foldingRange") do |document|
         Requests::FoldingRanges.new(document).run
@@ -201,7 +201,7 @@ def folding_range(uri)
 
     sig do
       params(
-        uri: String,
+        uri: URI::Generic,
         position: Document::PositionShape,
       ).returns(T.nilable(Interface::Hover))
     end
@@ -227,27 +227,27 @@ def hover(uri, position)
       hover.response
     end
 
-    sig { params(uri: String, content_changes: T::Array[Document::EditShape], version: Integer).returns(Object) }
+    sig { params(uri: URI::Generic, content_changes: T::Array[Document::EditShape], version: Integer).returns(Object) }
     def text_document_did_change(uri, content_changes, version)
       @store.push_edits(uri: uri, edits: content_changes, version: version)
       VOID
     end
 
-    sig { params(uri: String, text: String, version: Integer).returns(Object) }
+    sig { params(uri: URI::Generic, text: String, version: Integer).returns(Object) }
     def text_document_did_open(uri, text, version)
       @store.set(uri: uri, source: text, version: version)
       VOID
     end
 
-    sig { params(uri: String).returns(Object) }
+    sig { params(uri: URI::Generic).returns(Object) }
     def text_document_did_close(uri)
       @store.delete(uri)
       VOID
     end
 
     sig do
       params(
-        uri: String,
+        uri: URI::Generic,
         positions: T::Array[Document::PositionShape],
       ).returns(T.nilable(T::Array[T.nilable(Requests::Support::SelectionRange)]))
     end
@@ -270,7 +270,7 @@ def selection_range(uri, positions)
       end
     end
 
-    sig { params(uri: String).returns(T.nilable(T::Array[Interface::TextEdit])) }
+    sig { params(uri: URI::Generic).returns(T.nilable(T::Array[Interface::TextEdit])) }
     def formatting(uri)
       # If formatter is set to `auto` but no supported formatting gem is found, don't attempt to format
       return if @store.formatter == "none"
@@ -280,7 +280,7 @@ def formatting(uri)
 
     sig do
       params(
-        uri: String,
+        uri: URI::Generic,
         position: Document::PositionShape,
         character: String,
       ).returns(T::Array[Interface::TextEdit])
@@ -291,7 +291,7 @@ def on_type_formatting(uri, position, character)
 
     sig do
       params(
-        uri: String,
+        uri: URI::Generic,
         position: Document::PositionShape,
       ).returns(T.nilable(T::Array[Interface::DocumentHighlight]))
     end
@@ -306,7 +306,7 @@ def document_highlight(uri, position)
       listener.response
     end
 
-    sig { params(uri: String, range: Document::RangeShape).returns(T.nilable(T::Array[Interface::InlayHint])) }
+    sig { params(uri: URI::Generic, range: Document::RangeShape).returns(T.nilable(T::Array[Interface::InlayHint])) }
     def inlay_hint(uri, range)
       document = @store.get(uri)
       return if document.syntax_error?
@@ -322,7 +322,7 @@ def inlay_hint(uri, range)
 
     sig do
       params(
-        uri: String,
+        uri: URI::Generic,
         range: Document::RangeShape,
         context: T::Hash[Symbol, T.untyped],
       ).returns(T.nilable(T::Array[Interface::CodeAction]))
@@ -335,7 +335,7 @@ def code_action(uri, range, context)
 
     sig { params(params: T::Hash[Symbol, T.untyped]).returns(Interface::CodeAction) }
     def code_action_resolve(params)
-      uri = params.dig(:data, :uri)
+      uri = URI(params.dig(:data, :uri))
       document = @store.get(uri)
       result = Requests::CodeActionResolve.new(document, params).run
 
@@ -363,7 +363,7 @@ def code_action_resolve(params)
       end
     end
 
-    sig { params(uri: String).returns(T.nilable(Interface::FullDocumentDiagnosticReport)) }
+    sig { params(uri: URI::Generic).returns(T.nilable(Interface::FullDocumentDiagnosticReport)) }
     def diagnostic(uri)
       response = @store.cache_fetch(uri, "textDocument/diagnostic") do |document|
         Requests::Diagnostics.new(document).run
@@ -372,7 +372,7 @@ def diagnostic(uri)
       Interface::FullDocumentDiagnosticReport.new(kind: "full", items: response.map(&:to_lsp_diagnostic)) if response
     end
 
-    sig { params(uri: String, range: Document::RangeShape).returns(Interface::SemanticTokens) }
+    sig { params(uri: URI::Generic, range: Document::RangeShape).returns(Interface::SemanticTokens) }
     def semantic_tokens_range(uri, range)
       document = @store.get(uri)
       start_line = range.dig(:start, :line)
@@ -390,7 +390,10 @@ def semantic_tokens_range(uri, range)
     end
 
     sig do
-      params(uri: String, position: Document::PositionShape).returns(T.nilable(T::Array[Interface::CompletionItem]))
+      params(
+        uri: URI::Generic,
+        position: Document::PositionShape,
+      ).returns(T.nilable(T::Array[Interface::CompletionItem]))
     end
     def completion(uri, position)
       document = @store.get(uri)

lib/ruby_lsp/extension.rb

@@ -109,7 +109,7 @@ def name; end
     # Creates a new CodeLens listener. This method is invoked on every CodeLens request
     sig do
       overridable.params(
-        uri: String,
+        uri: URI::Generic,
         emitter: EventEmitter,
         message_queue: Thread::Queue,
       ).returns(T.nilable(Listener[T::Array[Interface::CodeLens]]))

lib/ruby_lsp/internal.rb

@@ -7,6 +7,7 @@
 require "benchmark"
 require "bundler"
 require "uri"
+require "cgi"
 
 require "ruby-lsp"
 require "ruby_lsp/utils"

lib/ruby_lsp/requests/code_actions.rb

@@ -29,7 +29,7 @@ class CodeActions < BaseRequest
       def initialize(document, range, context)
         super(document)
 
-        @uri = T.let(document.uri, String)
+        @uri = T.let(document.uri, URI::Generic)
         @range = range
         @context = context
       end
@@ -63,14 +63,14 @@ def cover?(range)
           )
       end
 
-      sig { params(range: Document::RangeShape, uri: String).returns(Interface::CodeAction) }
+      sig { params(range: Document::RangeShape, uri: URI::Generic).returns(Interface::CodeAction) }
       def refactor_code_action(range, uri)
         Interface::CodeAction.new(
           title: "Refactor: Extract Variable",
           kind: Constant::CodeActionKind::REFACTOR_EXTRACT,
           data: {
             range: range,
-            uri: uri,
+            uri: uri.to_s,
           },
         )
       end

lib/ruby_lsp/requests/code_lens.rb

@@ -31,17 +31,17 @@ class CodeLens < Listener
       sig { override.returns(ResponseType) }
       attr_reader :response
 
-      sig { params(uri: String, emitter: EventEmitter, message_queue: Thread::Queue, test_library: String).void }
+      sig { params(uri: URI::Generic, emitter: EventEmitter, message_queue: Thread::Queue, test_library: String).void }
       def initialize(uri, emitter, message_queue, test_library)
         super(emitter, message_queue)
 
-        @uri = T.let(uri, String)
+        @uri = T.let(uri, URI::Generic)
         @external_listeners.concat(
           Extension.extensions.filter_map { |ext| ext.create_code_lens_listener(uri, emitter, message_queue) },
         )
         @test_library = T.let(test_library, String)
         @response = T.let([], ResponseType)
-        @path = T.let(T.must(URI(uri).path), String)
+        @path = T.let(uri.path, T.nilable(String))
         # visibility_stack is a stack of [current_visibility, previous_visibility]
         @visibility_stack = T.let([["public", "public"]], T::Array[T::Array[T.nilable(String)]])
         @class_stack = T.let([], T::Array[String])
@@ -106,7 +106,7 @@ def on_command(node)
         if ACCESS_MODIFIERS.include?(node_message) && node.arguments.parts.any?
           visibility, _ = @visibility_stack.pop
           @visibility_stack.push([node_message, visibility])
-        elsif @path.include?("Gemfile") && node_message.include?("gem") && node.arguments.parts.any?
+        elsif @path&.include?("Gemfile") && node_message.include?("gem") && node.arguments.parts.any?
           remote = resolve_gem_remote(node)
           return unless remote
 
@@ -160,7 +160,7 @@ def merge_response!(other)
       sig { params(node: SyntaxTree::Node, name: String, command: String, kind: Symbol).void }
       def add_test_code_lens(node, name:, command:, kind:)
         # don't add code lenses if the test library is not supported or unknown
-        return unless SUPPORTED_TEST_LIBRARIES.include?(@test_library)
+        return unless SUPPORTED_TEST_LIBRARIES.include?(@test_library) && @path
 
         arguments = [
           @path,
@@ -217,7 +217,7 @@ def resolve_gem_remote(node)
 
       sig { params(class_name: String, method_name: T.nilable(String)).returns(String) }
       def generate_test_command(class_name:, method_name: nil)
-        command = BASE_COMMAND + @path
+        command = BASE_COMMAND + T.must(@path)
 
         case @test_library
         when "minitest"

lib/ruby_lsp/requests/definition.rb

@@ -25,7 +25,7 @@ class Definition < Listener
       sig { override.returns(ResponseType) }
       attr_reader :response
 
-      sig { params(uri: String, emitter: EventEmitter, message_queue: Thread::Queue).void }
+      sig { params(uri: URI::Generic, emitter: EventEmitter, message_queue: Thread::Queue).void }
       def initialize(uri, emitter, message_queue)
         super(emitter, message_queue)
 
@@ -61,8 +61,8 @@ def on_command(node)
             )
           end
         when "require_relative"
-          current_file = T.must(URI.parse(@uri).path)
-          current_folder = Pathname.new(current_file).dirname
+          path = @uri.path
+          current_folder = path ? Pathname.new(CGI.unescape(path)).dirname : Dir.pwd
           candidate = File.expand_path(File.join(current_folder, required_file))
 
           if candidate

lib/ruby_lsp/requests/diagnostics.rb

@@ -25,7 +25,7 @@ class Diagnostics < BaseRequest
       def initialize(document)
         super(document)
 
-        @uri = T.let(document.uri, String)
+        @uri = T.let(document.uri, URI::Generic)
       end
 
       sig { override.returns(T.nilable(T.all(T::Array[Support::RuboCopDiagnostic], Object))) }
@@ -36,7 +36,8 @@ def run
         return unless defined?(Support::RuboCopDiagnosticsRunner)
 
         # Don't try to run RuboCop diagnostics for files outside the current working directory
-        return unless URI(@uri).path&.start_with?(T.must(WORKSPACE_URI.path))
+        path = @uri.path
+        return unless path.nil? || path.start_with?(T.must(WORKSPACE_URI.path))
 
         Support::RuboCopDiagnosticsRunner.instance.run(@uri, @document)
       end

lib/ruby_lsp/requests/document_link.rb

@@ -75,13 +75,13 @@ def gem_paths
       sig { override.returns(ResponseType) }
       attr_reader :response
 
-      sig { params(uri: String, emitter: EventEmitter, message_queue: Thread::Queue).void }
+      sig { params(uri: URI::Generic, emitter: EventEmitter, message_queue: Thread::Queue).void }
       def initialize(uri, emitter, message_queue)
         super(emitter, message_queue)
 
         # Match the version based on the version in the RBI file name. Notice that the `@` symbol is sanitized to `%40`
         # in the URI
-        version_match = /(?<=%40)[\d.]+(?=\.rbi$)/.match(uri)
+        version_match = uri.path ? /(?<=%40)[\d.]+(?=\.rbi$)/.match(CGI.unescape(uri.path)) : nil
         @gem_version = T.let(version_match && version_match[0], T.nilable(String))
         @response = T.let([], T::Array[Interface::DocumentLink])
 
@@ -95,7 +95,7 @@ def on_comment(node)
 
         uri = T.cast(URI(T.must(match[0])), URI::Source)
         gem_version = T.must(resolve_version(uri))
-        file_path = self.class.gem_paths.dig(uri.gem_name, gem_version, uri.path)
+        file_path = self.class.gem_paths.dig(uri.gem_name, gem_version, CGI.unescape(uri.path))
         return if file_path.nil?
 
         @response << Interface::DocumentLink.new(