Skip to content

Move conditional from Region to the CompoundStatement pin itself - #58

Merged
apiology merged 1 commit into
fix-1250-parameter-reassignment-typingfrom
move-conditional-to-pin
Aug 14, 2026
Merged

Move conditional from Region to the CompoundStatement pin itself#58
apiology merged 1 commit into
fix-1250-parameter-reassignment-typingfrom
move-conditional-to-pin

Conversation

@apiology

Copy link
Copy Markdown
Owner

Follow-up to castwide#1282 (via #57), stacked on the same head branch.

Summary

Region#conditional was a separate boolean threaded alongside compound_statement, requiring every node processor to pass both in lockstep (e.g. block_node.rb: compound_statement: block_pin, conditional: true). Keeping two parallel values in sync at every call site is exactly the kind of duplication #57 set out to remove, and it's the shape of bug that broke Block handling mid-refactor there (definite was briefly, incorrectly, derived from compound_statement.is_a?(Closure), which is true for Block despite a block body running zero or many times).

conditional is now a constructor attribute on Pin::CompoundStatement itself, set once where each construct is built (Pin::Block.new(..., conditional: true), Pin::Method.new(...) defaulting false), so there's only one thing to get right per site instead of two.

It can't be a class-level constant: the bare Pin::CompoundStatement class is used both for an if's own condition (never conditional) and for then/else/rhs/rescue bodies (always conditional) - same class, different instances, different answers - so it stays an instance attribute, same as closure:/compound_statement: already are.

lvasgn_node.rb's definite computation becomes a single-hop read: !region.compound_statement.conditional, no separate Region field. Pin::CompoundStatement#combine_with merges the new attribute via choose, since two versions of the same construct should already agree on it.

Test plan

  • bundle exec rspec spec/ - 1638 examples, 0 failures
  • bundle exec solargraph typecheck --level strong - diffed clean against the prior baseline (587 problems, unchanged)
  • bundle exec rubocop - clean on touched files

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

Region#conditional was a separate boolean threaded alongside
compound_statement, requiring every node processor to pass both in
lockstep (e.g. block_node.rb: compound_statement: block_pin,
conditional: true). Keeping two parallel values in sync at every
call site is exactly the kind of duplication this refactor set out
to remove, and it's the shape of bug that broke Block handling
mid-refactor (definite briefly, incorrectly, derived from
compound_statement.is_a?(Closure), which is true for Block despite
a block body running zero or many times).

conditional is now a constructor attribute on Pin::CompoundStatement
itself, set once where each construct is built (Pin::Block.new(...,
conditional: true), Pin::Method.new(...) defaulting false), so
there's only one thing to get right per site instead of two. It
can't be a class-level constant: the bare Pin::CompoundStatement
class is used both for an if's own condition (never conditional)
and for then/else/rhs/rescue bodies (always conditional) - same
class, different instances, different answers - so it stays an
instance attribute, same as closure:/compound_statement: already
are.

lvasgn_node.rb's definite computation becomes a single-hop read:
`!region.compound_statement.conditional`, no separate Region field.
Pin::CompoundStatement#combine_with merges the new attribute via
`choose`, since two versions of the same construct should already
agree on it.

Verified: full suite (1638 examples, 0 failures), typecheck
self-check diffed clean against the prior baseline (587 problems,
unchanged), rubocop clean on touched files.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbhZvdCv7xdziXyKJiPuGk
@apiology
apiology merged commit 9fe7637 into fix-1250-parameter-reassignment-typing Aug 14, 2026
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant