Revert "Add Environment::add_rbs and route EnvironmentLoader through it"

This reverts commit fcf2f0f.

Author: dak2

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

@@ -2,13 +2,8 @@ pub mod source;
 
 pub use source::{Source, SourceKind};
 
-use std::path::{Path, PathBuf};
-
-use crate::ast::AstConverter;
-use crate::buffer::Buffer;
 use crate::interners::Interners;
 use crate::loader::{EnvironmentLoader, LoadError};
-use crate::node;
 
 /// Owning the interners here gives a single `Environment` value the same
 /// role as the Ruby implementation's global name pool: names interned while
@@ -34,56 +29,12 @@ impl Environment {
         &self.sources
     }
 
-    /// Parses `content` with this environment's own [`Interners`] and adds it
-    /// as a source, without going through an [`EnvironmentLoader`] — the
-    /// entry point for editor / in-memory callers (`RBS::Environment#add_source`
-    /// equivalent, e.g. Steep). Parsing always uses this environment's
-    /// `Interners`, so a `Source` added this way can never carry ids from a
-    /// different environment.
-    pub fn add_rbs(
-        &mut self,
-        path: PathBuf,
-        content: String,
-        kind: SourceKind,
-    ) -> Result<(), LoadError> {
-        let signature = node::parse(&content).map_err(|message| LoadError::Parse {
-            path: path.clone(),
-            message,
-        })?;
-
-        let mut converter =
-            AstConverter::new(&mut self.interners.strings, &mut self.interners.type_names);
-        let directives = signature
-            .directives()
-            .iter()
-            .map(|node| converter.convert_directive(&node))
-            .collect();
-        let declarations = signature
-            .declarations()
-            .iter()
-            .map(|node| converter.convert_declaration(&node))
-            .collect();
-        // SignatureNode borrows `content` and has a Drop impl; drop it
-        // explicitly before moving `content` into the Buffer.
-        drop(signature);
-
-        self.sources.push(Source {
-            buffer: Buffer::new(path, content),
-            directives,
-            declarations,
-            kind,
-        });
-        Ok(())
+    pub(crate) fn interners_mut(&mut self) -> &mut Interners {
+        &mut self.interners
     }
 
-    /// Reads `path` and adds it as a source; a thin wrapper around
-    /// [`Environment::add_rbs`].
-    pub fn add_rbs_file(&mut self, path: &Path, kind: SourceKind) -> Result<(), LoadError> {
-        let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io {
-            path: path.to_path_buf(),
-            source,
-        })?;
-        self.add_rbs(path.to_path_buf(), content, kind)
+    pub(crate) fn add_source(&mut self, source: Source) {
+        self.sources.push(source);
     }
 
     pub fn from_loader(loader: &EnvironmentLoader) -> Result<Environment, LoadError> {
@@ -102,27 +53,21 @@ impl Default for Environment {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::ast::Declaration;
 
     #[test]
     fn environment_owns_interners() {
         let mut env = Environment::new();
-        env.add_rbs(
-            PathBuf::from("test.rbs"),
-            "class Foo\nend\n".to_string(),
-            SourceKind::Dir {
-                path: PathBuf::from("."),
-            },
-        )
-        .unwrap();
 
-        let [Declaration::Class(class)] = env.sources()[0].declarations.as_slice() else {
-            panic!("expected one class declaration");
-        };
+        let interners = env.interners_mut();
+        let symbol = interners.strings.intern("Foo");
+        let root = interners.type_names.absolute_root();
+        let name = interners.type_names.append(root, symbol);
+
         let interners = env.interners();
         assert_eq!(
-            interners.type_names.display(class.name, &interners.strings),
-            "Foo"
+            interners.type_names.display(name, &interners.strings),
+            "::Foo"
         );
+        assert!(env.sources().is_empty());
     }
 }

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

@@ -1,10 +1,14 @@
 use std::collections::HashSet;
 use std::fmt;
 use std::io;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
-use crate::environment::{Environment, SourceKind};
+use crate::ast::AstConverter;
+use crate::buffer::Buffer;
+use crate::environment::{Environment, Source, SourceKind};
 use crate::file_finder;
+use crate::interners::Interners;
+use crate::node;
 
 #[derive(Debug)]
 #[non_exhaustive]
@@ -98,7 +102,8 @@ impl EnvironmentLoader {
                 if !seen_files.insert(path.clone()) {
                     continue;
                 }
-                env.add_rbs_file(&path, kind.clone())?;
+                let source = parse_one(&path, &kind, env.interners_mut())?;
+                env.add_source(source);
                 loaded.push(LoadedFile {
                     path,
                     kind: kind.clone(),
@@ -135,3 +140,49 @@ impl EnvironmentLoader {
         result
     }
 }
+
+/// Deliberately a free function taking only the interners, so the load loop
+/// can later be parallelised by handing each worker its own [`Interners`].
+/// The parser's `SignatureNode` holds raw pointers and is not `Send`, so it
+/// must not escape this function — only the owned `Source` does.
+///
+/// Crate-private: a caller outside the crate has no way to merge its
+/// worker-local [`Interners`] into the environment, so the `Source` it
+/// produced would carry ids that environment cannot resolve.
+pub(crate) fn parse_one(
+    path: &Path,
+    kind: &SourceKind,
+    interners: &mut Interners,
+) -> Result<Source, LoadError> {
+    let content = std::fs::read_to_string(path).map_err(|source| LoadError::Io {
+        path: path.to_path_buf(),
+        source,
+    })?;
+
+    let signature = node::parse(&content).map_err(|message| LoadError::Parse {
+        path: path.to_path_buf(),
+        message,
+    })?;
+
+    let mut converter = AstConverter::new(&mut interners.strings, &mut interners.type_names);
+    let directives = signature
+        .directives()
+        .iter()
+        .map(|node| converter.convert_directive(&node))
+        .collect();
+    let declarations = signature
+        .declarations()
+        .iter()
+        .map(|node| converter.convert_declaration(&node))
+        .collect();
+    // SignatureNode borrows `content` and has a Drop impl; drop it
+    // explicitly before moving `content` into the Buffer.
+    drop(signature);
+
+    Ok(Source {
+        buffer: Buffer::new(path.to_path_buf(), content),
+        directives,
+        declarations,
+        kind: kind.clone(),
+    })
+}