Skip to content

Commit 0cb1a0e

Browse files
committed
Merge castwide#1274: fix @Generic return type lost with block param
Adds Pin::Callable#block_required? (default false), true only for RBS-sourced signatures with a non-optional block (`{ ... }`, not `?{ ... }`). Pin::Callable#arity_matches? previously rejected a call missing a block whenever the signature declared *any* block, even an optional one - so a bare `&block`/@yield-tag signature with a @Generic return type would get skipped by overload resolution for a callsite with no block, falling through to a less specific overload and losing the generic. Now it only rejects when block_required? is true. Conflict in lib/solargraph/rbs_map/conversions.rb: HEAD didn't know about the new block_required: keyword yet; took the incoming side, which wires overload.method_type.block&.required into the new Pin::Signature parameter (mirroring the equivalent, non-conflicting change already applied to rbs_translator.rb#to_signature). Verified: spec/source/chain/call_spec.rb, spec/rbs_map/conversions_spec.rb, spec/rbs_translator_spec.rb (59 examples, 0 failures, 3 pending), and a broader safety net - spec/type_checker, spec/source, spec/source_map/clip_spec.rb, spec/pin (792 examples, 0 failures, 25 pending) - all passing locally.
2 parents 10c5a55 + 922d073 commit 0cb1a0e

4 files changed

Lines changed: 42 additions & 5 deletions

File tree

lib/solargraph/pin/callable.rb

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,17 @@ class Callable < Closure
1414
# @param block [Signature, nil]
1515
# @param return_type [ComplexType, nil]
1616
# @param parameters [::Array<Pin::Parameter>]
17+
# @param block_required [Boolean] Whether callers must pass a block for
18+
# this signature to apply. Only ever true for RBS-sourced signatures
19+
# with a non-optional block (`{ ... }` rather than `?{ ... }`); a bare
20+
# `&block` parameter or YARD @yield tag never makes a block mandatory.
1721
# @param [Hash{Symbol => Object}] splat
18-
def initialize block: nil, return_type: nil, parameters: [], **splat
22+
def initialize block: nil, return_type: nil, parameters: [], block_required: false, **splat
1923
super(**splat)
2024
@block = block
2125
@return_type = return_type
2226
@parameters = parameters
27+
@block_required = block_required
2328
end
2429

2530
def reset_generated!
@@ -55,6 +60,7 @@ def combine_blocks other
5560
def combine_with other, attrs = {}
5661
new_attrs = {
5762
block: combine_blocks(other),
63+
block_required: block_required? || other.block_required?,
5864
return_type: combine_return_type(other)
5965
}.merge(attrs)
6066
new_attrs[:parameters] = choose_parameters(other).clone.freeze unless new_attrs.key?(:parameters)
@@ -241,7 +247,7 @@ def arity_matches? arguments, with_block
241247
argcount = arguments.length
242248
parcount = mandatory_positional_param_count
243249
parcount -= 1 if !parameters.empty? && parameters.last.block?
244-
return false if block? && !with_block
250+
return false if block? && block_required? && !with_block
245251
# @todo this and its caller should be changed so that this can
246252
# look at the kwargs provided and check names against what
247253
# we acccept
@@ -267,6 +273,11 @@ def block?
267273
!!@block
268274
end
269275

276+
# @return [Boolean]
277+
def block_required?
278+
!!@block_required
279+
end
280+
270281
protected
271282

272283
attr_writer :block

lib/solargraph/rbs_map/conversions.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,8 @@ def method_def_to_sigs decl, pin
549549
Pin::Signature.new(generics: generics, parameters: block_parameters, return_type: block_return_type, source: :rbs,
550550
type_location: type_location, closure: pin)
551551
end
552-
Pin::Signature.new(generics: generics, parameters: signature_parameters,
553-
return_type: signature_return_type, block: block, source: :rbs,
552+
Pin::Signature.new(generics: generics, parameters: signature_parameters, return_type: signature_return_type, block: block,
553+
block_required: overload.method_type.block&.required || false, source: :rbs,
554554
type_location: type_location, closure: pin)
555555
end
556556
end

lib/solargraph/rbs_translator.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ def self.to_signature method_type, closure, parameter_names = []
153153
block_return_type = to_complex_type(method_type.block.type.return_type)
154154
Pin::Signature.new(generics: generics, parameters: block_parameters, return_type: block_return_type, source: :rbs, type_location: closure.location, closure: closure)
155155
end
156-
Pin::Signature.new(generics: generics, parameters: parameters, return_type: return_type, block: block, source: :rbs, type_location: closure.location, closure: closure)
156+
Pin::Signature.new(generics: generics, parameters: parameters, return_type: return_type, block: block,
157+
block_required: method_type.block&.required || false, source: :rbs, type_location: closure.location, closure: closure)
157158
end
158159

159160
# @param type_name [RBS::TypeName]

spec/source/chain/call_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,31 @@ def baz
319319
expect(type.tag).to eq('String')
320320
end
321321

322+
it 'infers generic return types from @generic tag when method also takes an unused block param' do
323+
source = Solargraph::Source.load_string(%(
324+
class Foo
325+
def initialize; end
326+
def foo_method; 1; end
327+
end
328+
329+
class Repro
330+
# @generic T
331+
# @param clazz [Class<generic<T>>]
332+
# @return [generic<T>]
333+
def create_object(clazz, &unused)
334+
clazz.new
335+
end
336+
end
337+
338+
Repro.new.create_object(Foo)
339+
), 'test.rb')
340+
api_map = Solargraph::ApiMap.new
341+
api_map.map source
342+
chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(15, 20))
343+
type = chain.infer(api_map, Solargraph::Pin::ROOT_PIN, api_map.source_map('test.rb').locals)
344+
expect(type.tag).to eq('Foo')
345+
end
346+
322347
it 'infers generic return types from block from yield being a return node' do
323348
pending('deeper inference support')
324349

0 commit comments

Comments
 (0)