From 7208489d916a03117e0a2d5224854837c609a76d Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Fri, 13 Sep 2024 16:26:04 -0400 Subject: [PATCH] Automatically switch overload in signature help Co-authored-by: Andy Waite --- lib/ruby_indexer/lib/ruby_indexer/entry.rb | 18 ++--- lib/ruby_lsp/listeners/signature_help.rb | 79 +++++++++++++++------- test/requests/signature_help_test.rb | 62 +++++++++++++++++ 3 files changed, 124 insertions(+), 35 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb index 396a4941e..019eb9157 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb @@ -322,11 +322,6 @@ class Member < Entry sig { returns(T.nilable(Entry::Namespace)) } attr_reader :owner - sig { returns(T::Array[RubyIndexer::Entry::Parameter]) } - def parameters - T.must(signatures.first).parameters - end - sig do params( name: String, @@ -557,11 +552,6 @@ def initialize(target, unresolved_alias) @owner = T.let(unresolved_alias.owner, T.nilable(Entry::Namespace)) end - sig { returns(T::Array[Parameter]) } - def parameters - @target.parameters - end - sig { returns(String) } def decorated_parameters @target.decorated_parameters @@ -690,7 +680,13 @@ def keyword_arguments_match?(args, names) return true unless args return true if args.any? { |arg| arg.is_a?(Prism::AssocSplatNode) } - arg_names = args.filter_map { |arg| arg.key.value.to_sym if arg.is_a?(Prism::AssocNode) } + arg_names = args.filter_map do |arg| + next unless arg.is_a?(Prism::AssocNode) + + key = arg.key + key.value&.to_sym if key.is_a?(Prism::SymbolNode) + end + (arg_names - names).empty? end end diff --git a/lib/ruby_lsp/listeners/signature_help.rb b/lib/ruby_lsp/listeners/signature_help.rb index 993902dcb..59f2ac729 100644 --- a/lib/ruby_lsp/listeners/signature_help.rb +++ b/lib/ruby_lsp/listeners/signature_help.rb @@ -42,17 +42,46 @@ def on_call_node_enter(node) target_method = methods.first return unless target_method - parameters = target_method.parameters - name = target_method.name + signatures = target_method.signatures # If the method doesn't have any parameters, there's no need to show signature help - return if parameters.empty? + return if signatures.empty? + + name = target_method.name + title = +"" + + extra_links = if type.is_a?(TypeInferrer::GuessedType) + title << "\n\nGuessed receiver: #{type.name}" + "[Learn more about guessed types](#{GUESSED_TYPES_URL})" + end - label = "#{name}(#{parameters.map(&:decorated_name).join(", ")})" + active_signature, active_parameter = determine_active_signature_and_parameter(node, signatures) + signature_help = Interface::SignatureHelp.new( + signatures: generate_signatures(signatures, name, methods, title, extra_links), + active_signature: active_signature, + active_parameter: active_parameter, + ) + @response_builder.replace(signature_help) + end + + private + + sig do + params(node: Prism::CallNode, signatures: T::Array[RubyIndexer::Entry::Signature]).returns([Integer, Integer]) + end + def determine_active_signature_and_parameter(node, signatures) arguments_node = node.arguments arguments = arguments_node&.arguments || [] - active_parameter = (arguments.length - 1).clamp(0, parameters.length - 1) + + # Find the first signature that matches the current arguments. If the user is invoking a method incorrectly and + # none of the signatures match, we show the first one + active_sig_index = signatures.find_index do |signature| + signature.matches?(arguments) + end || 0 + + parameter_length = [T.must(signatures[active_sig_index]).parameters.length - 1, 0].max + active_parameter = (arguments.length - 1).clamp(0, parameter_length) # If there are arguments, then we need to check if there's a trailing comma after the end of the last argument # to advance the active parameter to the next one @@ -61,27 +90,29 @@ def on_call_node_enter(node) active_parameter += 1 end - title = +"" - - extra_links = if type.is_a?(TypeInferrer::GuessedType) - title << "\n\nGuessed receiver: #{type.name}" - "[Learn more about guessed types](#{GUESSED_TYPES_URL})" - end + [active_sig_index, active_parameter] + end - signature_help = Interface::SignatureHelp.new( - signatures: [ - Interface::SignatureInformation.new( - label: label, - parameters: parameters.map { |param| Interface::ParameterInformation.new(label: param.name) }, - documentation: Interface::MarkupContent.new( - kind: "markdown", - value: markdown_from_index_entries(title, methods, extra_links: extra_links), - ), + sig do + params( + signatures: T::Array[RubyIndexer::Entry::Signature], + method_name: String, + methods: T::Array[RubyIndexer::Entry], + title: String, + extra_links: T.nilable(String), + ).returns(T::Array[Interface::SignatureInformation]) + end + def generate_signatures(signatures, method_name, methods, title, extra_links) + signatures.map do |signature| + Interface::SignatureInformation.new( + label: "#{method_name}(#{signature.format})", + parameters: signature.parameters.map { |param| Interface::ParameterInformation.new(label: param.name) }, + documentation: Interface::MarkupContent.new( + kind: "markdown", + value: markdown_from_index_entries(title, methods, extra_links: extra_links), ), - ], - active_parameter: active_parameter, - ) - @response_builder.replace(signature_help) + ) + end end end end diff --git a/test/requests/signature_help_test.rb b/test/requests/signature_help_test.rb index 8f7d1ec99..488c9d2e6 100644 --- a/test/requests/signature_help_test.rb +++ b/test/requests/signature_help_test.rb @@ -395,4 +395,66 @@ def subscribe!(news_letter) assert_match("Guessed receiver: User", signature.documentation.value) end end + + def test_automatically_detects_active_overload + # First step overload: just a block + source = <<~RUBY + 5.step() + RUBY + + with_server(source) do |server, uri| + index = server.instance_variable_get(:@global_state).index + RubyIndexer::RBSIndexer.new(index).index_ruby_core + + server.process_message(id: 1, method: "textDocument/signatureHelp", params: { + textDocument: { uri: uri }, + position: { line: 0, character: 7 }, + context: {}, + }) + + result = server.pop_response.response + signature = result.signatures[result.active_signature] + assert_equal("step(&)", signature.label) + end + + # Second step overload: with positional arguments + source = <<~RUBY + 5.step(1) + RUBY + + with_server(source) do |server, uri| + index = server.instance_variable_get(:@global_state).index + RubyIndexer::RBSIndexer.new(index).index_ruby_core + + server.process_message(id: 2, method: "textDocument/signatureHelp", params: { + textDocument: { uri: uri }, + position: { line: 0, character: 8 }, + context: {}, + }) + + result = server.pop_response.response + signature = result.signatures[result.active_signature] + assert_equal("step(limit, step = , &)", signature.label) + end + + # Third step overload: with keyword arguments + source = <<~RUBY + 5.step(to: 5) + RUBY + + with_server(source) do |server, uri| + index = server.instance_variable_get(:@global_state).index + RubyIndexer::RBSIndexer.new(index).index_ruby_core + + server.process_message(id: 2, method: "textDocument/signatureHelp", params: { + textDocument: { uri: uri }, + position: { line: 0, character: 8 }, + context: {}, + }) + + result = server.pop_response.response + signature = result.signatures[result.active_signature] + assert_equal("step(to:, by: , &)", signature.label) + end + end end