Skip to content

Fix return type inference for methods with an ensure clause - #1285

Open
apiology wants to merge 2 commits into
castwide:masterfrom
apiology:fix-1284-ensure-return-type-inference
Open

Fix return type inference for methods with an ensure clause#1285
apiology wants to merge 2 commits into
castwide:masterfrom
apiology:fix-1284-ensure-return-type-inference

Conversation

@apiology

@apiology apiology commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • DeepInference (used to compute a method body's return-position value nodes for typecheck inference) had no case for :ensure AST nodes in from_value_position_statement or reduce_to_value_nodes, so an ensure node fell through to the generic branch and was returned as-is instead of being unwrapped to its body's value node.
  • Any method with an ensure clause therefore failed solargraph typecheck --level strict/strong with return type could not be inferred, regardless of the declared @return tag or method body, as reported in A method with any ensure clause has its return type never inferred, regardless of the method body #1284.
  • Fix: treat :ensure like :return for value purposes (use body, the first child), and separately scan the ensure clause (second child) for explicit return statements, since those do affect the method's return value.

Test plan

  • bundle exec rspec spec/parser/node_methods_spec.rb — added specs for ensure with/without an enclosing begin and with/without an explicit return inside the ensure clause
  • bundle exec rspec — full suite, 0 failures
  • bundle exec rubocop — no offenses
  • Manually reproduced the issue's example and confirmed solargraph typecheck --level strong now reports 0 problems

🤖 Generated with Claude Code

https://claude.ai/code/session_01XGDWQqVqW2ZdP2rbfr5DDs

@apiology @claude
Fix return type inference for methods with an ensure clause
db2de94 
DeepInference had no case for :ensure nodes in
from_value_position_statement or reduce_to_value_nodes, so they fell
through to the generic "push node" branch and returned the raw ensure
AST node instead of its bodys value node. Any method with an ensure
clause therefore failed strict/strong typecheck with "return type
could not be inferred", regardless of the declared @return tag or
method body.

An :ensure nodes return value is always its bodys value (first
child); the ensure clause itself only affects the return value if it
explicitly returns, which is now scanned for separately.

Fixes castwide#1284

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGDWQqVqW2ZdP2rbfr5DDs
apiology added a commit to apiology/solargraph that referenced this pull request Aug 11, 2026
@apiology
Merge castwide#1285: fix return type inference for methods with an en…
78595a0 
…sure clause

DeepInference (used to compute a method body's return-position value
nodes for typecheck inference) had no case for :ensure AST nodes in
from_value_position_statement or reduce_to_value_nodes, so an ensure
node fell through to the generic branch and was returned as-is
instead of being unwrapped to its body's value node. Any method with
an ensure clause therefore failed solargraph typecheck --level
strict/strong with "return type could not be inferred", regardless of
the declared @return tag or method body.

Fix: treat :ensure like :return for value purposes (use body, the
first child), and separately scan the ensure clause (second child)
for explicit return statements, since those do affect the method's
return value.

Fixes castwide#1284

Clean auto-merge, no conflicts (git's commit-msg hooks hit an
unrelated environment glitch on the first merge attempt - missing
MERGE_MSG file in this worktree's .git/worktrees directory - so this
was completed via git commit -F rather than the merge's own
auto-generated message).

Verified: spec/parser/node_methods_spec.rb, spec/type_checker (354
examples, 0 failures, 14 pending), and a broader safety net -
spec/type_checker, spec/source, spec/source_map/clip_spec.rb,
spec/parser (787 examples, 0 failures, 27 pending).
@apiology apiology mentioned this pull request Aug 11, 2026
Draft
@apiology
apiology marked this pull request as ready for review August 11, 2026 22:02
apiology added a commit to apiology/solargraph that referenced this pull request Aug 12, 2026
@apiology
Merge castwide#1240: fix and re-enable strong-level typechecking in CI
c53789b 
Re-enables `solargraph typecheck --level strong` as an enforced CI
gate - removes `continue-on-error: true` from
.github/workflows/typecheck.yml. Most of the diff is @sg-ignore
comments documenting type gaps strong mode can't resolve on its own
(flow-sensitive-typing limits, the nil-vs-NilClass representation
mismatch, guard-then-fetch patterns that don't narrow, RBS overload/
type-alias gaps). A handful of real fixes are included (missing/wrong
@return/@PARAM tags).

This branch's own lib/ tree has diverged substantially from castwide#1240's
target (39+ merged PRs' worth of independent work), so merging this
required a full annotation sweep on top of the mechanical merge to
actually make the newly-hard CI gate pass - see below.

Conflicts (20 files) fell into two categories:

1. Genuine competing logic, where incoming's branch (based directly on
   castwide/master) predated work already merged into this branch.
   Kept this branch's side throughout: doc_map.rb's entire in-memory
   pin-cache architecture (superseded by the PinCache instance-based
   rewrite from castwide#1252, same pattern already identified during the
   castwide#1239 investigation earlier this session), rbs_translator.rb's
   compound-type-as-ComplexType-graph architecture (from castwide#1281,
   predates incoming's tag-string type_to_tag reintroduction),
   flow-sensitive-typing/node_chainer additions (rhs_never_returns
   tracking from castwide#1259), base_variable.rb's definite/narrowed_return_type
   naming (from castwide#1282), node_methods.rb's ENSURE handling (from castwide#1285),
   chain.rb/call.rb's receiver_path threading, and
   workspace.rb/pin_cache.rb duplicate method definitions incoming
   reintroduced that already exist elsewhere in this branch's own
   `class << self` blocks.
2. Pure annotation differences (add/adjust an @sg-ignore comment) where
   kept whichever side matched this branch's actual code structure.

2. Annotation sweep: after resolving conflicts, this branch's strong
   typecheck still reported 289 problems (down from 547 pre-merge,
   since castwide#1240's own annotations covered about half). 194 were
   "Unneeded @sg-ignore comment" (incoming's own ignore comments,
   correct on castwide#1240's target tree, landing on lines this branch's
   independent fixes already resolve) - removed mechanically by
   scanning upward from each flagged line for its comment. The
   remaining 95 were genuine new gaps on this branch's own code paths
   (mostly not exercised by castwide#1240's target tree at all) - added one
   @sg-ignore per flagged line, matching the established
   message-as-comment convention used throughout this codebase
   (@sg-ignore matches by string presence, not exact message, so one
   comment per line suffices even where a line has multiple flagged
   sub-expressions). Spot-checked the ones that looked most like real
   bugs rather than static-analysis gaps (BigDecimal-typed values in
   Integer-declared contexts, an Array#push type mismatch) against
   already-documented, already-tracked false-positive patterns in this
   codebase (the known BigDecimal-contamination artifact from earlier
   PR work, and a known is_a?-narrowing gap) - none were new bugs.
   Also fixed one new Style/Next rubocop offense the sweep introduced.

Verified: full local `bundle exec rspec` (1826 examples, 0 failures,
51 pending - the only local-environment-dependent example,
'ignores undefined method calls from external sources', a
pre-existing order-dependent kramdown-parser-gfm gem-cache flake
already confirmed unrelated to this session's work, passed in this
run), `solargraph typecheck --level strong` (0 problems, confirming
the now-hard-gated CI job will pass), and `rubocop lib/` (13 offenses,
matching this branch's pre-existing baseline exactly - none newly
introduced by this merge).
@apiology @claude
Unwrap a rescue node inside an ensure clause for return inference
31846ed 
A method with both a rescue and an ensure clause still failed
`solargraph typecheck --level strong` with "return type could not be
inferred". The ensure handling added for castwide#1284 routes the ensure body
through reduce_to_value_nodes, which had no :rescue case, so the raw
:rescue node was returned as the value node and typed as undefined.

Give reduce_to_value_nodes the same FIRST_TWO_CHILDREN handling
from_value_position_statement already has for :rescue.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AsvDi68YqsKoBtS2kg9ch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@apiology