From 626091015f112916552dc45177fbd5c0c3485c0f Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Tue, 8 Sep 2026 14:36:48 -0400 Subject: [PATCH 1/2] Combine block signatures instead of picking one at call time A gem's plain &block signature and a workspace @!parse stub's yield-typed one only differ in the block's own type_arity, so combine_signatures_by_type_arity bucketed them as separate overloads and left picking between them to Chain::Call at every call site. Merge them into one signature before that bucketing instead, and make Callable#combine_blocks prefer the block that actually declares parameters rather than choosing arbitrarily. The merged pin is now correct on its own - hover and completion benefit too, not just inference at a call that happens to trigger dispatch_order. --- lib/solargraph/pin/callable.rb | 19 ++++++++------- lib/solargraph/pin/method.rb | 43 +++++++++++++++++++++++++++++++++- spec/pin/method_spec.rb | 26 ++++++++++++++++++++ spec/source_map/clip_spec.rb | 42 +++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/lib/solargraph/pin/callable.rb b/lib/solargraph/pin/callable.rb index ed87b79e4..310380756 100644 --- a/lib/solargraph/pin/callable.rb +++ b/lib/solargraph/pin/callable.rb @@ -34,18 +34,21 @@ def method_namespace closure.namespace end + # A block declaring no yielded parameters says nothing about + # what it yields, so it loses to a sibling that documents + # them, 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? || + (block.parameters.empty? && !other.block.nil? && !other.block.parameters.empty?) + return block if other.block.nil? || (other.block.parameters.empty? && !block.parameters.empty?) + 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] diff --git a/lib/solargraph/pin/method.rb b/lib/solargraph/pin/method.rb index c1ae78d0f..f2b25bbd9 100644 --- a/lib/solargraph/pin/method.rb +++ b/lib/solargraph/pin/method.rb @@ -492,9 +492,11 @@ def combine_signatures other # # @return [Array] def combine_signatures_by_type_arity(*signature_pins) + merged_pins = merge_signatures_differing_only_by_block_informativeness(signature_pins) + # @type [Hash{Array => Array}] 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,45 @@ def combine_signatures_by_type_arity(*signature_pins) by_type_arity.values.flatten end + # A block that declares no yielded parameters (e.g. `&block` + # with no @yieldparam) differs in type_arity from one that + # does, 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 informative block onward. + # + # @param signature_pins [Array] + # @return [Array] + def merge_signatures_differing_only_by_block_informativeness signature_pins + # @param sig [Pin::Signature] + # @param result [Array] + 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.empty? == sig2.block.parameters.empty? + sig1.type_arity[0..-2] == sig2.type_arity[0..-2] + end + # @param same_type_arity_signatures [Array] # # @return [Array] diff --git a/spec/pin/method_spec.rb b/spec/pin/method_spec.rb index 403f9c091..4468999ac 100644 --- a/spec/pin/method_spec.rb +++ b/spec/pin/method_spec.rb @@ -128,6 +128,32 @@ 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 '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]) diff --git a/spec/source_map/clip_spec.rb b/spec/source_map/clip_spec.rb index 2d0db48b8..92bb08e1c 100644 --- a/spec/source_map/clip_spec.rb +++ b/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 } From 5e40c804b44d2102111b5543951f12b57ddc120a Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Tue, 8 Sep 2026 15:57:18 -0400 Subject: [PATCH 2/2] Generalize block-informativeness merge past empty vs non-empty The merge only fired when one side's block declared zero yielded parameters. A block documenting one parameter and a sibling documenting two hit the identical type_arity mismatch and still failed to combine. Compare declared parameter counts instead of emptiness, so any side with fewer yielded parameters loses to one with more, not just the zero case. --- lib/solargraph/pin/callable.rb | 10 +++++----- lib/solargraph/pin/method.rb | 17 +++++++++-------- spec/pin/method_spec.rb | 28 ++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/lib/solargraph/pin/callable.rb b/lib/solargraph/pin/callable.rb index 310380756..a881ddd11 100644 --- a/lib/solargraph/pin/callable.rb +++ b/lib/solargraph/pin/callable.rb @@ -34,17 +34,17 @@ def method_namespace closure.namespace end - # A block declaring no yielded parameters says nothing about - # what it yields, so it loses to a sibling that documents - # them, rather than being picked between arbitrarily. + # 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 return other.block if block.nil? || - (block.parameters.empty? && !other.block.nil? && !other.block.parameters.empty?) - return block if other.block.nil? || (other.block.parameters.empty? && !block.parameters.empty?) + (!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] diff --git a/lib/solargraph/pin/method.rb b/lib/solargraph/pin/method.rb index f2b25bbd9..f1b41ab38 100644 --- a/lib/solargraph/pin/method.rb +++ b/lib/solargraph/pin/method.rb @@ -507,13 +507,14 @@ def combine_signatures_by_type_arity(*signature_pins) by_type_arity.values.flatten end - # A block that declares no yielded parameters (e.g. `&block` - # with no @yieldparam) differs in type_arity from one that - # does, 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 informative block onward. + # 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] # @return [Array] @@ -542,7 +543,7 @@ def merge_signatures_differing_only_by_block_informativeness signature_pins # @return [Boolean] def combinable_by_block_informativeness? sig1, sig2 return false if sig1.block.nil? || sig2.block.nil? - return false if sig1.block.parameters.empty? == sig2.block.parameters.empty? + return false if sig1.block.parameters.length == sig2.block.parameters.length sig1.type_arity[0..-2] == sig2.type_arity[0..-2] end diff --git a/spec/pin/method_spec.rb b/spec/pin/method_spec.rb index 4468999ac..6c1906f4e 100644 --- a/spec/pin/method_spec.rb +++ b/spec/pin/method_spec.rb @@ -154,6 +154,34 @@ def build(&block) = 'x' 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])