ensure clause breaks return type inference by apiology · Pull Request #1285 · castwide/solargraph

apiology · 2026-08-11T19:09:51Z

Problem: Any method with an ensure clause fails solargraph typecheck --level strong with return type could not be inferred, whatever the body returns and whatever @return declares.

class Repro
  # @return [String]
  def call
    'hello'
  ensure
    nil
  end
end

$ solargraph typecheck --level strong repro.rb
repro.rb:3: Repro#call return type could not be inferred

An empty ensure is enough to trigger it, so every method using one for cleanup is unusable at strict or strong. Fixes #1284.

Solution: Unwrap an :ensure node to its body's value, scanning the ensure clause separately for explicit returns, since only those reach the caller.

reduce_to_value_nodes also gains the :rescue case it never had, which is what a rescue nested inside an ensure body needs in order to resolve.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XGDWQqVqW2ZdP2rbfr5DDs

@return

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

@return

…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).

@PARAM

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).

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

…gration branch 2026-08-04 # Conflicts: # lib/solargraph/parser/parser_gem/node_methods.rb

…urn-type-inference

@todo

An :ensure node's second child is nil for a body-less ensure clause (e.g. `begin; foo; ensure; end`), but explicit_return_values_from_compound_statement declares its parent argument as Parser::AST::Node. `solargraph typecheck --level strong` flagged this at node_methods.rb:499: Wrong argument type for ...DeepInference.explicit_return_values_from_compound_statement: parent expected Parser::AST::Node, received AST::Node, nil Replace the placeholder `@sg-ignore Need to add nil check here` with a real is_a? guard before the call. That guard alone still leaves the same finding (Solargraph does not narrow `x.is_a?(Node)` inside an `if a && b` condition), which is the pre-existing, cataloged gap this file already suppresses ~10 times in reduce_to_value_nodes with the same text (also tracked in type_checker/rules.rb: "@todo 19: flow sensitive typing needs to narrow down type with an if is_a? check"). Reused that exact marker instead of inventing new wording. Widening explicit_return_values_from_compound_statement's own @PARAM to accept nil was a rejected alternative: it already returns [] for non-Node input, so it would also clear the finding, but it changes a declared contract shared by its other call sites (508, 548, 632) when the review asked for a check at this call site specifically. Added a spec for the empty-ensure-clause case, since none of the existing ensure specs cover a truly empty body (they all use `nil` or `return`, which produce a real child node, not a nil child).

apiology mentioned this pull request Aug 11, 2026

Integration test: 2026-08-04 apiology/solargraph#49

Draft

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 22, 2026

Merge castwide#1285 (fix-1284-ensure-return-type-inference) into inte…

2f031f5

…gration branch 2026-08-04 # Conflicts: # lib/solargraph/parser/parser_gem/node_methods.rb

Merge remote-tracking branch 'origin/master' into fix-1284-ensure-ret…

eeac254

…urn-type-inference

apiology marked this pull request as draft August 31, 2026 20:42

apiology changed the title ~~Fix return type inference for methods with an ensure clause~~ ensure clause breaks return type inference Sep 5, 2026

apiology marked this pull request as ready for review September 5, 2026 15:22

Navigation Menu

Uh oh!

ensure clause breaks return type inference - #1285

ensure clause breaks return type inference#1285
apiology wants to merge 4 commits into
castwide:masterfrom
apiology:fix-1284-ensure-return-type-inference

apiology commented Aug 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

apiology commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

apiology commented Aug 11, 2026 •

edited

Loading