ruby-rbs crate: Detect duplicated interface declarations in Environment

rust/ruby-rbs/Cargo.toml

@@ -17,6 +17,7 @@ include = [
 [dependencies]
 ruby-rbs-sys = { version = "0.3", path = "../ruby-rbs-sys" }
 xxhash-rust = { version = "0.8", features = ["xxh3"] }
+indexmap = "2"
 
 [build-dependencies]
 serde = { version = "1.0", features = ["derive"] }

rust/ruby-rbs/src/ast/declarations.rs

@@ -3,7 +3,7 @@ use crate::ast::comment::Comment;
 use crate::ast::location::{
     AliasDeclarationLocation, ClassDeclarationLocation, ClassSuperLocation,
     ConstantDeclarationLocation, GlobalDeclarationLocation, InterfaceDeclarationLocation,
-    ModuleDeclarationLocation, ModuleSelfLocation, TypeAliasDeclarationLocation,
+    LocationRange, ModuleDeclarationLocation, ModuleSelfLocation, TypeAliasDeclarationLocation,
 };
 use crate::ast::members::Member;
 use crate::ast::type_param::TypeParam;
@@ -22,6 +22,22 @@ pub enum Declaration {
     ModuleAlias(ModuleAliasDeclaration),
 }
 
+impl Declaration {
+    #[must_use]
+    pub(crate) fn location_range(&self) -> Option<LocationRange> {
+        match self {
+            Declaration::Class(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::Module(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::Interface(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::Constant(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::Global(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::TypeAlias(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::ClassAlias(d) => d.location.as_ref().map(|l| l.range),
+            Declaration::ModuleAlias(d) => d.location.as_ref().map(|l| l.range),
+        }
+    }
+}
+
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub enum ClassMember {
     Member(Member),

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

@@ -0,0 +1,23 @@
+use crate::ast::Declaration;
+
+use super::Environment;
+
+/// Only valid against the [`Environment`] that issued it.
+///
+/// Only top-level declarations are registered for now, so an id is just the
+/// declaration's position in `sources`.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub struct DeclId {
+    pub(super) source: u32,
+    pub(super) index: u32,
+}
+
+impl Environment {
+    /// # Panics
+    ///
+    /// May panic if `id` was issued by a different `Environment`.
+    #[must_use]
+    pub fn decl(&self, id: DeclId) -> &Declaration {
+        &self.sources[id.source as usize].declarations[id.index as usize]
+    }
+}

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

@@ -0,0 +1,29 @@
+use crate::ids::TypeName;
+
+use super::{DeclId, Environment};
+
+/// `name` repeats the table key, as in Ruby, so iterating entries alone
+/// yields their names.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+#[non_exhaustive]
+pub struct SingleEntry {
+    pub name: TypeName,
+    pub decl: DeclId,
+}
+
+impl Environment {
+    #[must_use]
+    pub fn is_interface_name(&self, name: TypeName) -> bool {
+        self.interface_decls.contains_key(&name)
+    }
+
+    #[must_use]
+    pub fn interface_entry(&self, name: TypeName) -> Option<&SingleEntry> {
+        self.interface_decls.get(&name)
+    }
+
+    /// In insertion order, like iterating Ruby's `interface_decls`.
+    pub fn interface_entries(&self) -> impl Iterator<Item = &SingleEntry> {
+        self.interface_decls.values()
+    }
+}

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

@@ -0,0 +1,52 @@
+use std::fmt;
+use std::path::PathBuf;
+
+use crate::ast::location::LocationRange;
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct DuplicatedDecl {
+    pub path: PathBuf,
+    pub location: Option<LocationRange>,
+}
+
+/// Owns rendered data instead of `DeclId`s because it outlives the
+/// `Environment` (`Environment::from_loader` drops it on error).
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct DuplicatedDeclarationError {
+    name: String,
+    decls: Vec<DuplicatedDecl>,
+}
+
+impl DuplicatedDeclarationError {
+    pub(crate) fn new(name: String, inserted: DuplicatedDecl, existing: DuplicatedDecl) -> Self {
+        Self {
+            name,
+            decls: vec![inserted, existing],
+        }
+    }
+
+    #[must_use]
+    pub fn name(&self) -> &str {
+        &self.name
+    }
+
+    /// The newly inserted declaration first, then the existing ones, as in
+    /// Ruby. Always at least two; a slice because Ruby's error takes
+    /// `*decls`, so class entries will report every reopening.
+    #[must_use]
+    pub fn decls(&self) -> &[DuplicatedDecl] {
+        &self.decls
+    }
+}
+
+impl fmt::Display for DuplicatedDeclarationError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        // Ruby's nil-location fallback: line:col needs the source text, which
+        // `Source` does not keep yet, and path alone matches no Ruby format.
+        // TODO: render `decls.last()` as `path:line:col...line:col`.
+        write!(f, "*:*:*...*:*: Duplicated declaration: {}", self.name)
+    }
+}
+
+impl std::error::Error for DuplicatedDeclarationError {}

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

@@ -0,0 +1,196 @@
+use crate::ast::Declaration;
+use crate::ids::TypeName;
+
+use super::entry::SingleEntry;
+use super::error::{DuplicatedDecl, DuplicatedDeclarationError};
+use super::source::Source;
+use super::{DeclId, Environment};
+
+impl Environment {
+    /// Like Ruby, the source and the entries inserted before a duplication
+    /// error are not rolled back. The rejected declaration itself is not
+    /// registered.
+    pub(crate) fn add_source(&mut self, source: Source) -> Result<(), DuplicatedDeclarationError> {
+        let source_index = u32::try_from(self.sources.len()).expect("too many sources");
+        let decl_count =
+            u32::try_from(source.declarations.len()).expect("too many declarations in one source");
+        self.sources.push(source);
+
+        for index in 0..decl_count {
+            self.insert_decl(DeclId {
+                source: source_index,
+                index,
+            })?;
+        }
+        Ok(())
+    }
+
+    fn insert_decl(&mut self, id: DeclId) -> Result<(), DuplicatedDeclarationError> {
+        let decl_name = match self.decl(id) {
+            Declaration::Interface(d) => d.name,
+            // TODO: register the other declaration kinds.
+            _ => return Ok(()),
+        };
+
+        let name = self.interners.type_names.to_absolute(decl_name);
+        if let Some(existing) = self.interface_decls.get(&name) {
+            return Err(self.duplicated_declaration(name, id, existing.decl));
+        }
+        self.interface_decls
+            .insert(name, SingleEntry { name, decl: id });
+
+        Ok(())
+    }
+
+    fn duplicated_declaration(
+        &self,
+        name: TypeName,
+        inserted: DeclId,
+        existing: DeclId,
+    ) -> DuplicatedDeclarationError {
+        let interners = &self.interners;
+        DuplicatedDeclarationError::new(
+            interners.type_names.display(name, &interners.strings),
+            self.duplicated_decl(inserted),
+            self.duplicated_decl(existing),
+        )
+    }
+
+    fn duplicated_decl(&self, id: DeclId) -> DuplicatedDecl {
+        DuplicatedDecl {
+            path: self.sources[id.source as usize].path.clone(),
+            location: self.decl(id).location_range(),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::path::PathBuf;
+
+    use crate::ast::{AstConverter, Declaration};
+    use crate::environment::{DuplicatedDeclarationError, Environment, Source, SourceKind};
+    use crate::ids::TypeName;
+    use crate::node;
+
+    fn parse_decls(env: &mut Environment, src: &str) -> Vec<Declaration> {
+        let signature = node::parse(src).expect("valid RBS source");
+        let interners = env.interners_mut();
+        let mut converter = AstConverter::new(&mut interners.strings, &mut interners.type_names);
+        signature
+            .declarations()
+            .iter()
+            .map(|node| converter.convert_declaration(&node))
+            .collect()
+    }
+
+    fn add_rbs_source(
+        env: &mut Environment,
+        path: &str,
+        declarations: Vec<Declaration>,
+    ) -> Result<(), DuplicatedDeclarationError> {
+        env.add_source(Source {
+            path: PathBuf::from(path),
+            directives: Vec::new(),
+            declarations,
+            kind: SourceKind::Dir {
+                path: PathBuf::from("."),
+            },
+        })
+    }
+
+    fn type_name(env: &mut Environment, name: &str) -> TypeName {
+        let interners = env.interners_mut();
+        interners.type_names.parse(&mut interners.strings, name)
+    }
+
+    // environment_test.rb:169
+    #[test]
+    fn interface_twice_duplication_error() {
+        let mut env = Environment::new();
+        let decls = parse_decls(&mut env, "interface _I\nend\ninterface _I\nend\n");
+
+        add_rbs_source(&mut env, "a.rbs", vec![decls[0].clone()]).unwrap();
+        let err = add_rbs_source(&mut env, "b.rbs", vec![decls[1].clone()]).unwrap_err();
+
+        assert_eq!(err.name(), "::_I");
+        let [inserted, existing] = err.decls() else {
+            panic!("expected two decls, got {:?}", err.decls());
+        };
+        assert_eq!(inserted.path, PathBuf::from("b.rbs"));
+        assert_eq!(inserted.location, decls[1].location_range());
+        assert_eq!(existing.path, PathBuf::from("a.rbs"));
+        assert_eq!(existing.location, decls[0].location_range());
+        assert_eq!(err.to_string(), "*:*:*...*:*: Duplicated declaration: ::_I");
+
+        // The rejected declaration is not registered.
+        assert_eq!(env.interface_entries().count(), 1);
+        let name = type_name(&mut env, "::_I");
+        let entry = env.interface_entry(name).unwrap();
+        assert_eq!(env.decl(entry.decl), &decls[0]);
+    }
+
+    #[test]
+    fn absolute_and_relative_names_collide() {
+        let mut env = Environment::new();
+        let decls = parse_decls(&mut env, "interface _I\nend\ninterface ::_I\nend\n");
+
+        add_rbs_source(&mut env, "a.rbs", vec![decls[0].clone()]).unwrap();
+        let err = add_rbs_source(&mut env, "b.rbs", vec![decls[1].clone()]).unwrap_err();
+
+        assert_eq!(err.name(), "::_I");
+    }
+
+    #[test]
+    fn duplication_within_a_source_keeps_earlier_entries() {
+        let mut env = Environment::new();
+        let decls = parse_decls(
+            &mut env,
+            "interface _A\nend\ninterface _I\nend\ninterface _I\nend\ninterface _B\nend\n",
+        );
+
+        let err = add_rbs_source(&mut env, "a.rbs", decls.clone()).unwrap_err();
+
+        let [inserted, existing] = err.decls() else {
+            panic!("expected two decls, got {:?}", err.decls());
+        };
+        assert_eq!(inserted.path, PathBuf::from("a.rbs"));
+        assert_eq!(inserted.location, decls[2].location_range());
+        assert_eq!(existing.path, PathBuf::from("a.rbs"));
+        assert_eq!(existing.location, decls[1].location_range());
+
+        // Not rolled back: the source and the entries before the duplicate stay.
+        assert_eq!(env.sources().len(), 1);
+        let a = type_name(&mut env, "::_A");
+        let i = type_name(&mut env, "::_I");
+        let b = type_name(&mut env, "::_B");
+        let names: Vec<_> = env.interface_entries().map(|e| e.name).collect();
+        assert_eq!(names, [a, i]);
+        assert_eq!(env.decl(env.interface_entry(i).unwrap().decl), &decls[1]);
+        // Declarations after the duplicate are never reached.
+        assert!(!env.is_interface_name(b));
+    }
+
+    #[test]
+    fn distinct_interfaces_are_registered_in_order() {
+        let mut env = Environment::new();
+        let decls = parse_decls(&mut env, "interface _A\nend\ninterface _B\nend\n");
+
+        add_rbs_source(&mut env, "a.rbs", vec![decls[0].clone()]).unwrap();
+        add_rbs_source(&mut env, "b.rbs", vec![decls[1].clone()]).unwrap();
+
+        let a = type_name(&mut env, "::_A");
+        let b = type_name(&mut env, "::_B");
+        let unknown = type_name(&mut env, "::_C");
+        assert!(env.is_interface_name(a));
+        assert!(env.is_interface_name(b));
+        assert!(!env.is_interface_name(unknown));
+        assert!(env.interface_entry(unknown).is_none());
+
+        let names: Vec<_> = env.interface_entries().map(|e| e.name).collect();
+        assert_eq!(names, [a, b]);
+        for (entry, decl) in env.interface_entries().zip(&decls) {
+            assert_eq!(env.decl(entry.decl), decl);
+        }
+    }
+}

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

@@ -1,7 +1,15 @@
+mod decl;
+mod entry;
+mod error;
+mod insert;
 pub mod source;
 
+pub use decl::DeclId;
+pub use entry::SingleEntry;
+pub use error::{DuplicatedDecl, DuplicatedDeclarationError};
 pub use source::{Source, SourceKind};
 
+use crate::ids::{IdIndexMap, TypeNameTag};
 use crate::interners::Interners;
 use crate::loader::{EnvironmentLoader, LoadError};
 
@@ -12,6 +20,7 @@ use crate::loader::{EnvironmentLoader, LoadError};
 pub struct Environment {
     interners: Interners,
     sources: Vec<Source>,
+    interface_decls: IdIndexMap<TypeNameTag, SingleEntry>,
 }
 
 impl Environment {
@@ -32,10 +41,6 @@ impl Environment {
         &mut self.interners
     }
 
-    pub(crate) fn add_source(&mut self, source: Source) {
-        self.sources.push(source);
-    }
-
     pub fn from_loader(loader: &EnvironmentLoader) -> Result<Environment, LoadError> {
         let mut env = Environment::new();
         loader.load(&mut env)?;