diff --git a/AGENTS.md b/AGENTS.md index f76e2ecb..4c93d2fa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,6 +78,10 @@ AST to save in the graph - `rust/rubydex/src/resolution.rs`: the Resolution stage that computes fully qualified names, creates declarations, resolves constant references, and linearizes ancestor chains +Inheritance edges: namespaces store only *immediate* descendants (direct superclass/include/prepend children, plus +`extend`-via-singleton). For transitive descendants, call `Graph::transitive_descendants(root)` — a BFS iterator over +the direct-child DAG with dedup. Never add a stored transitive set; it would defeat the memory-bounded invariant. + ### Commands When necessary, commands can be executed for the Rust code. diff --git a/docs/architecture.md b/docs/architecture.md index 51560a8c..9e72f183 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -97,6 +97,14 @@ Rubydex represents the codebase as a graph, where entities are nodes and relatio - `model/declaration.rs`: Global declarations produced during resolution - `model/graph.rs`: The main graph structure containing all entities +### Inheritance edges + +Each namespace stores only its **immediate** descendants — the declarations that directly inherit from, include, or prepend it. For singleton classes, the edge is recorded against the singleton from an `extend`. + +Transitive descendants are computed on demand via `Graph::transitive_descendants(root)`, a BFS iterator over the direct-child DAG with visited-set dedup. + +Use the iterator (not the stored field) anywhere you need the full transitive set — the stored field is intentionally the direct-edge subset to keep memory bounded. + ### ID Types Connections between nodes use hashed IDs defined in `ids.rs`: diff --git a/rust/rubydex-mcp/src/server.rs b/rust/rubydex-mcp/src/server.rs index 239aed38..d87603b6 100644 --- a/rust/rubydex-mcp/src/server.rs +++ b/rust/rubydex-mcp/src/server.rs @@ -379,19 +379,19 @@ impl RubydexServer { fn get_descendants(&self, Parameters(params): Parameters) -> String { let state = ensure_graph_ready!(self); let graph = state.graph.as_ref().unwrap(); - let (_, decl) = lookup_declaration!(graph, ¶ms.name); - let namespace = require_namespace!(decl, ¶ms.name, "get_descendants"); + let (decl_id, decl) = lookup_declaration!(graph, ¶ms.name); + require_namespace!(decl, ¶ms.name, "get_descendants"); let limit = params.limit.filter(|&l| l > 0).unwrap_or(50).min(500); // default 50, max 500 let offset = params.offset.unwrap_or(0); let (descendants, total) = paginate!( - namespace.descendants().iter(), + graph.transitive_descendants(decl_id), offset, limit, - |id| graph.declarations().get(id).is_some(), - |id| { - let desc_decl = graph.declarations().get(id)?; + |id: &DeclarationId| graph.declarations().get(id).is_some(), + |id: DeclarationId| { + let desc_decl = graph.declarations().get(&id)?; Some(serde_json::json!({ "name": desc_decl.name(), "kind": desc_decl.kind(), diff --git a/rust/rubydex-sys/src/declaration_api.rs b/rust/rubydex-sys/src/declaration_api.rs index 0b05231a..98bea8d5 100644 --- a/rust/rubydex-sys/src/declaration_api.rs +++ b/rust/rubydex-sys/src/declaration_api.rs @@ -382,14 +382,16 @@ pub unsafe extern "C" fn rdx_declaration_descendants(pointer: GraphPointer, decl let declarations = with_graph(pointer, |graph| { let declaration_id = DeclarationId::new(decl_id); - let Some(Declaration::Namespace(declaration)) = graph.declarations().get(&declaration_id) else { + if !matches!( + graph.declarations().get(&declaration_id), + Some(Declaration::Namespace(_)) + ) { return Vec::new(); - }; + } - declaration - .descendants() - .iter() - .map(|id| CDeclaration::from_declaration(*id, graph.declarations().get(id).unwrap())) + graph + .transitive_descendants(declaration_id) + .map(|id| CDeclaration::from_declaration(id, graph.declarations().get(&id).unwrap())) .collect::>() }); diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index 64366c99..9d2c81d2 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -107,7 +107,7 @@ macro_rules! namespace_declaration { /// declaration inherits from ancestors: Ancestors, /// The set of declarations that inherit from this declaration - descendants: IdentityHashSet, + immediate_descendants: IdentityHashSet, /// The singleton class associated with this declaration singleton_class_id: Option, /// Diagnostics associated with this declaration @@ -124,7 +124,7 @@ macro_rules! namespace_declaration { references: IdentityHashSet::default(), owner_id, ancestors: Ancestors::Partial(Vec::new()), - descendants: IdentityHashSet::default(), + immediate_descendants: IdentityHashSet::default(), singleton_class_id: None, diagnostics: Vec::new(), } @@ -206,20 +206,20 @@ macro_rules! namespace_declaration { matches!(&self.ancestors, Ancestors::Complete(_) | Ancestors::Cyclic(_)) } - pub fn add_descendant(&mut self, descendant_id: DeclarationId) { - self.descendants.insert(descendant_id); + pub fn add_immediate_descendant(&mut self, descendant_id: DeclarationId) { + self.immediate_descendants.insert(descendant_id); } - fn remove_descendant(&mut self, descendant_id: &DeclarationId) { - self.descendants.remove(descendant_id); + fn remove_immediate_descendant(&mut self, descendant_id: &DeclarationId) { + self.immediate_descendants.remove(descendant_id); } - pub fn clear_descendants(&mut self) { - self.descendants.clear(); + pub fn clear_immediate_descendants(&mut self) { + self.immediate_descendants.clear(); } - pub fn descendants(&self) -> &IdentityHashSet { - &self.descendants + pub fn immediate_descendants(&self) -> &IdentityHashSet { + &self.immediate_descendants } } }; @@ -526,16 +526,16 @@ impl Namespace { } #[must_use] - pub fn descendants(&self) -> &IdentityHashSet { - all_namespaces!(self, it => it.descendants()) + pub fn immediate_descendants(&self) -> &IdentityHashSet { + all_namespaces!(self, it => it.immediate_descendants()) } - pub fn add_descendant(&mut self, descendant_id: DeclarationId) { - all_namespaces!(self, it => it.add_descendant(descendant_id)); + pub fn add_immediate_descendant(&mut self, descendant_id: DeclarationId) { + all_namespaces!(self, it => it.add_immediate_descendant(descendant_id)); } - pub fn remove_descendant(&mut self, descendant_id: &DeclarationId) { - all_namespaces!(self, it => it.remove_descendant(descendant_id)); + pub fn remove_immediate_descendant(&mut self, descendant_id: &DeclarationId) { + all_namespaces!(self, it => it.remove_immediate_descendant(descendant_id)); } pub fn for_each_ancestor(&self, mut f: F) @@ -545,19 +545,12 @@ impl Namespace { all_namespaces!(self, it => it.ancestors().iter().for_each(&mut f)); } - pub fn for_each_descendant(&self, mut f: F) - where - F: FnMut(&DeclarationId), - { - all_namespaces!(self, it => it.descendants().iter().for_each(&mut f)); - } - pub fn clear_ancestors(&mut self) { all_namespaces!(self, it => it.set_ancestors(Ancestors::Partial(vec![]))); } - pub fn clear_descendants(&mut self) { - all_namespaces!(self, it => it.clear_descendants()); + pub fn clear_immediate_descendants(&mut self) { + all_namespaces!(self, it => it.clear_immediate_descendants()); } #[must_use] diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 798e292f..eae57adf 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -1,4 +1,5 @@ use std::collections::HashSet; +use std::collections::VecDeque; use std::collections::hash_map::Entry; use std::path::PathBuf; @@ -1107,7 +1108,6 @@ impl Graph { let should_remove = decl.has_no_definitions() || !self.declarations.contains_key(decl.owner_id()); if should_remove { - // Queue members + singleton for removal if let Some(ns) = decl.as_namespace() { if let Some(singleton_id) = ns.singleton_class() { queue.push(InvalidationItem::Declaration(*singleton_id)); @@ -1115,9 +1115,7 @@ impl Graph { for member_decl_id in ns.members().values() { queue.push(InvalidationItem::Declaration(*member_decl_id)); } - for descendant_id in ns.descendants() { - queue.push(InvalidationItem::Declaration(*descendant_id)); - } + queue.extend(self.transitive_descendants(decl_id).map(InvalidationItem::Declaration)); } // Unresolve names and cascade. Reference dependents from surviving @@ -1166,24 +1164,31 @@ impl Graph { return; }; - // Remove self from each ancestor's descendant set - for ancestor in &namespace.clone_ancestors() { - if let Ancestor::Complete(ancestor_id) = ancestor - && let Some(anc_decl) = self.declarations.get_mut(ancestor_id) + // Only direct parents actually contain `decl_id`; walking the full chain + // does O(chain_len) hash-miss no-ops on non-direct ancestors, which is cheap. + let ancestor_ids: Vec = namespace + .ancestors() + .into_iter() + .filter_map(|a| match a { + Ancestor::Complete(id) => Some(*id), + Ancestor::Partial(_) => None, + }) + .collect(); + + for ancestor_id in ancestor_ids { + if let Some(anc_decl) = self.declarations.get_mut(&ancestor_id) && let Some(ns) = anc_decl.as_namespace_mut() { - ns.remove_descendant(&decl_id); + ns.remove_immediate_descendant(&decl_id); } } - let namespace = self.declarations.get_mut(&decl_id).unwrap().as_namespace_mut().unwrap(); + queue.extend(self.transitive_descendants(decl_id).map(InvalidationItem::Declaration)); - namespace.for_each_descendant(|descendant_id| { - queue.push(InvalidationItem::Declaration(*descendant_id)); - }); + let namespace = self.declarations.get_mut(&decl_id).unwrap().as_namespace_mut().unwrap(); namespace.clear_ancestors(); - namespace.clear_descendants(); + namespace.clear_immediate_descendants(); self.push_work(Unit::Ancestors(decl_id)); @@ -1426,6 +1431,58 @@ impl Graph { } } +impl Graph { + /// Yields `root` first, then every transitively reachable descendant exactly once, in + /// BFS order. Useful anywhere the caller would otherwise need a full transitive set. + #[must_use] + pub fn transitive_descendants(&self, root: DeclarationId) -> TransitiveDescendants<'_> { + TransitiveDescendants::new(self, root) + } +} + +/// BFS iterator over the direct-child DAG rooted at a namespace, yielding `root` +/// first and then every transitively reachable descendant exactly once. +/// +/// Construct via [`Graph::transitive_descendants`]. The visited set dedups when +/// the same declaration is reachable via multiple paths (e.g. inheritance plus +/// direct include of a shared module). +pub struct TransitiveDescendants<'a> { + graph: &'a Graph, + queue: VecDeque, + visited: IdentityHashSet, +} + +impl<'a> TransitiveDescendants<'a> { + fn new(graph: &'a Graph, root: DeclarationId) -> Self { + let mut queue = VecDeque::new(); + queue.push_back(root); + Self { + graph, + queue, + visited: IdentityHashSet::default(), + } + } +} + +impl Iterator for TransitiveDescendants<'_> { + type Item = DeclarationId; + + fn next(&mut self) -> Option { + while let Some(id) = self.queue.pop_front() { + if !self.visited.insert(id) { + continue; + } + if let Some(ns) = self.graph.declarations().get(&id).and_then(|d| d.as_namespace()) { + self.queue.extend(ns.immediate_descendants().iter().copied()); + } + return Some(id); + } + None + } +} + +impl std::iter::FusedIterator for TransitiveDescendants<'_> {} + #[cfg(test)] mod tests { use super::*; @@ -1433,8 +1490,8 @@ mod tests { use crate::model::declaration::Ancestors; use crate::test_utils::GraphTest; use crate::{ - assert_declaration_does_not_exist, assert_dependents, assert_descendants, assert_members_eq, - assert_no_diagnostics, assert_no_members, + assert_declaration_does_not_exist, assert_dependents, assert_descendants, assert_immediate_descendants, + assert_members_eq, assert_no_diagnostics, assert_no_members, }; #[test] @@ -1633,7 +1690,7 @@ mod tests { panic!("Expected Foo to be a class"); }; assert!(matches!(foo.ancestors(), Ancestors::Partial(a) if a.is_empty())); - assert!(foo.descendants().is_empty()); + assert!(foo.immediate_descendants().is_empty()); let Declaration::Namespace(Namespace::Class(baz)) = context.graph().declarations().get(&DeclarationId::from("Baz")).unwrap() @@ -1641,14 +1698,14 @@ mod tests { panic!("Expected Baz to be a class"); }; assert!(matches!(baz.ancestors(), Ancestors::Partial(a) if a.is_empty())); - assert!(baz.descendants().is_empty()); + assert!(baz.immediate_descendants().is_empty()); let Declaration::Namespace(Namespace::Module(bar)) = context.graph().declarations().get(&DeclarationId::from("Bar")).unwrap() else { panic!("Expected Bar to be a module"); }; - assert!(!bar.descendants().contains(&DeclarationId::from("Foo"))); + assert!(!bar.immediate_descendants().contains(&DeclarationId::from("Foo"))); } context.resolve(); @@ -2327,6 +2384,124 @@ mod tests { "Bar name should be removed from the names map" ); } + + #[test] + fn transitive_descendants_yields_root_first_then_chain() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + module M; end + class A; include M; end + class B < A; end + " + }); + context.resolve(); + + let ids: Vec<_> = context + .graph() + .transitive_descendants(DeclarationId::from("M")) + .collect(); + + assert_eq!(ids.first().copied(), Some(DeclarationId::from("M"))); + assert!(ids.contains(&DeclarationId::from("A"))); + assert!(ids.contains(&DeclarationId::from("B"))); + let mut uniq = ids.clone(); + uniq.sort_by_key(DeclarationId::get); + uniq.dedup(); + assert_eq!(uniq.len(), ids.len(), "iterator must not yield duplicates"); + } + + #[test] + fn transitive_descendants_dedups_across_multiple_paths() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + module M; end + class A; include M; end + class B < A + include M + end + " + }); + context.resolve(); + + let ids: Vec<_> = context + .graph() + .transitive_descendants(DeclarationId::from("M")) + .collect(); + + // B is reachable via M -> A -> B and via M -> B. Must appear exactly once. + let b_count = ids.iter().filter(|id| **id == DeclarationId::from("B")).count(); + assert_eq!( + b_count, 1, + "B must be yielded exactly once, got {b_count} (ids: {ids:?})" + ); + } + + #[test] + fn transitive_descendants_leaf_returns_only_root() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", "class Leaf; end"); + context.resolve(); + + let ids: Vec<_> = context + .graph() + .transitive_descendants(DeclarationId::from("Leaf")) + .collect(); + + assert_eq!(ids, vec![DeclarationId::from("Leaf")]); + } + + #[test] + fn transitive_descendants_include_chain() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + module A; end + module B; include A; end + class C; include B; end + " + }); + context.resolve(); + + let ids: Vec<_> = context + .graph() + .transitive_descendants(DeclarationId::from("A")) + .collect(); + + assert!(ids.contains(&DeclarationId::from("A"))); + assert!(ids.contains(&DeclarationId::from("B"))); + assert!(ids.contains(&DeclarationId::from("C"))); + } + + #[test] + fn immediate_descendants_are_direct_only() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + class Foo; end + class Bar < Foo; end + class Baz < Bar; end + " + }); + context.resolve(); + + // Foo directly parents Bar (not Baz). + assert_immediate_descendants!(context, "Foo", ["Bar"]); + // Bar directly parents Baz. + assert_immediate_descendants!(context, "Bar", ["Baz"]); + + // Negative assertion: Baz is a transitive descendant of Foo (via Bar), + // not an immediate one. Guards against regression to transitive storage. + let foo = context.graph().declarations().get(&DeclarationId::from("Foo")).unwrap(); + assert!( + !foo.as_namespace() + .unwrap() + .immediate_descendants() + .contains(&DeclarationId::from("Baz")), + "Baz is transitive of Foo (via Bar), not an immediate descendant", + ); + } } #[cfg(test)] diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index e7ed826c..c4c4c841 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1,7 +1,4 @@ -use std::{ - collections::{HashSet, VecDeque, hash_map::Entry}, - hash::BuildHasher, -}; +use std::collections::{HashSet, VecDeque, hash_map::Entry}; use crate::model::{ built_in::{BASIC_OBJECT_ID, CLASS_ID, KERNEL_ID, MODULE_ID, OBJECT_ID}, @@ -39,7 +36,6 @@ impl Outcome { } struct LinearizationContext { - descendants: IdentityHashSet, seen_ids: IdentityHashSet, cyclic: bool, partial: bool, @@ -48,7 +44,6 @@ struct LinearizationContext { impl LinearizationContext { fn new() -> Self { Self { - descendants: IdentityHashSet::default(), seen_ids: IdentityHashSet::default(), cyclic: false, partial: false, @@ -59,7 +54,6 @@ impl LinearizationContext { /// the linearization algorithm, regardless of whether we are returning a cached result or a freshly built ancestor /// chain fn finalize(&mut self, declaration_id: DeclarationId) { - self.descendants.remove(&declaration_id); self.seen_ids.remove(&declaration_id); } } @@ -704,6 +698,18 @@ impl<'a> Resolver<'a> { self.linearize_ancestors(declaration_id, &mut context) } + /// Record `child_id` as an immediate descendant of `parent_id`, unless they are + /// the same declaration — a namespace is never its own immediate descendant, even + /// under cyclic self-inclusion like `module A; include A; end`. + fn record_immediate_descendant(&mut self, parent_id: DeclarationId, child_id: DeclarationId) { + if parent_id != child_id + && let Some(decl) = self.graph.declarations_mut().get_mut(&parent_id) + && let Some(ns) = decl.as_namespace_mut() + { + ns.add_immediate_descendant(child_id); + } + } + /// Linearizes the ancestors of a declaration, returning the list of ancestor declaration IDs /// /// # Panics @@ -714,14 +720,10 @@ impl<'a> Resolver<'a> { { let declaration = self.graph.declarations_mut().get_mut(&declaration_id).unwrap(); - // Add this declaration to the descendants so that we capture transitive descendant relationships - context.descendants.insert(declaration_id); - // Return the cached ancestors if we already computed them. If they are partial ancestors, ignore the cache to try // again if declaration.as_namespace().unwrap().has_complete_ancestors() { let cached = declaration.as_namespace().unwrap().clone_ancestors(); - self.propagate_descendants(&mut context.descendants, &cached); context.finalize(declaration_id); return cached; @@ -745,18 +747,6 @@ impl<'a> Resolver<'a> { context.finalize(declaration_id); return estimated_ancestors; } - - // Automatically track descendants as we recurse. This has to happen before checking the cache since we may have - // already linearized the parent's ancestors, but it's the first time we're discovering the descendant - for descendant in &context.descendants { - self.graph - .declarations_mut() - .get_mut(&declaration_id) - .unwrap() - .as_namespace_mut() - .unwrap() - .add_descendant(*descendant); - } } let parent_ancestors = self.linearize_parent_ancestors(declaration_id, context); @@ -799,7 +789,7 @@ impl<'a> Resolver<'a> { } let (linearized_prepends, linearized_includes) = - self.linearize_mixins(context, mixins, parent_ancestors.as_ref()); + self.linearize_mixins(declaration_id, context, mixins, parent_ancestors.as_ref()); // Build the final list let mut ancestors = Vec::new(); @@ -845,17 +835,19 @@ impl<'a> Resolver<'a> { Declaration::Namespace(Namespace::Class(_)) => { let definition_ids = declaration.definitions().to_vec(); - Some(match self.linearize_parent_class(&definition_ids, context) { - Ancestors::Complete(ids) => ids, - Ancestors::Cyclic(ids) => { - context.cyclic = true; - ids - } - Ancestors::Partial(ids) => { - context.partial = true; - ids - } - }) + Some( + match self.linearize_parent_class(declaration_id, &definition_ids, context) { + Ancestors::Complete(ids) => ids, + Ancestors::Cyclic(ids) => { + context.cyclic = true; + ids + } + Ancestors::Partial(ids) => { + context.partial = true; + ids + } + }, + ) } Declaration::Namespace(Namespace::SingletonClass(_)) => { let owner_id = *declaration.owner_id(); @@ -865,6 +857,8 @@ impl<'a> Resolver<'a> { context.partial = true; } + self.record_immediate_descendant(singleton_parent_id, declaration_id); + Some(match self.linearize_ancestors(singleton_parent_id, context) { Ancestors::Complete(ids) => ids, Ancestors::Cyclic(ids) => { @@ -885,6 +879,7 @@ impl<'a> Resolver<'a> { /// modules are deduplicated against them fn linearize_mixins( &mut self, + child_id: DeclarationId, context: &mut LinearizationContext, mixins: Vec, parent_ancestors: Option<&Vec>, @@ -910,6 +905,8 @@ impl<'a> Resolver<'a> { continue; }; + self.record_immediate_descendant(module_id, child_id); + let ids = match self.linearize_ancestors(module_id, context) { Ancestors::Complete(ids) => ids, Ancestors::Cyclic(ids) => { @@ -949,6 +946,8 @@ impl<'a> Resolver<'a> { continue; }; + self.record_immediate_descendant(module_id, child_id); + let mut ids = match self.linearize_ancestors(module_id, context) { Ancestors::Complete(ids) => ids, Ancestors::Cyclic(ids) => { @@ -988,29 +987,6 @@ impl<'a> Resolver<'a> { (linearized_prepends, linearized_includes) } - /// Propagate descendants to all cached ancestors - fn propagate_descendants( - &mut self, - descendants: &mut HashSet, - cached: &Ancestors, - ) { - if !descendants.is_empty() { - for ancestor in cached { - if let Ancestor::Complete(ancestor_id) = ancestor { - for descendant in descendants.iter() { - self.graph - .declarations_mut() - .get_mut(ancestor_id) - .unwrap() - .as_namespace_mut() - .unwrap() - .add_descendant(*descendant); - } - } - } - } - } - // Handles the resolution of the namespace name, the creation of the declaration and membership fn handle_constant_declaration( &mut self, @@ -1802,10 +1778,12 @@ impl<'a> Resolver<'a> { fn linearize_parent_class( &mut self, + child_id: DeclarationId, definition_ids: &[DefinitionId], context: &mut LinearizationContext, ) -> Ancestors { let (picked_parent, unresolved_parent) = self.get_parent_class(definition_ids); + self.record_immediate_descendant(picked_parent, child_id); let mut result = self.linearize_ancestors(picked_parent, context); if let Some(name_id) = unresolved_parent { diff --git a/rust/rubydex/src/test_utils/graph_test.rs b/rust/rubydex/src/test_utils/graph_test.rs index 6a1a673c..4dc0cfd5 100644 --- a/rust/rubydex/src/test_utils/graph_test.rs +++ b/rust/rubydex/src/test_utils/graph_test.rs @@ -520,32 +520,46 @@ macro_rules! assert_ancestors_eq { #[macro_export] macro_rules! assert_descendants { ($context:expr, $parent:expr, $descendants:expr) => { + let parent_id = $crate::model::ids::DeclarationId::from($parent); + let actual: Vec<_> = $context.graph().transitive_descendants(parent_id).collect(); + + for descendant in &$descendants { + let descendant_id = $crate::model::ids::DeclarationId::from(*descendant); + + assert!( + actual.contains(&descendant_id), + "Expected '{}' to be a descendant of '{}'", + $context.graph().declarations().get(&descendant_id).unwrap().name(), + $context.graph().declarations().get(&parent_id).unwrap().name() + ); + } + }; +} + +#[cfg(test)] +#[macro_export] +macro_rules! assert_immediate_descendants { + ($context:expr, $parent:expr, $descendants:expr) => { + let parent_id = $crate::model::ids::DeclarationId::from($parent); let parent = $context .graph() .declarations() - .get(&$crate::model::ids::DeclarationId::from($parent)) - .unwrap(); - let actual = match parent { - $crate::model::declaration::Declaration::Namespace($crate::model::declaration::Namespace::Class(class)) => { - class.descendants().iter().cloned().collect::>() - } - $crate::model::declaration::Declaration::Namespace($crate::model::declaration::Namespace::Module( - module, - )) => module.descendants().iter().cloned().collect::>(), - $crate::model::declaration::Declaration::Namespace( - $crate::model::declaration::Namespace::SingletonClass(singleton), - ) => singleton.descendants().iter().cloned().collect::>(), - _ => panic!("Tried to get descendants for a declaration that isn't a namespace"), - }; + .get(&parent_id) + .expect("parent declaration exists"); + let ns = parent.as_namespace().expect("parent is a namespace"); + let actual: Vec<_> = ns.immediate_descendants().iter().copied().collect(); for descendant in &$descendants { let descendant_id = $crate::model::ids::DeclarationId::from(*descendant); - assert!( actual.contains(&descendant_id), - "Expected '{}' to be a descendant of '{}'", + "Expected '{}' to be an IMMEDIATE descendant of '{}' (actual: {:?})", $context.graph().declarations().get(&descendant_id).unwrap().name(), - parent.name() + parent.name(), + actual + .iter() + .map(|id| $context.graph().declarations().get(id).unwrap().name()) + .collect::>(), ); } }; diff --git a/test/declaration_test.rb b/test/declaration_test.rb index db735db1..11086a89 100644 --- a/test/declaration_test.rb +++ b/test/declaration_test.rb @@ -245,9 +245,9 @@ class Child < Parent graph.index_all(context.glob("**/*.rb")) graph.resolve - assert_equal(["Child", "Parent"], graph["Parent"].descendants.map(&:name)) - assert_equal(["Child", "Foo"], graph["Foo"].descendants.map(&:name)) - assert_equal(["Child", "Bar"], graph["Bar"].descendants.map(&:name)) + assert_equal(["Child", "Parent"].sort, graph["Parent"].descendants.map(&:name).sort) + assert_equal(["Child", "Foo"].sort, graph["Foo"].descendants.map(&:name).sort) + assert_equal(["Child", "Bar"].sort, graph["Bar"].descendants.map(&:name).sort) end end