From 16249b945e5677dd4a265f5e4fcfb8e8f384a62c Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Thu, 31 Aug 2023 12:38:28 -0400 Subject: [PATCH] wip --- Gemfile.lock | 2 +- VERSION | 2 +- .../lib/ruby_indexer/configuration.rb | 2 +- lib/ruby_lsp/executor.rb | 58 ++++----- lib/ruby_lsp/requests/code_lens.rb | 4 +- lib/ruby_lsp/requests/document_symbol.rb | 117 +++++++++--------- lib/ruby_lsp/requests/on_type_formatting.rb | 7 +- test/executor_test.rb | 8 +- test/integration_test.rb | 8 +- test/requests/code_lens_expectations_test.rb | 18 +++ test/requests/definition_expectations_test.rb | 2 - test/requests/on_type_formatting_test.rb | 39 +++++- 12 files changed, 161 insertions(+), 106 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index c31461c571..3224b8794a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - ruby-lsp (0.9.1) + ruby-lsp (0.9.2) language_server-protocol (~> 3.17.0) sorbet-runtime syntax_tree (>= 6.1.1, < 7) diff --git a/VERSION b/VERSION index f374f6662e..2003b639c4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.9.1 +0.9.2 diff --git a/lib/ruby_indexer/lib/ruby_indexer/configuration.rb b/lib/ruby_indexer/lib/ruby_indexer/configuration.rb index 7e102e3ca9..368c9177c5 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/configuration.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/configuration.rb @@ -134,7 +134,7 @@ def files_to_index sig { returns(Regexp) } def magic_comment_regex - /^\s*#\s*#{@excluded_magic_comments.join("|")}/ + @magic_comment_regex ||= T.let(/^\s*#\s*#{@excluded_magic_comments.join("|")}/, T.nilable(Regexp)) end private diff --git a/lib/ruby_lsp/executor.rb b/lib/ruby_lsp/executor.rb index fb3181f1e5..33acc50865 100644 --- a/lib/ruby_lsp/executor.rb +++ b/lib/ruby_lsp/executor.rb @@ -204,8 +204,6 @@ def did_change_watched_files(changes) sig { void } def perform_initial_indexing - return unless @store.experimental_features - # The begin progress invocation happens during `initialize`, so that the notification is sent before we are # stuck indexing files RubyIndexer.configuration.load_config @@ -567,7 +565,7 @@ def initialize_request(options) Interface::DocumentSymbolClientCapabilities.new( hierarchical_document_symbol_support: true, symbol_kind: { - value_set: Requests::DocumentSymbol::SYMBOL_KIND.values, + value_set: (Constant::SymbolKind::FILE..Constant::SymbolKind::TYPE_PARAMETER).to_a, }, ) end @@ -629,37 +627,35 @@ def initialize_request(options) ) end - if @store.experimental_features - # Dynamically registered capabilities - file_watching_caps = options.dig(:capabilities, :workspace, :didChangeWatchedFiles) - - # Not every client supports dynamic registration or file watching - if file_watching_caps&.dig(:dynamicRegistration) && file_watching_caps&.dig(:relativePatternSupport) - @message_queue << Request.new( - message: "client/registerCapability", - params: Interface::RegistrationParams.new( - registrations: [ - # Register watching Ruby files - Interface::Registration.new( - id: "workspace/didChangeWatchedFiles", - method: "workspace/didChangeWatchedFiles", - register_options: Interface::DidChangeWatchedFilesRegistrationOptions.new( - watchers: [ - Interface::FileSystemWatcher.new( - glob_pattern: "**/*.rb", - kind: Constant::WatchKind::CREATE | Constant::WatchKind::CHANGE | Constant::WatchKind::DELETE, - ), - ], - ), + # Dynamically registered capabilities + file_watching_caps = options.dig(:capabilities, :workspace, :didChangeWatchedFiles) + + # Not every client supports dynamic registration or file watching + if file_watching_caps&.dig(:dynamicRegistration) && file_watching_caps&.dig(:relativePatternSupport) + @message_queue << Request.new( + message: "client/registerCapability", + params: Interface::RegistrationParams.new( + registrations: [ + # Register watching Ruby files + Interface::Registration.new( + id: "workspace/didChangeWatchedFiles", + method: "workspace/didChangeWatchedFiles", + register_options: Interface::DidChangeWatchedFilesRegistrationOptions.new( + watchers: [ + Interface::FileSystemWatcher.new( + glob_pattern: "**/*.rb", + kind: Constant::WatchKind::CREATE | Constant::WatchKind::CHANGE | Constant::WatchKind::DELETE, + ), + ], ), - ], - ), - ) - end - - begin_progress("indexing-progress", "Ruby LSP: indexing files") + ), + ], + ), + ) end + begin_progress("indexing-progress", "Ruby LSP: indexing files") + Interface::InitializeResult.new( capabilities: Interface::ServerCapabilities.new( text_document_sync: Interface::TextDocumentSyncOptions.new( diff --git a/lib/ruby_lsp/requests/code_lens.rb b/lib/ruby_lsp/requests/code_lens.rb index 2f079a36b7..5dc87cae0e 100644 --- a/lib/ruby_lsp/requests/code_lens.rb +++ b/lib/ruby_lsp/requests/code_lens.rb @@ -65,7 +65,7 @@ def on_class(node) class_name = node.constant.constant.value @class_stack.push(class_name) - if class_name.end_with?("Test") + if @path && class_name.end_with?("Test") add_test_code_lens( node, name: class_name, @@ -89,7 +89,7 @@ def on_def(node) visibility, _ = @visibility_stack.last if visibility == "public" method_name = node.name.value - if method_name.start_with?("test_") + if @path && method_name.start_with?("test_") add_test_code_lens( node, name: method_name, diff --git a/lib/ruby_lsp/requests/document_symbol.rb b/lib/ruby_lsp/requests/document_symbol.rb index 1da3d1efad..caa829adff 100644 --- a/lib/ruby_lsp/requests/document_symbol.rb +++ b/lib/ruby_lsp/requests/document_symbol.rb @@ -32,38 +32,6 @@ class DocumentSymbol < Listener ResponseType = type_member { { fixed: T::Array[Interface::DocumentSymbol] } } - SYMBOL_KIND = T.let( - { - file: 1, - module: 2, - namespace: 3, - package: 4, - class: 5, - method: 6, - property: 7, - field: 8, - constructor: 9, - enum: 10, - interface: 11, - function: 12, - variable: 13, - constant: 14, - string: 15, - number: 16, - boolean: 17, - array: 18, - object: 19, - key: 20, - null: 21, - enummember: 22, - struct: 23, - event: 24, - operator: 25, - typeparameter: 26, - }.freeze, - T::Hash[Symbol, Integer], - ) - ATTR_ACCESSORS = T.let(["attr_reader", "attr_writer", "attr_accessor"].freeze, T::Array[String]) class SymbolHierarchyRoot @@ -92,13 +60,17 @@ def initialize(emitter, message_queue) T::Array[T.any(SymbolHierarchyRoot, Interface::DocumentSymbol)], ) + @external_listeners.concat( + Extension.extensions.filter_map { |ext| ext.create_document_symbol_listener(emitter, message_queue) }, + ) + emitter.register( self, :on_class, :after_class, :on_call, - :on_constant_path_write, :on_constant_write, + :on_constant_path_write, :on_def, :after_def, :on_module, @@ -108,13 +80,20 @@ def initialize(emitter, message_queue) ) end + # Merges responses from other listeners + sig { override.params(other: Listener[ResponseType]).returns(T.self_type) } + def merge_response!(other) + @response.concat(other.response) + self + end + sig { params(node: YARP::ClassNode).void } def on_class(node) @stack << create_document_symbol( name: node.constant_path.location.slice, - kind: :class, + kind: Constant::SymbolKind::CLASS, range_node: node, - selection_range_node: node.constant_path, + selection_range_node: node.constant_path.location, ) end @@ -125,7 +104,7 @@ def after_class(node) sig { params(node: YARP::CallNode).void } def on_call(node) - return unless ATTR_ACCESSORS.include?(node.name) && node.receiver.nil? + return unless ATTR_ACCESSORS.include?(node.name) arguments = node.arguments return unless arguments @@ -135,18 +114,18 @@ def on_call(node) create_document_symbol( name: argument.value, - kind: :field, + kind: Constant::SymbolKind::FIELD, range_node: argument, - selection_range_node: argument, + selection_range_node: argument.location, ) end end - sig { params(node: YARP::ConstantPathWriteNode).void } + sig { params(node: YARP::ConstantPathWrite).void } def on_constant_path_write(node) create_document_symbol( name: node.target.location.slice, - kind: :constant, + kind: Constant::SymbolKind::CONSTANT, range_node: node, selection_range_node: node.target, ) @@ -156,7 +135,7 @@ def on_constant_path_write(node) def on_constant_write(node) create_document_symbol( name: node.name, - kind: :constant, + kind: Constant::SymbolKind::CONSTANT, range_node: node, selection_range_node: node.name_loc, ) @@ -176,9 +155,9 @@ def on_def(node) symbol = create_document_symbol( name: name, - kind: kind, + kind: Constant::SymbolKind::METHOD, range_node: node, - selection_range_node: node.name_loc, + selection_range_node: node.name_loc ) @stack << symbol @@ -193,9 +172,9 @@ def after_def(node) def on_module(node) @stack << create_document_symbol( name: node.constant_path.location.slice, - kind: :module, + kind: Constant::SymbolKind::MODULE, range_node: node, - selection_range_node: node.constant_path, + selection_range_node: node.constant_path.location ) end @@ -208,7 +187,7 @@ def after_module(node) def on_instance_variable_write(node) create_document_symbol( name: node.name, - kind: :variable, + kind: Constant::SymbolKind::VARIABLE, range_node: node, selection_range_node: node.name_loc, ) @@ -218,34 +197,58 @@ def on_instance_variable_write(node) def on_class_variable_write(node) create_document_symbol( name: node.name, - kind: :variable, + kind: Constant::SymbolKind::VARIABLE, range_node: node, selection_range_node: node.name_loc, ) end + # sig { params(node: YARP::TopConstField).void } + # def on_top_const_field(node) + # create_document_symbol( + # name: node.constant.value, + # kind: Constant::SymbolKind::CONSTANT, + # range_node: node, + # selection_range_node: node.constant, + # ) + # end + + # sig { params(node: YARP::VarField).void } + # def on_var_field(node) + # value = node.value + # kind = case value + # when YARP::Const + # Constant::SymbolKind::CONSTANT + # when YARP::CVar, YARP::IVar + # Constant::SymbolKind::VARIABLE + # else + # return + # end + + # create_document_symbol( + # name: value.value, + # kind: kind, + # range_node: node, + # selection_range_node: value, + # ) + # end + private sig do params( name: String, - kind: Symbol, + kind: Integer, range_node: YARP::Node, - selection_range_node: T.any(YARP::Node, YARP::Location), + selection_range_node: YARP::Location, ).returns(Interface::DocumentSymbol) end def create_document_symbol(name:, kind:, range_node:, selection_range_node:) - selection_range = if selection_range_node.is_a?(YARP::Node) - range_from_syntax_tree_node(selection_range_node) - else - range_from_location(selection_range_node) - end - symbol = Interface::DocumentSymbol.new( name: name, - kind: SYMBOL_KIND[kind], + kind: kind, range: range_from_syntax_tree_node(range_node), - selection_range: selection_range, + selection_range: range_from_location(selection_range_node), children: [], ) diff --git a/lib/ruby_lsp/requests/on_type_formatting.rb b/lib/ruby_lsp/requests/on_type_formatting.rb index 3bc5d2a1aa..6ce2bcee8f 100644 --- a/lib/ruby_lsp/requests/on_type_formatting.rb +++ b/lib/ruby_lsp/requests/on_type_formatting.rb @@ -112,11 +112,12 @@ def handle_statement_end next_line = @lines[@position[:line] + 1] if current_line.nil? || current_line.strip.empty? - add_edit_with_text(" \n#{indents}end") + add_edit_with_text("\n") + add_edit_with_text("#{indents}end") move_cursor_to(@position[:line], @indentation + 2) elsif next_line.nil? || next_line.strip.empty? - add_edit_with_text("#{indents}end", { line: @position[:line] + 1, character: @position[:character] }) - move_cursor_to(@position[:line], @indentation + 3) + add_edit_with_text("#{indents}end\n", { line: @position[:line] + 1, character: @position[:character] }) + move_cursor_to(@position[:line] - 1, @indentation + @previous_line.size + 1) end end diff --git a/test/executor_test.rb b/test/executor_test.rb index 550595de07..076abe6e00 100644 --- a/test/executor_test.rb +++ b/test/executor_test.rb @@ -109,14 +109,12 @@ def test_initialize_uses_utf_16_if_no_encodings_are_specified end def test_initialized_populates_index - @store.experimental_features = true @executor.execute({ method: "initialized", params: {} }) index = @executor.instance_variable_get(:@index) refute_empty(index.instance_variable_get(:@entries)) end def test_initialized_recovers_from_indexing_failures - @store.experimental_features = true RubyIndexer::Index.any_instance.expects(:index_all).once.raises(StandardError, "boom!") @executor.execute({ method: "initialized", params: {} }) @@ -204,6 +202,12 @@ def test_shows_error_if_formatter_set_to_rubocop_but_rubocop_not_available assert_equal("none", @store.formatter) refute_empty(@message_queue) + + # Account for starting and ending the progress notifications during initialized + assert_equal("window/workDoneProgress/create", @message_queue.pop.message) + assert_equal("$/progress", @message_queue.pop.message) + assert_equal("$/progress", @message_queue.pop.message) + notification = T.must(@message_queue.pop) assert_equal("window/showMessage", notification.message) assert_equal( diff --git a/test/integration_test.rb b/test/integration_test.rb index 52dc04feb0..e0ed3ba732 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -69,7 +69,7 @@ def test_document_symbol response = make_request("textDocument/documentSymbol", { textDocument: { uri: @uri } }) symbol = response[:result].first assert_equal("Foo", symbol[:name]) - assert_equal(RubyLsp::Requests::DocumentSymbol::SYMBOL_KIND[:class], symbol[:kind]) + assert_equal(LanguageServer::Protocol::Constant::SymbolKind::CLASS, symbol[:kind]) end def test_document_highlight @@ -372,11 +372,9 @@ def test_diagnostics end def test_workspace_symbol - initialize_lsp(["workspaceSymbol"], experimental_features_enabled: true) + initialize_lsp(["workspaceSymbol"]) open_file_with("class Foo\nend") # Read the response for the progress indicator notifications - read_response("window/workDoneProgress/create") - read_response("$/progress") read_response("textDocument/didOpen") # Populate the index @@ -455,6 +453,8 @@ def initialize_lsp(enabled_features, experimental_features_enabled: false) enabled_providers = enabled_features.map { |feature| FEATURE_TO_PROVIDER[feature] } assert_equal([:positionEncoding, :textDocumentSync, *enabled_providers], response[:capabilities].keys) + read_response("window/workDoneProgress/create") + read_response("$/progress") end def open_file_with(content) diff --git a/test/requests/code_lens_expectations_test.rb b/test/requests/code_lens_expectations_test.rb index 1b3fcd1113..85ea946ecf 100644 --- a/test/requests/code_lens_expectations_test.rb +++ b/test/requests/code_lens_expectations_test.rb @@ -85,6 +85,24 @@ def test_bar; end assert_empty(response) end + def test_no_code_lens_for_unsaved_files + source = <<~RUBY + class FooTest < Test::Unit::TestCase + def test_bar; end + end + RUBY + uri = URI::Generic.build(scheme: "untitled", opaque: "Untitled-1") + + document = RubyLsp::Document.new(source: source, version: 1, uri: uri) + + emitter = RubyLsp::EventEmitter.new + listener = RubyLsp::Requests::CodeLens.new(uri, emitter, @message_queue, "minitest") + emitter.visit(document.tree) + response = listener.response + + assert_empty(response) + end + def test_code_lens_extensions skip diff --git a/test/requests/definition_expectations_test.rb b/test/requests/definition_expectations_test.rb index 2b0cac57a3..d29d676828 100644 --- a/test/requests/definition_expectations_test.rb +++ b/test/requests/definition_expectations_test.rb @@ -12,7 +12,6 @@ def run_expectations(source) position = @__params&.first || { character: 0, line: 0 } store = RubyLsp::Store.new - store.experimental_features = true store.set(uri: URI("file:///folder/fake.rb"), source: source, version: 1) executor = RubyLsp::Executor.new(store, message_queue) @@ -60,7 +59,6 @@ def test_jumping_to_default_gems uri = URI::Generic.from_path(path: path) store = RubyLsp::Store.new - store.experimental_features = true store.set(uri: URI("file:///folder/fake.rb"), source: <<~RUBY, version: 1) Pathname RUBY diff --git a/test/requests/on_type_formatting_test.rb b/test/requests/on_type_formatting_test.rb index 67fbdc2c6d..ea242f7e5e 100644 --- a/test/requests/on_type_formatting_test.rb +++ b/test/requests/on_type_formatting_test.rb @@ -20,7 +20,11 @@ def test_adding_missing_ends expected_edits = [ { range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, - newText: " \nend", + newText: "\n", + }, + { + range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, + newText: "end", }, { range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, @@ -302,7 +306,11 @@ def test_breaking_line_between_keyword_and_more_content expected_edits = [ { range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, - newText: " \nend", + newText: "\n", + }, + { + range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, + newText: "end", }, { range: { start: { line: 1, character: 2 }, end: { line: 1, character: 2 } }, @@ -328,4 +336,31 @@ def test_breaking_line_between_keyword_when_there_is_content_on_the_next_line edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 0, character: 2 }, "\n").run assert_empty(edits) end + + def test_breaking_line_immediately_after_keyword + document = RubyLsp::Document.new(source: +"", version: 1, uri: URI("file:///fake.rb")) + + document.push_edits( + [{ + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, + text: " def\nfoo", + }], + version: 2, + ) + document.parse + + edits = RubyLsp::Requests::OnTypeFormatting.new(document, { line: 1, character: 2 }, "\n").run + expected_edits = [ + { + range: { start: { line: 2, character: 2 }, end: { line: 2, character: 2 } }, + newText: " end\n", + }, + { + range: { start: { line: 0, character: 6 }, end: { line: 0, character: 6 } }, + newText: "$0", + }, + ] + + assert_equal(expected_edits.to_json, T.must(edits).to_json) + end end