Conversation
Pin::Callable#return_type declares [ComplexType, nil], and Pin::Method#return_type overrides it with a body Solargraph infers the same way: @return_type ||= return_type_from_inline_rbs || ComplexType.new(...). The left side of that || is declared nilable; the right side never returns nil at runtime. Solargraph does not narrow the || away, so the combined type stays nilable at every call site even though a real return type has always been established by the time these run. Add #return_type!, which raises instead of returning nil, and swap it in at the call sites whose receiver is always a Pin::Callable, across pin/callable.rb, pin/method.rb, pin/signature.rb, source/chain.rb, source/chain/call.rb and type_checker.rb. That clears 9 @sg-ignore markers and resolves 3 strong-mode findings master reports today: Callable#typify's "Unresolved call to defined?"/"to qualify" and Method#dodgy_visibility_source?'s "return type could not be inferred". #return_type! carries one marker of its own, so the net change is 8. Solargraph does not know that raise never returns, so it infers through `x || raise(...)` and reports "return type could not be inferred". castwide#1277 teaches it that raise and abort return RBS's bot, closing castwide#1276; the marker cites that PR and clears on a pin bump once it lands. A guard-clause spelling (`raise ... if type.nil?`) does not dodge the gap - it trades that finding for "Declared return type does not match inferred type ... nil". Sites left on their existing markers, each verified by stripping the marker and re-typechecking: - chain/call.rb's match_overload_type (2 markers): new_signature_pin is declared [Pin::Signature, nil] and is reassigned on the line above. Narrowing a local after definite reassignment is castwide/solargraph castwide#1338, which is still open, so the receiver stays nilable here and return_type! cannot help - the nilability is the receiver's, not the return type's. - chain/call.rb's Call#inferred_pins result.map block (3 markers): the block iterates a mix of Pin::Method and pass-through non-Method pins (e.g. Pin::Parameter, which has its own unrelated #return_type). return_type! typechecks clean there but raises NoMethodError at runtime during Solargraph's own self-hosted typecheck, because duck-typed union call resolution only requires one union member to respond, not all of them. - chain/call.rb's with_params call (1 marker): unrelated to nilability. #self_to_type widens to ComplexType, ComplexType::UniqueType once its receiver stops being plain nil, and with_params only accepts ComplexType. - type_checker.rb's variable_type_tag_problems (1 marker): pin there is a Pin::BaseVariable, whose #return_type is a separate method. Full-workspace `solargraph typecheck --level strong`, core and stdlib uncached first, message text compared with line numbers stripped: 600 problems on 2b9e317, 597 here, nothing introduced. type_checker/rules.rb's ledger is deliberately not touched. Those counts are hand-maintained and already disagree with what the strings grep to, so a marker change is not the place to move them.
#return_type! raises when no return type has been established, but it built that message with #inner_desc, and #inner_desc describes the pin by way of #type_desc, which calls #to_rbs and #return_type.name. Both dereference the return type that is, by construction, nil whenever the raise fires. So the error path could never produce its own error: callers got NoMethodError: undefined method `name` for nil:NilClass, raised from pin/base.rb, instead of a message naming the callable. Identify the callable with #path (falling back to the closure's, since a Pin::Signature has no path of its own) - both are readable without touching the return type. Cover both directions in spec/pin/callable_spec.rb: the resolving case returns the very type the callable was built with, and the raising case asserts the whole message, which is what fails if anything in it starts describing the missing type again.
apiology
force-pushed
the
fix-callable-return-type-nilable
branch
from
September 30, 2026 22:00
d6e6b15 to
8608f39
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude: This PR was written by Claude Code on behalf of @apiology.
Problem:
Pin::Callable#return_typeis declared[ComplexType, nil], andPin::Method#return_typeoverrides it with@return_type ||= return_type_from_inline_rbs || ComplexType.new(...)— the right side of that||is never nil, but Solargraph's inference doesn't narrow that out, so every downstream call site inherits the nilability even after a real return type has always been established.Solution: Adds
Pin::Callable#return_type!(raises instead of returning nil), following the existingclosure!/location!precedent from #110 and #109. Swapped in at 12 of 18 sites verified always-safe, clearing 13 markers. 6 sites left alone: 3 hit a real Solargraph tool defect (a duck-typed union only requires one member to respond to a call, not all — flagged, not fixed here), and 3 are different mechanisms entirely (a different declared type widening, a different class's#return_type, and two unrelated bare errors).Test plan: Full-workspace
solargraph typecheck --level strongmessage-text diff before/after is empty — same 4 pre-existing, unrelated problems both times. Fullrspec: 2205 examples, 0 failures, 45 pending. RuboCop clean.