Skip to content

Improved RBS type alias support - #1281

Open
apiology wants to merge 11 commits into
castwide:masterfrom
apiology:fix-1255-rbs-type-alias-expansion
Open

apiology wants to merge 11 commits into
castwide:masterfrom
apiology:fix-1255-rbs-type-alias-expansion

Conversation

@apiology

@apiology apiology commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem: solargraph typecheck --level strong rejects valid arguments when a parameter is typed by an RBS type alias.

# core/builtin.rbs
type path = string | _ToPath

# stdlib/fileutils/0/fileutils.rbs
type pathlist = ::path | Array[::path]
def self?.ln_sf: (pathlist src, ::path dest, ?noop: boolish, ?verbose: boolish) -> void
require 'fileutils'

class Repro
  # @param source [String]
  # @param dest [String]
  # @return [void]
  def link(source, dest)
    # Wrong argument type for FileUtils.ln_sf: src expected
    #   FileUtils::path, Array<FileUtils::path>, received String
    FileUtils.ln_sf(source, dest)
  end
end

String satisfies string, so pathlist accepts it; every stdlib and gem method with an alias-typed parameter is affected.

Solution: type_to_tag expands an RBS::Types::Alias to its underlying type, falling back to the nominal tag for recursive aliases and for generic ones like type box[T] = Array[T] | nil, whose args are not substituted into the expansion.

Alias names resolve against a core-aware copy of the environment, because RbsMap loads each library with core_root: nil and resolve_type_names rebinds a reference it cannot find rather than raising. That copy reparses RBS core, so it is built only for a library whose aliases show the rebinding: one of 41 here.

Test plan: FileUtils.ln_sf('a', 'b') reports 0 problems found against real stdlib RBS; the spec suite's self-contained RBS does not cover that path.

Cost: on a cold cache build of this bundle, not distinguishable from master — 113.3s against 113.7s over four interleaved runs on Ruby 3.2.6 / rbs 4.1.2.

Fixes #1255.

🤖 Generated with Claude Code

RbsTranslator.type_to_tag converted an RBS::Types::Alias to a tag using
only the alias's own name, so a parameter typed via an RBS type alias
(e.g. FileUtils::path = string | _ToPath) never got compared against its
actual union/member types during strict typechecking, only against a
nominal tag matching nothing. solargraph typecheck --level strong
rejected valid calls like FileUtils.ln_sf(a_string, another_string).

Type aliases are now expanded to their underlying type wherever a tag is
built, with cycle detection for recursive aliases and a nominal-tag
fallback for generic aliases (substituting type args into the expansion
is a separate feature). As a result, alias names no longer appear
verbatim in hover, completion, or typecheck messages for aliased types,
e.g. FileUtils.ln_sf's src param now reads as String, FileUtils::_ToPath
instead of FileUtils::path.

sg-ignore comments added throughout RbsTranslator#type_to_tag and its
call sites are pre-existing typecheck debt (Solargraph does not do
flow-sensitive narrowing on case/when over RBS::Types::Bases::Base
subtypes, see castwide#1240) that the repo pre-commit hook
newly flags because this change touches nearly every line of that
method to thread the new type_alias_decls/expanding_aliases parameters
through.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B7nm8YrByTsmvqxyBW8Nvk
apiology added a commit to apiology/solargraph that referenced this pull request Aug 11, 2026
RbsTranslator.type_to_tag converted an RBS::Types::Alias to a tag
using only the alias's own name, so a parameter typed via an RBS type
alias (e.g. FileUtils::path = string | _ToPath) never got compared
against its actual union/member types during strict typechecking -
solargraph typecheck --level strong rejected valid calls like
FileUtils.ln_sf(a_string, another_string). Type aliases are now
expanded to their underlying type wherever a tag is built, with cycle
detection for recursive aliases (falls back to the nominal tag) and a
nominal-tag fallback for generic aliases (type box[T] = Array[T] |
nil), since substituting args into the expansion is a separate
feature and doing it naively would leak an unbound generic<T> tag.

Fixes castwide#1255

Conflicts in lib/solargraph/rbs_translator.rb and
lib/solargraph/rbs_map/conversions.rb: incoming's branch, based
directly on castwide/master, predates this branch's to_complex_type
refactor, which builds compound RBS types (Union, Optional, Tuple,
Intersection, and previously also Alias/ClassInstance/ClassSingleton)
directly as a ComplexType/UniqueType object graph instead of via
type_to_tag's tag-string concatenation - a joined string can't
represent grouping the tag grammar has no syntax for (e.g. a union
nested inside an intersection). Ported the alias-expansion feature
into that architecture instead of reverting to the tag-string
approach: to_complex_type gained a new `when RBS::Types::Alias`
branch (mirroring incoming's expansion/fallback logic) and now
threads type_alias_decls/expanding_aliases through every recursive
call (including through build_unique_type, which needed the same
threading added to its own type_args recursion - missing that caused
a SystemStackError on recursive aliases whose expansion contains
themselves inside a type arg, e.g. `type json = String |
Array[json]`, since the args recursion silently reset the
cycle-detection list). type_to_tag stays leaf-only, as it already was
on this branch; incoming's type_tag/build_type helper methods (needed
only by its tag-string approach) weren't ported, since
build_unique_type already does the same job through the correct
ComplexType-graph recursion.

Also fixed a silent merge corruption in prepend_to_pin/extend_to_pin:
git's 3-way merge combined non-conflicting-but-related hunks from each
side - HEAD had `generic_values = type.all_params.map(&:rooted_tags)`
feeding a `generic_values:` key in the pins.push call, incoming
independently dropped both the assignment and the key on its own
(pre-rooted-names) branch - and the merge silently kept HEAD's
`generic_values:` reference while taking incoming's unassigned
`type.all_params.map(&:rooted_tags)` line, producing an undefined
local variable crash that only the full-project self-typecheck (not
the rspec suite, which never exercises `prepend`/`extend` RBS members)
caught. include_to_pin, identically structured, merged correctly and
was the tell. Restored the assignment in both methods to match.

Verified: spec/rbs_translator_spec.rb, spec/rbs_map/conversions_spec.rb
(27 examples, 0 failures, including all 4 new alias-expansion specs -
basic expansion, declaration-order independence, recursive alias
cycle detection, generic alias fallback), and a broader safety net -
spec/rbs_map, spec/type_checker, spec/source, spec/complex_type_spec.rb,
spec/complex_type (666 examples, 0 failures, 32 pending).
apiology added a commit to apiology/solargraph that referenced this pull request Aug 12, 2026
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).
…expand

RbsMap loads every stdlib/gem library's RBS with core_root: nil to avoid
re-declaring core (already loaded once via RbsMap::CoreMap). But some
stdlib aliases reference a name declared only in core -- e.g.
fileutils.rbs's `type path = ::path` points at core's own `path` alias --
and without core in the environment, RBS::Environment#resolve_type_names
silently rebinds that reference back onto itself instead of raising. The
alias-expansion recursion guard added by 8b97bb1 then (correctly)
detects that self-reference and falls back to a nominal tag, so
FileUtils.mkdir_p/ln_sf etc. still failed strict typecheck against valid
String arguments -- the single-alias case from castwide#1255, not just the
nested pathlist case flagged in review.

Resolve type alias names against a copy of the environment that does
include core, while pin generation still uses the core-less environment
to avoid duplicating core's pins.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D2sPgNckmZHyL49MQLVMRg
apiology added a commit to apiology/solargraph that referenced this pull request Aug 12, 2026
… so cross-namespace aliases expand

RbsMap loads every stdlib/gem library's RBS with core_root: nil to
avoid re-declaring core (already loaded once via RbsMap::CoreMap).
But some stdlib aliases reference a name declared only in core - e.g.
fileutils.rbs's `type path = ::path` points at core's own `path`
alias - and without core in the environment,
RBS::Environment#resolve_type_names silently rebinds that reference
back onto itself instead of raising. The alias-expansion recursion
guard added by the original castwide#1281 merge then (correctly) detects that
self-reference and falls back to a nominal tag, so
FileUtils.mkdir_p/ln_sf etc. still failed strict typecheck against
valid String arguments - the single-alias case from castwide#1255, not just
the nested pathlist case flagged in review.

Resolves type alias names against a copy of the environment that does
include core, while pin generation still uses the core-less
environment to avoid duplicating core's pins.

Clean auto-merge, no conflicts.

Verified: spec/rbs_map/conversions_spec.rb, spec/rbs_translator_spec.rb
(28 examples, 0 failures), and a broader safety net -
spec/type_checker, spec/source, spec/rbs_map,
spec/complex_type_spec.rb, spec/complex_type,
spec/workspace/gemspecs_resolve_require_spec.rb (686 examples, 0
failures, 32 pending).
apiology added a commit to apiology/solargraph that referenced this pull request Aug 12, 2026
…wide#1281 fix

#49 CI (commit a6942fc, merge of latest
castwide#1281 - resolve type alias names against RBS core)
failed Solargraph/strong: 9 "Unneeded @sg-ignore comment" problems,
all wrapping FileUtils.rm_rf/rm_f/mkdir_p calls in Rakefile,
lib/solargraph/pin_cache.rb, and lib/solargraph/shell.rb. Each ignore
was covering "Wrong argument type for FileUtils.*: list expected
FileUtils::path, ..., received String" - exactly the FileUtils::path
alias-resolution gap the just-merged fix closes, so on CI's Ruby 4.0 +
fresh RBS collection environment these calls now typecheck cleanly
without the ignore.

Confirmed genuinely environment-dependent, not a stale local cache:
cleared ~/.cache/solargraph/ruby-3.2.6/rbs-4.1.2/solargraph-* and
reran - 4 of the 9 removed comments (3 in pin_cache.rb, 1 in
shell.rb) are still needed on this local environment (Ruby 3.2.6, RBS
4.1.2). Matches the same Ruby/RBS-version-dependent FileUtils::path
resolution pattern already established for the Vernier-gem and
Gem::StubSpecification gaps during the castwide#1240 merge
- kept the removal as-is to match CI (the actual enforced gate)
rather than restoring for local parity.

Verified: spec/pin_cache_spec.rb, spec/shell_spec.rb (44 examples, 0
failures), and `rubocop lib/solargraph/pin_cache.rb
lib/solargraph/shell.rb Rakefile` (0 offenses).
Comment thread lib/solargraph/rbs_map/conversions.rb Outdated
Comment thread lib/solargraph/rbs_map/conversions.rb
Comment thread lib/solargraph/rbs_map/conversions.rb
Fix the annotation rather than suppressing the error at the call sites.

RbsTranslator.to_parameter_pins was declared to accept only RBS::MethodType,
but is called with RBS::Types::Block at two sites. Its body only reads
method_type.type, which both classes provide, so the declared parameter type
was too narrow. Widening it to [RBS::MethodType, RBS::Types::Block] removes
both @sg-ignore comments this branch had added.

One suppression remains at the block call site, reworded to describe the error
that is actually left: the `if method_type.block` guard does not narrow nil out
of the second `method_type.block` call. That is a flow-sensitive typing gap
rather than an annotation error, tracked as
castwide#1249.

Report the RBS::DuplicatedDeclarationError fallback in core_aware_environment
at warn rather than debug, naming the affected libraries and the consequence,
so a degraded conformance check is visible at the default log level.

Drop a dead `decl.super_class.name.to_s` expression in class_decl_to_pin whose
value was discarded.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CXmnT5gSB1PheL9UbiGEVA
apiology added a commit to apiology/solargraph that referenced this pull request Aug 18, 2026
Two conflicts:

conversions.rb superclass generic_values - kept this branch rooted_tags
form rather than the PR to_s; the rooted form arrived here in a later
merge and reverting it would drop rooted names from superclass reference
pins.

rbs_translator.rb - kept this branch to_restarg_return_type and
to_kwrestarg_return_type helpers, and took the PR widened doc type on
to_parameter_pins (RBS::MethodType, RBS::Types::Block).
apiology added a commit to apiology/solargraph that referenced this pull request Aug 18, 2026
The comment cited castwide#1249: the
`if method_type.block` guard did not narrow nil out of the second
`method_type.block` call. With castwide#1281 merged the guard narrows, and
typecheck reported the comment as unneeded. Removing exactly that comment
and re-checking returns 4 problems, the accepted baseline.
Comment thread lib/solargraph/rbs_map/conversions.rb Outdated
Comment thread lib/solargraph/rbs_map/conversions.rb
to_complex_type and type_to_tag both declared their RBS type
parameter as RBS::Types::Bases::Base, which only covers the Bases::*
variants. Every call site that dispatches on a concrete node
(Optional, Union, Tuple, etc.) needed an @sg-ignore to work around
the mismatch. RBS::Types::t is the gem's own alias for the full node
union and resolves cleanly, clearing 12 @sg-ignore comments across
the two files. Two call sites inside type_to_tag's own case/when
(Literal#literal, Variable#name) needed a new "flow sensitive typing
should support case/when" ignore, matching the existing convention
used throughout the same method, since Solargraph doesn't narrow
types across case/when branches.

Full-project strong typecheck: 494 -> 492 problems, same 88/250
files affected.
decl.type/name/location inside convert_decl_to_pin's TypeAlias
branch were tagged as separate, unrelated limitations, but decl is
still typed as the case discriminant RBS::AST::Declarations::Base
here, the same case/when narrowing gap already tagged on the
sibling branches a few lines up. Confirmed by stripping each ignore:
both report "Unresolved call to <method> on
RBS::AST::Declarations::Base".

The Solargraph::Pin::Reference::TypeAlias.new return_type mismatch
ignore is gone outright - alias_return_type now resolves cleanly
now that to_complex_type has a wider annotation.

Full-project strong typecheck: 492 -> 491 problems.
prepend_to_pin had no coverage: nothing in the core, stdlib or spec RBS
corpus declares a `prepend` member, so the Prepend arm of the member
dispatch never ran. An RBS class that prepends a module reaches it
directly and produces the Pin::Reference::Prepend this asserts.

Full suite: 1631 examples, 0 failures, 60 pending. `rake undercover`
reports no missing coverage.
Nothing calls this method. A whole-tree grep at this commit finds only
its own definition, and the same holds on origin/master, so it was
already unused before this branch touched it. A GitHub code search for
the name turns up no Ruby consumer outside this repo either.

It duplicates build_type in the private `class << self` section, which
is what type_tag - and therefore the alias expansion this branch adds -
actually routes through. Threading type_alias_decls through this copy
as well is what put its map block into undercover's changeset, where it
reported 0 hits on every line.

Full suite: 1631 examples, 0 failures, 60 pending. `rake undercover`
reports no missing coverage.
apiology added a commit to apiology/solargraph that referenced this pull request Sep 5, 2026
Brings castwide#1281 up to 0aa9e65. This branch already had
an older squash of that work, 8b97bb1, so what arrives is the
refinements made since.

Eleven conflicts.

lib/solargraph/rbs_translator.rb, six: took this branch throughout. It
keeps build_unique_type, which that branch removed as uncalled but which
has three live call sites here; keeps to_restarg_return_type and
to_kwrestarg_return_type, which that branch does not have; keeps the
@sg-ignore wording that cites an issue URL over the prose form; and
keeps the keyword-parameter signature of type_to_tag, since every call
site here passes keywords.

lib/solargraph/rbs_map/conversions.rb, three: took this branch.
rooted_tags preserves the :: prefixes that to_s drops, and building the
nil union through the object model avoids re-parsing a tag string.

spec/rbs_map/conversions_spec.rb, two: kept both sides. Git aligned two
independent context blocks as one conflict - implicitly-returns-nil here,
a prepended module there.

That prepend example then failed: it expects the module name 'Bar' where
the combined tree produces '::Bar', because other work here roots those
names. Expectation updated.

.rubocop_todo.yml regenerated against the merged tree.

2184 examples, 0 failures, 45 pending.
@apiology apiology changed the title Expand RBS type aliases before conformance checks Expand RBS type aliases so typecheck sees their real types Sep 5, 2026
RbsMap loads one library per instance, and the core-aware environment
built for it reparses and re-resolves the whole of RBS core every time.
Core is identical on every pass, so all but one of those builds is
wasted: across solargraph's own bundle, only fileutils declares an alias
that a core-less resolution gets wrong.

Gate the build on that symptom. resolve_type_names rebinds a target it
cannot find into the local namespace rather than raising, so a reference
into absent core leaves the alias naming either itself or a name nothing
declares; either one is cheap to spot in the core-less environment we
already hold.

An earlier form of the predicate looked only for a self-reference and
was unsound: "type wrapped_path = ::path" rebinds to Foo::path, not to
wrapped_path, so the spec covering a name declared only in RBS core
failed. Checking for an undeclared target as well covers both shapes.

Measured on this bundle, 41 libraries with RBS, cold cache: 41 core-aware
builds become 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R41wDBRgrL63vb3XGpCdsd
@apiology apiology changed the title Expand RBS type aliases so typecheck sees their real types Improved RBS type alias support Sep 5, 2026
apiology and others added 2 commits September 5, 2026 21:28
Six comment blocks added by this branch ran well over the per-method
budget, the worst a fourteen-line prose block above
core_aware_environment restating what the method and its warn message
already say.

Each is cut to the constraint a cold reader cannot get from the code:
why a non-core RbsMap cannot bind an alias into core, why a generic
alias falls back rather than expanding, and what an unbindable
reference leaves behind. The fileutils walkthrough and the
rescue-branch narration go; the PR description and this history carry
them.

The suppression on the block-parameter call becomes its bare issue
URL, since the issue is the explanation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R41wDBRgrL63vb3XGpCdsd
prepend_to_pin and extend_to_pin build the reference type parameters and
then drop them on the floor; they ought to reach Prepend.new and
Extend.new. Removing the assignment turned that into a bare expression,
which silences Lint/UselessAssignment and with it the only signal that
the bug is there to reproduce and fix.

Passing the values through is out of scope here, so put the assignment
back and let the cop keep reporting it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R41wDBRgrL63vb3XGpCdsd
@apiology
apiology marked this pull request as ready for review September 6, 2026 03:17
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.

Typecheck rejects arguments that satisfy an RBS type alias, because aliases aren't expanded before comparison

1 participant