Stop losing flow-typing facts, and stop flagging unreachable code as errors - #56
Draft
apiology wants to merge 9 commits into
Draft
Stop losing flow-typing facts, and stop flagging unreachable code as errors#56apiology wants to merge 9 commits into
apiology wants to merge 9 commits into
Conversation
Pin::Base#== compares #location but not #presence. combine_with results choose the earliest assignment's #location, so two combined pins covering a different number of assignments to the same variable can share #location while covering different #presence ranges - e.g. one pin combined through a variable's first reassignment, another combined through its second. Any caller keying off of #== (e.g. Array#include?) treated these as the same pin. BaseVariable#== now also compares presence, intersection_return_type, and exclude_return_type. Adds two regression specs: - spec/pin/base_variable_spec.rb: directly exercises the equality gap above - fails without the fix, passes with it. - spec/type_checker/levels/strong_spec.rb: a 13-line repro (castwide#1288 (comment)) where this equality gap, combined with in-flight flow-sensitive-typing work (castwide#1258, castwide#1282), produces a false "Unresolved call" via Chain's inference recursion guard. Not currently reachable on master alone (verified neither castwide#1258 nor castwide#1282 reproduces it in isolation, only the two combined) - kept as a standing guard so whatever future combination reintroduces the failure mode gets caught regardless of merge order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YT7qJXsRVt8W7ULFmwjvLj
BaseVariable#== now compares presence (97127a9), so pins produced by independent flow-sensitive-typing facts at the same location (e.g. the two operands of x.nil? || x.is_a?(Foo)) no longer get deduplicated via Array#include? before being combined. Combining them unions their exclude_return_type sets, which can end up excluding every member of the declared type - collapsing ComplexType#exclude's result to undefined even though the type was well-defined before exclusion. That surfaced as new self-hosted strong-typecheck failures on lib/solargraph/pin/base.rb ("Unresolved call to inspect") reported at castwide#1293 (comment), introduced by 97127a9 without touching that file at all. ComplexType#exclude now treats an exclusion built from more than one excluded type as a no-op when it would otherwise remove every possible type, since that combination reflects contradictory flow facts (unreachable/defensive code) rather than a real type error. A single-source exhaustive exclusion (e.g. a variable directly assigned nil despite a non-nilable declared type) still collapses to undefined as before. Verified against the parent commit (8fda633): whole-project solargraph typecheck --level strong problem count drops from 533 to 531, an exact match after removing the two new false positives with no other diff. Full test suite: 1626 examples, 0 failures.
@todo comment only - explains that once castwide#1277 (RBS bottom type) lands, ComplexType#exclude could tag `bot` for any exhausted exclusion instead of just the multi-source case handled here, making this branch's special-casing unnecessary. The two PRs are otherwise independent: this touches only #exclude, castwide#1277 touches #qualify and elsewhere in complex_type.rb, so they merge cleanly in either order with no coordination needed.
ComplexType#exclude previously only collapsed to `undefined` for single-item exclusions (e.g. `!x.is_a?(Foo)` narrowing a declared Foo to nothing), leaving the multi-item case as a same-type no-op. Now that castwide#1277 gives RBS's bottom type its own tag, both cases can collapse to `bot` instead - it correctly signals "this code is unreachable" rather than "this type is unknown." That surfaces a second gap: Call#resolve had no path for a bot-typed receiver, so any method call chained onto one came back "Unresolved call to <method> on bot" - a false positive, since bot is a subtype of everything and the call is unreachable code anyway. Adds a bot? branch that returns a DuckMethod pin (return type bot) so downstream resolution has a real Pin::Method to work with while bot keeps propagating. Fail-first verified: reverting just the Call#resolve change while keeping the exclude change reproduces "Unresolved call to length on bot"; with both, the new strict-level spec passes.
The integration-branch merge of this commit surfaced (via Solargraph/strong, which runs with SOLARGRAPH_ASSERTS=on) "Closure not set on Solargraph::Pin::DuckMethod ... from :chain": the bot? DuckMethod pin never passed closure:, which is harmless until something downstream reads Pin::Base#closure, which asserts under strict mode when unset. Passes name_pin.closure through, matching the fix already applied to the integration branch's equivalent method_stack_pins code path.
apiology
changed the title
… fix-pin-equality-missing-presence
3 tasks
apiology
added a commit
that referenced
this pull request
Aug 18, 2026
Clean auto-merge. Its branch already carried castwide#1277, merged here first, so nothing of castwide#1277 arrives twice.
The four `@sg-ignore Should add type check on other` comments in `BaseVariable#==` existed because `other` is declared `Object`, which has no `assignment`, `presence`, `intersection_return_type`, or `exclude_return_type`. With solargraph-rails loaded, that plugin declares ActiveSupport's `Object#presence` as `() -> self?`. That unrelated same-named method satisfied `other.presence`, so the suppression on that line stopped suppressing anything and CI's plugin-loaded jobs reported it as `Unneeded @sg-ignore comment`, while the other three still suppressed real findings without the plugin. No single set of suppressions could satisfy both configurations while `other` stayed `Object`. The guard is the fix the suppression text named. It is behaviorally equivalent to what was there: `super` is `Pin::Base#==`, which starts with `nearly?`, which starts with `instance_of?(other.class)`, so a non-`BaseVariable` `other` already returned false. Verified with `solargraph typecheck --level strong` both with and without solargraph-rails configured: base_variable.rb reports zero findings from this method in both, where the plugin configuration previously reported one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015AsvDi68YqsKoBtS2kg9ch
apiology
changed the title
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.
Two flow-typing facts about the same variable could be silently collapsed into one, losing the second.
Pin::Base#==compares#locationbut not#presence.combine_withgives its result the earliest assignment's location, so two pins covering different numbers of assignments share a location while covering different presence ranges — and every#==caller,Array#include?among them, treated them as the same pin.BaseVariable#==now also comparespresence,narrowed_return_typeandexclude_return_type.Keeping both facts lets their
exclude_return_typesets union until every member of the declared type is excluded — a guard likex.nil? || x.is_a?(Foo)covering everything. That means the code past it is unreachable, not mistyped, so it is now taggedbot(from castwide#1277) rather thanundefined, and a call on a bot-typed receiver resolves to aDuckMethodpin returningbot— vacuously valid, and bot keeps propagating — instead of being flagged unresolved.#==also opens withreturn false unless other.is_a?(BaseVariable).superalready returned false for other classes viaPin::Base#nearly?'sinstance_of?; stating it explicitly removes four@sg-ignoresuppressions.Test plan
rspec— 2 failures, bothspec/shell_spec.rb"unbundled environments", present on castwide master8fda63384.rubocop— count unchanged,.rubocop_todo.ymluntouched.run_solargraph_rspec_specsfails identically before this branch.This PR was written by Claude Code on behalf of @apiology.