Improve signature combination

.github/workflows/rspec.yml

@@ -21,20 +21,51 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        ruby-version: ['3.0', '3.1', '3.2', '3.3', '3.4', '4.0']
-        rbs-version: ['3.6.1', '3.9.5', '4.0.0.dev.4']
+        ruby-version: ['3.0', '3.1', '3.2', '3.3', '3.4', '4.0', 'head']
+        rbs-version: ['3.6.1', '3.8.1', '3.9.5', '3.10.0', '4.0.0.dev.4']
         # Ruby 3.0 doesn't work with RBS 3.9.4 or 4.0.0.dev.4
         exclude:
+          # only include the 3.0 variants we include later
           - ruby-version: '3.0'
-            rbs-version: '3.9.5'
-          - ruby-version: '3.0'
-            rbs-version: '4.0.0.dev.4'
-          # Missing require in 'rbs collection update' - hopefully
-          # fixed in next RBS release
+          # only include the 3.1 variants we include later
+          - ruby-version: '3.1'
+          # only include the 3.2 variants we include later
+          - ruby-version: '3.2'
+          # only include the 3.3 variants we include later
+          - ruby-version: '3.3'
+          # only include the 3.4 variants we include later
+          - ruby-version: '3.4'
+          # only include the 4.0 variants we include later
           - ruby-version: '4.0'
+          # Don't exclude 'head' - let's test all RBS versions we
+          # can there.
+          #
+          #
+          # Just exclude some odd-ball compatibility issues we can't
+          # work around:
+          #
+          # https://github.com/castwide/solargraph/actions/runs/20627923548/job/59241444380?pr=1102
+          - ruby-version: 'head'
             rbs-version: '3.6.1'
-          - ruby-version: '4.0'
+          - ruby-version: 'head'
+            rbs-version: '3.8.1'
+          - ruby-version: 'head'
+            rbs-version: '4.0.0.dev.4'
+        include:
+          - ruby-version: '3.0'
+            rbs-version: '3.6.1'
+          - ruby-version: '3.1'
+            rbs-version: '3.6.1'
+          - ruby-version: '3.2'
+            rbs-version: '3.8.1'
+          - ruby-version: '3.3'
+            rbs-version: '3.9.5'
+          - ruby-version: '3.3'
+            rbs-version: '3.10.0'
+          - ruby-version: '3.4'
             rbs-version: '4.0.0.dev.4'
+          - ruby-version: '4.0'
+            rbs-version: '3.10.0'
     steps:
     - uses: actions/checkout@v3
     - name: Set up Ruby

.rubocop_todo.yml

@@ -468,6 +468,7 @@ Metrics/ClassLength:
   Exclude:
     - 'lib/solargraph/api_map.rb'
     - 'lib/solargraph/language_server/host.rb'
+    - 'lib/solargraph/pin/method.rb'
     - 'lib/solargraph/rbs_map/conversions.rb'
     - 'lib/solargraph/type_checker.rb'
 
@@ -1175,7 +1176,6 @@ Style/StringLiterals:
 # This cop supports safe autocorrection (--autocorrect).
 Style/SuperArguments:
   Exclude:
-    - 'lib/solargraph/pin/base_variable.rb'
     - 'lib/solargraph/pin/callable.rb'
     - 'lib/solargraph/pin/method.rb'
     - 'lib/solargraph/pin/signature.rb'

lib/solargraph/doc_map.rb

@@ -346,7 +346,7 @@ def gemspec_or_preference gemspec
     end
 
     # @param gemspec [Gem::Specification]
-    # @param version [Gem::Version]
+    # @param version [Gem::Version, String]
     # @return [Gem::Specification]
     def change_gemspec_version gemspec, version
       Gem::Specification.find_by_name(gemspec.name, "= #{version}")

lib/solargraph/parser/parser_gem/node_methods.rb

@@ -119,7 +119,6 @@ def convert_hash node
           result
         end
 
-        # @sg-ignore Wrong argument type for AST::Node.new: type expected AST::_ToSym, received :nil
         NIL_NODE = ::Parser::AST::Node.new(:nil)
 
         # @param node [Parser::AST::Node]

lib/solargraph/pin/base.rb

@@ -205,7 +205,9 @@ def combine_return_type(other)
 
       def dodgy_return_type_source?
         # uses a lot of 'Object' instead of 'self'
-        location&.filename&.include?('core_ext/object/')
+        location&.filename&.include?('core_ext/object/') ||
+          # ditto
+          location&.filename&.include?('stdlib/date/0/date.rbs')
       end
 
       # when choices are arbitrary, make sure the choice is consistent

lib/solargraph/pin/callable.rb

@@ -89,18 +89,40 @@ def blockless_parameters
         end
       end
 
-      # @return [Array]
+      # e.g., [["T"], "", "?", "foo:"] - parameter arity declarations,
+      #   ignoring positional names.  Used to match signatures.
+      #
+      # @return [Array<Array<String>, String>]
       def arity
         [generics, blockless_parameters.map(&:arity_decl), block&.arity]
       end
 
+      # e.g., [["T"], "1", "?3", "foo:5"] - parameter arity
+      #   declarations, including the number of unique types in each
+      #   parameter.  Used to determine whether combining two
+      #   signatures has lost useful information mapping specific
+      #   parameter types to specific return types.
+      #
+      # @return [Array<Array, String>]
+      def type_arity
+        [generics, blockless_parameters.map(&:type_arity_decl), block&.type_arity]
+      end
+
+      # Same as type_arity, but includes return type arity at the front.
+      #
+      # @return [Array<Array, String, nil>]
+      def full_type_arity
+        [return_type ? return_type.items.count.to_s : nil] + type_arity
+      end
+
       # @param generics_to_resolve [Enumerable<String>]
       # @param arg_types [Array<ComplexType>, nil]
       # @param return_type_context [ComplexType, nil]
       # @param yield_arg_types [Array<ComplexType>, nil]
       # @param yield_return_type_context [ComplexType, nil]
       # @param context [ComplexType, nil]
       # @param resolved_generic_values [Hash{String => ComplexType}]
+      #
       # @return [self]
       def resolve_generics_from_context(generics_to_resolve,
                                         arg_types = nil,
@@ -150,6 +172,7 @@ def method_name
       # @param yield_return_type_context [ComplexType, nil]
       # @param context [ComplexType, nil]
       # @param resolved_generic_values [Hash{String => ComplexType}]
+      #
       # @return [self]
       def resolve_generics_from_context_until_complete(generics_to_resolve,
                                                        arg_types = nil,
@@ -215,8 +238,13 @@ def mandatory_positional_param_count
         parameters.count(&:arg?)
       end
 
+      # @return [String]
+      def parameters_to_rbs
+        rbs_generics + '(' + parameters.map { |param| param.to_rbs }.join(', ') + ') ' + (block.nil? ? '' : '{ ' + block.to_rbs + ' } ')
+      end
+
       def to_rbs
-        rbs_generics + '(' + parameters.map { |param| param.to_rbs }.join(', ') + ') ' + (block.nil? ? '' : '{ ' + block.to_rbs + ' } ') + '-> ' + return_type.to_rbs
+        parameters_to_rbs + '-> ' + return_type.to_rbs
       end
 
       def block?

lib/solargraph/pin/method.rb

@@ -34,26 +34,6 @@ def initialize visibility: :public, explicit: true, block: :undefined, node: nil
         @anon_splat = anon_splat
       end
 
-      # @param signature_pins [Array<Pin::Signature>]
-      # @return [Array<Pin::Signature>]
-      def combine_all_signature_pins(*signature_pins)
-        # @type [Hash{Array => Array<Pin::Signature>}]
-        by_arity = {}
-        signature_pins.each do |signature_pin|
-          by_arity[signature_pin.arity] ||= []
-          by_arity[signature_pin.arity] << signature_pin
-        end
-        by_arity.transform_values! do |same_arity_pins|
-          # @param memo [Pin::Signature, nil]
-          # @param signature [Pin::Signature]
-          same_arity_pins.reduce(nil) do |memo, signature|
-            next signature if memo.nil?
-            memo.combine_with(signature)
-          end
-        end
-        by_arity.values.flatten
-      end
-
       # @param other [Pin::Method]
       # @return [::Symbol]
       def combine_visibility(other)
@@ -66,20 +46,6 @@ def combine_visibility(other)
         end
       end
 
-      # @param other [Pin::Method]
-      # @return [Array<Pin::Signature>]
-      def combine_signatures(other)
-        all_undefined = signatures.all? { |sig| sig.return_type.undefined? }
-        other_all_undefined = other.signatures.all? { |sig| sig.return_type.undefined? }
-        if all_undefined && !other_all_undefined
-          other.signatures
-        elsif other_all_undefined && !all_undefined
-          signatures
-        else
-          combine_all_signature_pins(*signatures, *other.signatures)
-        end
-      end
-
       def combine_with(other, attrs = {})
         priority_choice = choose_priority(other)
         return priority_choice unless priority_choice.nil?
@@ -485,6 +451,71 @@ def dodgy_visibility_source?
 
       private
 
+      # @param other [Pin::Method]
+      # @return [Array<Pin::Signature>]
+      def combine_signatures(other)
+        all_undefined = signatures.all? { |sig| !sig.return_type&.defined? }
+        other_all_undefined = other.signatures.all? { |sig| !sig.return_type&.defined? }
+        if all_undefined && !other_all_undefined
+          other.signatures
+        elsif other_all_undefined && !all_undefined
+          signatures
+        else
+          combine_signatures_by_type_arity(*signatures, *other.signatures)
+        end
+      end
+
+      # @param signature_pins [Array<Pin::Signature>]
+      #
+      # @return [Array<Pin::Signature>]
+      def combine_signatures_by_type_arity(*signature_pins)
+        # @type [Hash{Array => Array<Pin::Signature>}]
+        by_type_arity = {}
+        signature_pins.each do |signature_pin|
+          by_type_arity[signature_pin.type_arity] ||= []
+          by_type_arity[signature_pin.type_arity] << signature_pin
+        end
+
+        by_type_arity.transform_values! do |same_type_arity_signatures|
+          combine_same_type_arity_signatures same_type_arity_signatures
+        end
+        by_type_arity.values.flatten
+      end
+
+      # @param same_type_arity_signatures [Array<Pin::Signature>]
+      #
+      # @return [Array<Pin::Signature>]
+      def combine_same_type_arity_signatures(same_type_arity_signatures)
+        # This is an O(n^2) operation, so bail out if n is not small
+        return same_type_arity_signatures if same_type_arity_signatures.length > 10
+
+        # @param old_signatures [Array<Pin::Signature>]
+        # @param new_signature [Pin::Signature]
+        same_type_arity_signatures.reduce([]) do |old_signatures, new_signature|
+          next [new_signature] if old_signatures.empty?
+
+          found_merge = false
+          old_signatures.flat_map do |old_signature|
+            potential_new_signature = old_signature.combine_with(new_signature)
+
+            if potential_new_signature.type_arity == old_signature.type_arity
+              # the number of types in each parameter and return type
+              # match, so we found compatible signatures to merge.  If
+              # we increased the number of types, we'd potentially
+              # have taken away the ability to use parameter types to
+              # choose the correct return type (while Ruby doesn't
+              # dispatch based on type, RBS does distinguish overloads
+              # based on types, not just arity, allowing for type
+              # information describing how methods behave based on
+              # their input types)
+              old_signatures - [old_signature] + [potential_new_signature]
+            else
+              old_signatures + [new_signature]
+            end
+          end
+        end
+      end
+
       # @param name [String]
       # @param asgn [Boolean]
       #

lib/solargraph/pin/parameter.rb

@@ -72,6 +72,11 @@ def arity_decl
         end
       end
 
+      # @return [String]
+      def type_arity_decl
+        arity_decl + return_type.items.count.to_s
+      end
+
       def arg?
         decl == :arg
       end

lib/solargraph/workspace.rb

@@ -70,10 +70,10 @@ def merge *sources
 
       includes_any = false
       sources.each do |source|
-        if directory == "*" || config.calculated.include?(source.filename)
-          source_hash[source.filename] = source
-          includes_any = true
-        end
+        next unless directory == "*" || config.calculated.include?(source.filename)
+
+        source_hash[source.filename] = source
+        includes_any = true
       end
 
       includes_any

spec/pin/method_spec.rb

@@ -549,6 +549,16 @@ class Foo
       expect(pin.return_type).to be_undefined
     end
 
+    it 'combines signatures by type' do
+      # Integer+ in RBS is a number of signatures that dispatch based
+      # on type.  Let's make sure we combine those with anything else
+      # found (e.g., additions from the BigDecimal RBS collection)
+      # without collapsing signatures
+      api_map = Solargraph::ApiMap.load_with_cache(Dir.pwd, nil)
+      method = api_map.get_method_stack('Integer', '+', scope: :instance).first
+      expect(method.signatures.count).to be > 3
+    end
+
     it 'infers untagged types from instance variables' do
       source = Solargraph::Source.load_string(%(
         class Foo