Keep block parameter types documented by @!parse stubs

lib/solargraph/pin/callable.rb

@@ -34,18 +34,21 @@ def method_namespace
         closure.namespace
       end
 
+      # A block documented with fewer yielded parameters says less
+      # about what it yields than a sibling with more, so it loses to
+      # that sibling rather than being picked between arbitrarily.
+      #
       # @param other [self]
       #
       # @return [Pin::Signature, nil]
       def combine_blocks other
-        if block.nil?
-          other.block
-        elsif other.block.nil?
-          block
-        else
-          # @type [Pin::Signature, nil]
-          choose_pin_attr(other, :block)
-        end
+        return other.block if block.nil? ||
+                              (!other.block.nil? && block.parameters.length < other.block.parameters.length)
+        return block if other.block.nil? || (block.parameters.length > other.block.parameters.length)
+        return block.combine_with(other.block) if block.arity == other.block.arity
+
+        # @type [Pin::Signature, nil]
+        choose_pin_attr(other, :block)
       end
 
       # @param other [self]

lib/solargraph/pin/method.rb

@@ -492,9 +492,11 @@ def combine_signatures other
       #
       # @return [Array<Pin::Signature>]
       def combine_signatures_by_type_arity(*signature_pins)
+        merged_pins = merge_signatures_differing_only_by_block_informativeness(signature_pins)
+
         # @type [Hash{Array => Array<Pin::Signature>}]
         by_type_arity = {}
-        signature_pins.each do |signature_pin|
+        merged_pins.each do |signature_pin|
           by_type_arity[signature_pin.type_arity] ||= []
           by_type_arity[signature_pin.type_arity] << signature_pin
         end
@@ -505,6 +507,46 @@ def combine_signatures_by_type_arity(*signature_pins)
         by_type_arity.values.flatten
       end
 
+      # A block documented with fewer yielded parameters than a
+      # sibling signature's block (including zero, e.g. `&block` with
+      # no @yieldparam at all) differs in type_arity from it, so
+      # combine_signatures_by_type_arity's bucketing would otherwise
+      # keep them as separate overloads. They're the same overload,
+      # just documented with different completeness - merge them
+      # here, before that bucketing, so the merged signature carries
+      # the more complete block onward.
+      #
+      # @param signature_pins [Array<Pin::Signature>]
+      # @return [Array<Pin::Signature>]
+      def merge_signatures_differing_only_by_block_informativeness signature_pins
+        # @param sig [Pin::Signature]
+        # @param result [Array<Pin::Signature>]
+        signature_pins.each_with_object([]) do |sig, result|
+          # @type [Integer, nil]
+          match_index = result.find_index { |existing| combinable_by_block_informativeness?(existing, sig) }
+          if match_index.nil?
+            result << sig
+          else
+            existing = result.fetch(match_index)
+            # Bypass Callable#combine_with's default parameter zip: it
+            # compares the signatures' full arity, which folds in the
+            # block's own arity and would raise here even though the
+            # blockless parameters already match (that's what
+            # combinable_by_block_informativeness? just verified).
+            result[match_index] = existing.combine_with(sig, parameters: existing.parameters)
+          end
+        end
+      end
+
+      # @param sig1 [Pin::Signature]
+      # @param sig2 [Pin::Signature]
+      # @return [Boolean]
+      def combinable_by_block_informativeness? sig1, sig2
+        return false if sig1.block.nil? || sig2.block.nil?
+        return false if sig1.block.parameters.length == sig2.block.parameters.length
+        sig1.type_arity[0..-2] == sig2.type_arity[0..-2]
+      end
+
       # @param same_type_arity_signatures [Array<Pin::Signature>]
       #
       # @return [Array<Pin::Signature>]

spec/pin/method_spec.rb

@@ -128,6 +128,60 @@ def bazzle; end
     expect(result.length).to eq(signatures.length)
   end
 
+  it 'merges a block signature that declares parameters with one that does not' do
+    plain_impl = Solargraph::SourceMap.load_string(%(
+      module Widgetbox
+        class << self
+          # @return [String]
+          def build(&block) = 'x'
+        end
+      end
+    ), 'widgetbox.rb')
+    parse_stub = Solargraph::SourceMap.load_string(%(
+      # @!parse
+      #   module Widgetbox
+      #     class << self
+      #       # @yieldparam config [String]
+      #       # @return [String]
+      #       def build(&block); end
+      #     end
+      #   end
+    ), 'annotations.rb')
+    api_map = Solargraph::ApiMap.new
+    api_map.catalog Solargraph::Bench.new(source_maps: [plain_impl, parse_stub])
+    method = api_map.get_method_stack('Widgetbox', 'build', scope: :class).first
+    expect(method.signatures.length).to eq(1)
+    expect(method.signatures.first.block.parameters.map(&:name)).to eq(['config'])
+  end
+
+  it 'merges a block signature with more declared parameters than one with fewer' do
+    fewer_params = Solargraph::SourceMap.load_string(%(
+      module Widgetbox
+        class << self
+          # @yieldparam a [String]
+          # @return [String]
+          def build(&block) = 'x'
+        end
+      end
+    ), 'widgetbox.rb')
+    more_params = Solargraph::SourceMap.load_string(%(
+      # @!parse
+      #   module Widgetbox
+      #     class << self
+      #       # @yieldparam a [String]
+      #       # @yieldparam b [Integer]
+      #       # @return [String]
+      #       def build(&block); end
+      #     end
+      #   end
+    ), 'annotations.rb')
+    api_map = Solargraph::ApiMap.new
+    api_map.catalog Solargraph::Bench.new(source_maps: [fewer_params, more_params])
+    method = api_map.get_method_stack('Widgetbox', 'build', scope: :class).first
+    expect(method.signatures.length).to eq(1)
+    expect(method.signatures.first.block.parameters.map(&:name)).to eq(%w[a b])
+  end
+
   it 'does not merge with changes in parameters' do
     # @todo Method pin parameters are pins now
     pin1 = described_class.new(name: 'bar', parameters: %w[one two])

spec/source_map/clip_spec.rb

@@ -1964,6 +1964,48 @@ def resource_subtype; end
     expect(type.tag).to eq('String')
   end
 
+  it 'yields the block parameter type declared by a cross-file @!parse stub' do
+    # The plain implementation only documents that it takes a block (no
+    # @yieldparam), simulating a gem's pins loading before a workspace
+    # @!parse stub that adds the block's parameter type.
+    plain_impl = Solargraph::SourceMap.load_string(%(
+      module Widgetbox
+        class Collection
+          # @return [String]
+          def name
+            'collection'
+          end
+        end
+
+        class << self
+          # @return [Widgetbox::Collection]
+          def build(&block)
+            Collection.new
+          end
+        end
+      end
+    ), 'widgetbox.rb')
+    parse_stub = Solargraph::SourceMap.load_string(%(
+      # @!parse
+      #   module Widgetbox
+      #     class << self
+      #       # @yieldparam config [Widgetbox::Collection]
+      #       # @return [Widgetbox::Collection]
+      #       def build(&block); end
+      #     end
+      #   end
+    ), 'annotations.rb')
+    caller_source = Solargraph::Source.load_string(%(
+      Widgetbox.build do |config|
+        config
+      end
+    ), 'test.rb')
+    api_map = Solargraph::ApiMap.new
+    api_map.catalog Solargraph::Bench.new(source_maps: [plain_impl, parse_stub, Solargraph::SourceMap.map(caller_source)])
+    clip = api_map.clip_at('test.rb', [2, 14])
+    expect(clip.infer.tag).to eq('Widgetbox::Collection')
+  end
+
   it 'uses simple return value of block to infer return value of Enumerable#map' do
     source = Solargraph::Source.load_string(%(
       a = ['a'].map { 123 }