Skip to content

Add EnvironmentLoader for ruby-rbs crate - #3050

Open
dak2 wants to merge 25 commits into
ruby:masterfrom
dak2:environment-loader-ruby-rbs-crate
Open

Add EnvironmentLoader for ruby-rbs crate#3050
dak2 wants to merge 25 commits into
ruby:masterfrom
dak2:environment-loader-ruby-rbs-crate

Conversation

@dak2

@dak2 dak2 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Mirrors RBS::EnvironmentLoader: resolves core, library, and explicit-directory sources in Ruby's load order, then parses each .rbs file once into an Environment.

#2954

Architecture differences from RBS::EnvironmentLoader

  • Ruby's loader both resolves what to load (gems, versions, manifest dependencies) and reads it.
  • The Rust one only reads: it takes resolved directories, so everything needing RubyGems knowledge stays in Ruby, and Rust does the walking, parsing, and interning.
  • That drops the dependency machinery, and makes the name pool per-Environment state instead of a process-global one.
Ruby Rust
Library resolution resolves name/version → directory via gem_sig_path / Repository, and expands manifest.yaml dependencies add_library(name, path) takes an already-resolved directory; no Repository, no manifest expansion, no implicit stringio
Registration API one add(path:/library:/version:) with keyword modes add_library / add_dir, the modes separated by type
Duplicate registration libs is a Set, to stop dependency expansion from recursing plain Vec; with no expansion, first-wins on seen_files is enough
Name pool process-global symbols and TypeName objects Environment owns the Interners, so loaded names stay resolvable for its lifetime
File walk Pathname.glob("**/*.rbs"), sorted as /-joined strings recursive read_dir, sorted by platform path; symlinked directories are not followed, and only PermissionDenied / NotFound are skipped

Usage

Register the directories on the loader, then load them into an Environment:

let loader = EnvironmentLoader::new(Some(core_root))
    .add_library("pathname", pathname_sig_dir) // resolved on the Ruby side
    .add_dir(PathBuf::from("sig"));

let env = Environment::from_loader(&loader)?;

for source in env.sources() {
    // source.path, source.kind (Core / Library { name, path } / Dir { path }),
    // source.directives, source.declarations
}

load can also fill an existing Environment, and returns what it read, in the
order it read it:

let mut env = Environment::new();
let loaded: Vec<LoadedFile> = loader.load(&mut env)?; // { path, kind } per file

Notes

  • Directories are visited core → libraries → dirs, and a path already read is skipped, so the first source registering a file wins.
  • LoadError is Io { path, .. } or Parse { path, message }; the sources read before the failure are already in the Environment.
  • The Environment owns the interners, so env.interners() is what resolves and displays the interned names held by the loaded declarations.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

Mirrors RBS::EnvironmentLoader: resolves core, library (with
manifest.yaml dependency expansion), and explicit-directory
sources in Ruby's load order, then parses each .rbs file once
into an Environment.

ruby#2954
Comment thread rust/ruby-rbs/src/loader/mod.rs Outdated
Comment thread rust/ruby-rbs/src/environment/source.rs Outdated
Comment thread rust/ruby-rbs/src/loader/gem_sig_resolver.rs Outdated
dak2 and others added 18 commits August 11, 2026 17:06
The library client passes resolved gem paths directly; the resolver
indirection was unnecessary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Optional builder setters let core and stdlib be skipped by silent
omission instead of explicit choice.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
version alone can't recover where a library's signatures came from; path can.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes rejection of YAML that Ruby's YAML.safe_load accepts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Without this, the natural default configuration (core_root + stdlib_root,
no extra wiring) always failed with UnknownLibrary("stringio"), since the
loading repository stayed empty unlike Ruby's auto-registered stdlib.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
An invalid version string was indistinguishable from a missing gem at the
lookup boundary, so every caller had to guard against it separately. Parsing
once in the loader and carrying the result alongside its Library makes that
mismatch unrepresentable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
collect() propagated every io::Error, so one EACCES subdirectory
failed the entire load. Ruby's Dir.glob silently skips such
directories, so only skip PermissionDenied and NotFound to match it,
while still surfacing genuine I/O failures.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
line_ranges, line_count, line, pos_to_loc, loc_to_pos, and
last_position had no callers outside their own tests, and their
byte-offset semantics diverge from Ruby's character-offset RBS::Buffer
anyway.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merging upstream/master reintroduced this arm earlier in the match
(added independently on master), leaving the branch's copy at the end
unreachable. With RUSTFLAGS=-D warnings this failed cargo test and
cargo clippy in CI.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Steep-style callers need to add parsed sources to an Environment
without going through EnvironmentLoader's directory scan. Parsing
always uses the environment's own Interners, so a source added this
way can never carry ids from a different environment. load() now
calls add_rbs_file too, so parsing lives in one place.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ruby now resolves gem name/version to a directory before calling
Rust, so add_library just takes the path. Drop the loader's own
resolution path with it: dependency expansion, the stdlib/loading
repositories, and the version-related LoadError variants.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Unused now that add_library takes an already-resolved path instead
of a name/version pair. Recoverable from git history if Rust ever
needs its own version comparison again.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ruby's RBS::Buffer exists because RBS::Location is lazy: it holds a
reference to the buffer and slices out text on demand for #source and
pos_to_loc. The Rust port mirrored that shape, but the architecture
underneath it isn't lazy — AstConverter eagerly interns or copies
every semantically relevant string (names, comments, annotations,
literals) at conversion time, and LocationRange is a plain 4x u32
offset Copy type with no reference back into the source. There is no
deferred slicing step left that would ever need the buffer, so
nothing in the crate calls Buffer::content() after parse_one()
returns — it's a write-only store.

Keeping it anyway means holding the full text of every loaded file
for the Environment's lifetime for no benefit: ~4.3MB across core +
stdlib alone, more with gems, growing for as long as the Environment
lives.

Source.buffer: Buffer is replaced with Source.path: PathBuf, keeping
just enough to identify the file for error reporting. parse_one() no
longer moves `content` into a Buffer, so the explicit drop(signature)
that used to be needed to end its borrow first is gone too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dak2
dak2 force-pushed the environment-loader-ruby-rbs-crate branch from 3ffad9d to eddc09a Compare August 14, 2026 00:28
dak2 and others added 6 commits August 14, 2026 10:10
The "skip this entry" match was written out three times, differing only
in whether it returned or continued. Collapse it into one `skippable`
adapter returning `io::Result<Option<T>>`, so the policy for which IO
errors Ruby swallows lives in one place.

The entry name was also derived twice per entry: from
`entry.file_name()` for the leading-`.` test, then again from
`path.file_name()` for the leading-`_` test. Derive it once from
`path`, which drops the `OsString` that `DirEntry::file_name` allocates
for every entry walked, matched or not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
each_dir built a Vec of owned pairs before the load loop started,
cloning every registered path twice: once into the SourceKind and once
as the directory to walk. The kinds still have to be built, but the
directory to walk can be borrowed from the loader, which outlives the
load call.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing calls it. It forwards to StringInterner::merge and
TypeNameInterner::merge, which are already covered by tests next to
their own definitions, so its test asserted nothing new either.

The parallel load loop parse_one is shaped for would need it back, but
re-adding a two-line delegation then is cheaper than carrying an
untested-by-use API until that lands.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Default delegated to a new() that listed the fields by hand, so adding
a field meant remembering to update new(). Reverse the delegation and
derive Default, leaving the generated code as the only place that
knows the fields.

This is also the shape StringInterner and Interners already use.
TypeNameInterner keeps its hand-written Default because it has real
initialisation to do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bool it returns is only accepted by file_finder::each_file, which
is pub(crate), so a caller outside the crate could obtain the answer
but had nowhere to pass it. The variants themselves stay public, so no
information is withdrawn from consumers of Source.kind.

Also record on Library why it carries the resolved path, since dropping
version for path was a deliberate choice and the field reads like
duplication next to each_dir without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing pinned down the return value: every assertion against it was
answerable from env.sources(), and one of them compared the two lengths
against each other. Add load_reports_only_what_this_call_added, which
loads twice into one Environment and checks the second call reports
only its own file — the case env.sources() cannot answer, and the
reason the return value is not just a projection of the environment.

Say as much on load itself, and note on LoadedFile that it corresponds
to an entry of what RBS::EnvironmentLoader#load returns, at file rather
than declaration granularity.

The remaining tests reuse the tree helper this file already had instead
of hand-rolling tempdir plus write, and from_loader_is_the_primary_entry_point
no longer reads stdlib to assert that something was loaded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dak2

dak2 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Remove Buffer. In details, 2b3df5a

@dak2
dak2 marked this pull request as ready for review August 14, 2026 01:37
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.

2 participants