Skip to content

Commit e5c8bbe

Browse files
dak2claude
andcommitted
Register stdlib_root into EnvironmentLoader's loading repository
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>
1 parent 70b438c commit e5c8bbe

2 files changed

Lines changed: 57 additions & 28 deletions

File tree

rust/ruby-rbs/src/loader/mod.rs

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -103,33 +103,31 @@ pub struct LoadedFile {
103103
pub struct EnvironmentLoader {
104104
core_root: Option<PathBuf>,
105105
stdlib_root: Option<PathBuf>,
106-
repository: Repository,
106+
repository_dirs: Vec<PathBuf>,
107107
libs: Vec<Library>,
108108
dirs: Vec<PathBuf>,
109109
}
110110

111111
impl EnvironmentLoader {
112112
/// `core_root` is `None` to skip core; `stdlib_root` is the stdlib
113-
/// signature directory used for dependency expansion, mirroring
114-
/// `Collection::Sources::Stdlib` (which Ruby pins to its bundled
115-
/// `stdlib/`) — `None` disables manifest-based dependency expansion.
113+
/// signature directory used for dependency expansion and for resolving
114+
/// requested libraries — `None` disables both.
116115
/// Both are required arguments, not builder defaults, so callers decide
117116
/// them explicitly instead of silently getting `None`.
118-
///
119-
/// `stdlib_root` is independent of [`EnvironmentLoader::repository`]:
120-
/// add the same directory there to make the expanded libraries loadable.
121117
pub fn new(core_root: Option<PathBuf>, stdlib_root: Option<PathBuf>) -> Self {
122118
EnvironmentLoader {
123119
core_root,
124120
stdlib_root,
125-
repository: Repository::new(),
121+
repository_dirs: Vec::new(),
126122
libs: Vec::new(),
127123
dirs: Vec::new(),
128124
}
129125
}
130126

131-
pub fn repository(mut self, repository: Repository) -> Self {
132-
self.repository = repository;
127+
/// Adds a repository root for resolving libraries, searched after
128+
/// `stdlib_root`; a later root wins for the same gem and version.
129+
pub fn add_repository_dir(mut self, path: PathBuf) -> Self {
130+
self.repository_dirs.push(path);
133131
self
134132
}
135133

@@ -157,11 +155,12 @@ impl EnvironmentLoader {
157155
/// as the Ruby implementation adding sources as it walks the directories.
158156
pub fn load(&self, env: &mut Environment) -> Result<Vec<LoadedFile>, LoadError> {
159157
let stdlib = self.stdlib_repository()?;
158+
let repository = self.loading_repository()?;
160159

161160
let mut loaded = Vec::new();
162161
let mut seen_files: HashSet<PathBuf> = HashSet::new();
163162

164-
for (kind, dir, skip_hidden) in self.each_dir(&stdlib)? {
163+
for (kind, dir, skip_hidden) in self.each_dir(&stdlib, &repository)? {
165164
let files =
166165
file_finder::each_file(&dir, skip_hidden).map_err(|source| LoadError::Io {
167166
path: dir.clone(),
@@ -189,17 +188,20 @@ impl EnvironmentLoader {
189188

190189
/// Resolves directories in the Ruby implementation's order:
191190
/// core, then libraries (with dependencies), then explicit directories.
192-
fn each_dir(&self, stdlib: &Repository) -> Result<Vec<(SourceKind, PathBuf, bool)>, LoadError> {
191+
fn each_dir(
192+
&self,
193+
stdlib: &Repository,
194+
repository: &Repository,
195+
) -> Result<Vec<(SourceKind, PathBuf, bool)>, LoadError> {
193196
let mut result = Vec::new();
194197

195198
if let Some(core) = &self.core_root {
196199
result.push((SourceKind::Core, core.clone(), true));
197200
}
198201

199202
for library in self.resolved_libraries(stdlib)? {
200-
let dir = self
201-
.library_dir(&library)
202-
.ok_or_else(|| LoadError::UnknownLibrary {
203+
let dir =
204+
library_dir(repository, &library).ok_or_else(|| LoadError::UnknownLibrary {
203205
name: library.name.clone(),
204206
version: library.version.clone(),
205207
})?;
@@ -231,6 +233,25 @@ impl EnvironmentLoader {
231233
Ok(repository)
232234
}
233235

236+
/// `stdlib_root` is registered first, standing in for Ruby's
237+
/// `Repository.new` auto-registering `DEFAULT_STDLIB_ROOT`.
238+
fn loading_repository(&self) -> Result<Repository, LoadError> {
239+
let mut repository = Repository::new();
240+
if let Some(root) = &self.stdlib_root {
241+
repository.add(root).map_err(|source| LoadError::Io {
242+
path: root.clone(),
243+
source,
244+
})?;
245+
}
246+
for dir in &self.repository_dirs {
247+
repository.add(dir).map_err(|source| LoadError::Io {
248+
path: dir.clone(),
249+
source,
250+
})?;
251+
}
252+
Ok(repository)
253+
}
254+
234255
/// Expands manifest dependencies depth-first in request order, matching
235256
/// the Ruby implementation's insertion-ordered `Set` of libraries.
236257
fn resolved_libraries(&self, stdlib: &Repository) -> Result<Vec<Library>, LoadError> {
@@ -300,12 +321,12 @@ impl EnvironmentLoader {
300321

301322
Ok(())
302323
}
324+
}
303325

304-
fn library_dir(&self, library: &Library) -> Option<PathBuf> {
305-
self.repository
306-
.lookup(&library.name, library.version.as_deref())
307-
.map(Path::to_path_buf)
308-
}
326+
fn library_dir(repository: &Repository, library: &Library) -> Option<PathBuf> {
327+
repository
328+
.lookup(&library.name, library.version.as_deref())
329+
.map(Path::to_path_buf)
309330
}
310331

311332
/// Reads, parses, and converts a single signature file into an owned

rust/ruby-rbs/tests/loader.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ fn loads_core_stdlib_and_dependencies_in_ruby_order() {
4444
Some(repo_root().join("core")),
4545
Some(repo_root().join("stdlib")),
4646
)
47-
.repository(stdlib_repository())
4847
.add_library("bigdecimal-math", None);
4948

5049
let mut env = Environment::new();
@@ -72,8 +71,7 @@ fn from_loader_is_the_primary_entry_point() {
7271
let loader = EnvironmentLoader::new(
7372
Some(repo_root().join("core")),
7473
Some(repo_root().join("stdlib")),
75-
)
76-
.repository(stdlib_repository());
74+
);
7775

7876
let env = Environment::from_loader(&loader).unwrap();
7977

@@ -139,6 +137,19 @@ fn parse_errors_carry_the_file_path() {
139137
));
140138
}
141139

140+
#[test]
141+
fn default_configuration_loads_without_extra_repository_wiring() {
142+
let loader = EnvironmentLoader::new(
143+
Some(repo_root().join("core")),
144+
Some(repo_root().join("stdlib")),
145+
);
146+
147+
let mut env = Environment::new();
148+
let loaded = loader.load(&mut env).unwrap();
149+
150+
assert!(loaded.iter().any(|f| f.kind == lib("stringio")));
151+
}
152+
142153
#[test]
143154
fn core_requires_resolvable_stringio() {
144155
let loader = EnvironmentLoader::new(Some(repo_root().join("core")), None);
@@ -157,7 +168,7 @@ fn custom_repository_manifests_do_not_expand_dependencies() {
157168
// Without stdlib_root the bigdecimal-math manifest is ignored even
158169
// though the loading repository resolves the library itself.
159170
let loader = EnvironmentLoader::new(None, None)
160-
.repository(stdlib_repository())
171+
.add_repository_dir(repo_root().join("stdlib"))
161172
.add_library("bigdecimal-math", None);
162173

163174
let mut env = Environment::new();
@@ -205,11 +216,8 @@ fn library_dirs_skip_underscore_directories() {
205216
("gem1/1.2.3/_private/b.rbs", "class Person::Internal\nend\n"),
206217
]);
207218

208-
let mut repository = Repository::new();
209-
repository.add(dir.path()).unwrap();
210-
211219
let loader = EnvironmentLoader::new(None, None)
212-
.repository(repository)
220+
.add_repository_dir(dir.path().to_path_buf())
213221
.add_library("gem1", Some("1.2.3"));
214222

215223
let mut env = Environment::new();

0 commit comments

Comments
 (0)