diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index b7c761b1f..2d2f447b2 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::diagnostic::{Diagnostic, Rule}; use crate::model::{ @@ -51,11 +48,32 @@ enum SingletonAncestors { Deferred, } +#[derive(Default)] +struct ChainState { + cyclic: bool, + partial: bool, +} + +impl ChainState { + /// Fold a nested linearization's result into this frame's state, returning its ancestor list + fn fold(&mut self, ancestors: Ancestors) -> Vec { + match ancestors { + Ancestors::Complete(ids) => ids, + Ancestors::Cyclic(ids) => { + self.cyclic = true; + ids + } + Ancestors::Partial(ids) => { + self.partial = true; + ids + } + } + } +} + struct LinearizationContext { descendants: IdentityHashSet, seen_ids: IdentityHashSet, - cyclic: bool, - partial: bool, } impl LinearizationContext { @@ -63,8 +81,6 @@ impl LinearizationContext { Self { descendants: IdentityHashSet::default(), seen_ids: IdentityHashSet::default(), - cyclic: false, - partial: false, } } @@ -83,6 +99,8 @@ pub struct Resolver<'a> { unit_queue: VecDeque, /// Whether we made any progress in the last pass of the resolution loop made_progress: bool, + /// The linearization context carried across recursion + context: LinearizationContext, } impl<'a> Resolver<'a> { @@ -91,6 +109,7 @@ impl<'a> Resolver<'a> { graph, unit_queue: VecDeque::new(), made_progress: false, + context: LinearizationContext::new(), } } @@ -257,7 +276,7 @@ impl<'a> Resolver<'a> { /// Handles a unit of work for linearizing ancestors of a declaration fn handle_ancestor_unit(&mut self, id: DeclarationId) { - match self.ancestors_of(id) { + match self.linearize_ancestors(id) { Ancestors::Complete(_) | Ancestors::Cyclic(_) => { // We succeeded in some capacity this time self.made_progress = true; @@ -928,7 +947,7 @@ impl<'a> Resolver<'a> { fn schedule_singleton_ancestors(&mut self, id: DeclarationId, mode: SingletonAncestors) { match mode { SingletonAncestors::Eager => { - let _ = self.ancestors_of(id); + let _ = self.linearize_ancestors(id); } SingletonAncestors::Enqueue => { self.unit_queue.push_back(Unit::Ancestors(id)); @@ -945,35 +964,24 @@ impl<'a> Resolver<'a> { /// /// Can panic if there's inconsistent data in the graph #[must_use] - fn ancestors_of(&mut self, declaration_id: DeclarationId) -> Ancestors { - let mut context = LinearizationContext::new(); - self.linearize_ancestors(declaration_id, &mut context) - } - - /// Linearizes the ancestors of a declaration, returning the list of ancestor declaration IDs - /// - /// # Panics - /// - /// Can panic if there's inconsistent data in the graph - #[must_use] - fn linearize_ancestors(&mut self, declaration_id: DeclarationId, context: &mut LinearizationContext) -> Ancestors { + fn linearize_ancestors(&mut self, declaration_id: DeclarationId) -> Ancestors { { 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); + self.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); + self.propagate_descendants(&cached); - context.finalize(declaration_id); + self.context.finalize(declaration_id); return cached; } - if !context.seen_ids.insert(declaration_id) { + if !self.context.seen_ids.insert(declaration_id) { // If we find a cycle when linearizing ancestors, it's an error that the programmer must fix. However, we try to // still approximate features by assuming that it must inherit from `Object` at some point (which is what most // classes/modules inherit from). This is not 100% correct, but it allows us to provide a bit better IDE support @@ -988,19 +996,20 @@ impl<'a> Resolver<'a> { .unwrap() .set_ancestors(estimated_ancestors.clone()); - context.finalize(declaration_id); + self.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 let namespace = declaration.as_namespace_mut().unwrap(); - for descendant in &context.descendants { + for descendant in &self.context.descendants { namespace.add_descendant(*descendant); } } - let parent_ancestors = self.linearize_parent_ancestors(declaration_id, context); + let mut state = ChainState::default(); + let parent_ancestors = self.linearize_parent_ancestors(declaration_id, &mut state); let declaration = self.graph.declarations().get(&declaration_id).unwrap(); let mut mixins = Vec::new(); @@ -1038,7 +1047,7 @@ impl<'a> Resolver<'a> { } let (linearized_prepends, linearized_includes) = - self.linearize_mixins(context, mixins, parent_ancestors.as_ref()); + self.linearize_mixins(mixins, parent_ancestors.as_ref(), &mut state); // Build the final list let mut ancestors = Vec::new(); @@ -1049,9 +1058,9 @@ impl<'a> Resolver<'a> { ancestors.extend(parents); } - let result = if context.cyclic { + let result = if state.cyclic { Ancestors::Cyclic(ancestors) - } else if context.partial { + } else if state.partial { Ancestors::Partial(ancestors) } else { Ancestors::Complete(ancestors) @@ -1065,50 +1074,32 @@ impl<'a> Resolver<'a> { .unwrap() .set_ancestors(result.clone()); - context.finalize(declaration_id); + self.context.finalize(declaration_id); result } fn linearize_parent_ancestors( &mut self, declaration_id: DeclarationId, - context: &mut LinearizationContext, + state: &mut ChainState, ) -> Option> { let declaration = self.graph.declarations().get(&declaration_id).unwrap(); match declaration { Declaration::Namespace(Namespace::Class(_)) => { - Some(match self.linearize_superclass(declaration_id, context)? { - Ancestors::Complete(ids) => ids, - Ancestors::Cyclic(ids) => { - context.cyclic = true; - ids - } - Ancestors::Partial(ids) => { - context.partial = true; - ids - } - }) + let superclass = self.linearize_superclass(declaration_id, state)?; + Some(state.fold(superclass)) } Declaration::Namespace(Namespace::SingletonClass(_)) => { let owner_id = *declaration.owner_id(); let (singleton_parent_id, partial_singleton) = self.singleton_parent_id(owner_id); if partial_singleton { - context.partial = true; + state.partial = true; } - Some(match self.linearize_ancestors(singleton_parent_id, context) { - Ancestors::Complete(ids) => ids, - Ancestors::Cyclic(ids) => { - context.cyclic = true; - ids - } - Ancestors::Partial(ids) => { - context.partial = true; - ids - } - }) + let parent = self.linearize_ancestors(singleton_parent_id); + Some(state.fold(parent)) } _ => None, } @@ -1118,9 +1109,9 @@ impl<'a> Resolver<'a> { /// modules are deduplicated against them fn linearize_mixins( &mut self, - context: &mut LinearizationContext, mixins: Vec, parent_ancestors: Option<&Vec>, + state: &mut ChainState, ) -> (VecDeque, VecDeque) { let mut linearized_prepends = VecDeque::new(); let mut linearized_includes = VecDeque::new(); @@ -1143,17 +1134,8 @@ impl<'a> Resolver<'a> { continue; }; - let ids = match self.linearize_ancestors(module_id, context) { - Ancestors::Complete(ids) => ids, - Ancestors::Cyclic(ids) => { - context.cyclic = true; - ids - } - Ancestors::Partial(ids) => { - context.partial = true; - ids - } - }; + let module_ancestors = self.linearize_ancestors(module_id); + let ids = state.fold(module_ancestors); // Only reorder if there are new modules to add. If all modules being // prepended are already in the chain (e.g., `prepend A` when A is already @@ -1170,7 +1152,7 @@ impl<'a> Resolver<'a> { NameRef::Unresolved(_) => { // We haven't been able to resolve this name yet, so we push it as a partial linearization to finish // later - context.partial = true; + state.partial = true; linearized_prepends.push_front(Ancestor::Partial(*constant_reference.name_id())); } } @@ -1182,17 +1164,8 @@ impl<'a> Resolver<'a> { continue; }; - let mut ids = match self.linearize_ancestors(module_id, context) { - Ancestors::Complete(ids) => ids, - Ancestors::Cyclic(ids) => { - context.cyclic = true; - ids - } - Ancestors::Partial(ids) => { - context.partial = true; - ids - } - }; + let module_ancestors = self.linearize_ancestors(module_id); + let mut ids = state.fold(module_ancestors); // Prepended module are deduped based only on other prepended modules ids.retain(|id| { @@ -1210,7 +1183,7 @@ impl<'a> Resolver<'a> { NameRef::Unresolved(_) => { // We haven't been able to resolve this name yet, so we push it as a partial linearization to finish // later - context.partial = true; + state.partial = true; linearized_includes.push_front(Ancestor::Partial(*constant_reference.name_id())); } } @@ -1222,11 +1195,9 @@ impl<'a> Resolver<'a> { } /// Propagate descendants to all cached ancestors - fn propagate_descendants( - &mut self, - descendants: &mut HashSet, - cached: &Ancestors, - ) { + fn propagate_descendants(&mut self, cached: &Ancestors) { + let descendants = &self.context.descendants; + if !descendants.is_empty() { for ancestor in cached { if let Ancestor::Complete(ancestor_id) = ancestor { @@ -1238,7 +1209,7 @@ impl<'a> Resolver<'a> { .as_namespace_mut() .unwrap(); - for descendant in descendants.iter() { + for descendant in descendants { namespace.add_descendant(*descendant); } } @@ -1749,7 +1720,7 @@ impl<'a> Resolver<'a> { /// Search for a member in a declaration's ancestor chain. fn search_ancestors(&mut self, declaration_id: DeclarationId, str_id: StringId) -> Outcome { - match self.ancestors_of(declaration_id) { + match self.linearize_ancestors(declaration_id) { Ancestors::Complete(ids) | Ancestors::Cyclic(ids) => ids .iter() .find_map(|ancestor_id| { @@ -2064,16 +2035,12 @@ impl<'a> Resolver<'a> { )) } - fn linearize_superclass( - &mut self, - declaration_id: DeclarationId, - context: &mut LinearizationContext, - ) -> Option { + fn linearize_superclass(&mut self, declaration_id: DeclarationId, state: &mut ChainState) -> Option { let (superclass_id, unresolved_superclass) = self.get_superclass(declaration_id)?; - let mut result = self.linearize_ancestors(superclass_id, context); + let mut result = self.linearize_ancestors(superclass_id); if let Some(name_id) = unresolved_superclass { - context.partial = true; + state.partial = true; // Insert the unresolved superclass as a Partial ancestor at the front of the chain, so it // appears before the default Object ancestors diff --git a/rust/rubydex/src/resolution_tests.rs b/rust/rubydex/src/resolution_tests.rs index 09afad7a3..b053a7028 100644 --- a/rust/rubydex/src/resolution_tests.rs +++ b/rust/rubydex/src/resolution_tests.rs @@ -1494,6 +1494,43 @@ mod include_tests { assert_descendants!(context, "Bar", ["Baz"]); assert_descendants!(context, "Foo", ["Bar", "Baz"]); } + + #[test] + fn unresolved_include_followed_by_resolved_include_stays_partial() { + let mut context = graph_test(); + context.index_uri("file:///foo.rb", { + r" + module M + end + + class C + include Missing + include M + end + " + }); + context.resolve(); + + assert_ancestors_eq!( + context, + "C", + ["C", "M", Partial("Missing"), "Object", "Kernel", "BasicObject"] + ); + assert!( + matches!( + context + .graph() + .declarations() + .get(&DeclarationId::from("C")) + .unwrap() + .as_namespace() + .unwrap() + .ancestors(), + Ancestors::Partial(_) + ), + "C should have a partial chain: the `Missing` include never resolves" + ); + } } mod prepend_tests {