diff --git a/lib/solargraph/pin/callable.rb b/lib/solargraph/pin/callable.rb index ed87b79e4..a881ddd11 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 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] diff --git a/lib/solargraph/pin/method.rb b/lib/solargraph/pin/method.rb index c1ae78d0f..f1b41ab38 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,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] + # @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.length == sig2.block.parameters.length + 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..6c1906f4e 100644 --- a/spec/pin/method_spec.rb +++ b/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]) 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 }