From 83c64843ee40504332e23721e0844e3f310ebd3c Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Tue, 27 Jan 2026 11:56:45 -0500 Subject: [PATCH 1/3] Share the unit queue through the resolver Instead of passing it around. Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/resolution.rs | 94 +++++++++++++++------------------- 1 file changed, 42 insertions(+), 52 deletions(-) diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 1308b5eb..3aafb5ff 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -65,11 +65,19 @@ impl LinearizationContext { pub struct Resolver<'a> { graph: &'a mut Graph, + /// Contains all units of work for resolution, sorted in order for resolution (less complex constant names first) + unit_queue: VecDeque, + /// Whether we made any progress in the last pass of the resolution loop + made_progress: bool, } impl<'a> Resolver<'a> { pub fn new(graph: &'a mut Graph) -> Self { - Self { graph } + Self { + graph, + unit_queue: VecDeque::new(), + made_progress: false, + } } /// Runs the resolution phase on the graph. The resolution phase is when 4 main pieces of information are computed: @@ -112,35 +120,35 @@ impl<'a> Resolver<'a> { ); } - let (mut unit_queue, other_ids) = self.sorted_units(); + let other_ids = self.prepare_units(); loop { // Flag to ensure the end of the resolution loop. We go through all items in the queue based on its current // length. If we made any progress in this pass of the queue, we can continue because we're unlocking more work // to be done - let mut made_progress = false; + self.made_progress = false; // Loop through the current length of the queue, which won't change during this pass. Retries pushed to the back // are only processed in the next pass, so that we can assess whether we made any progress - for _ in 0..unit_queue.len() { - let Some(unit_id) = unit_queue.pop_front() else { + for _ in 0..self.unit_queue.len() { + let Some(unit_id) = self.unit_queue.pop_front() else { break; }; match unit_id { Unit::Definition(id) => { - self.handle_definition_unit(&mut unit_queue, &mut made_progress, unit_id, id); + self.handle_definition_unit(unit_id, id); } Unit::Reference(id) => { - self.handle_reference_unit(&mut unit_queue, &mut made_progress, unit_id, id); + self.handle_reference_unit(unit_id, id); } Unit::Ancestors(id) => { - self.handle_ancestor_unit(&mut unit_queue, &mut made_progress, id); + self.handle_ancestor_unit(id); } } } - if !made_progress || unit_queue.is_empty() { + if !self.made_progress || self.unit_queue.is_empty() { break; } } @@ -158,13 +166,7 @@ impl<'a> Resolver<'a> { } /// Handles a unit of work for resolving a constant definition - fn handle_definition_unit( - &mut self, - unit_queue: &mut VecDeque, - made_progress: &mut bool, - unit_id: Unit, - id: DefinitionId, - ) { + fn handle_definition_unit(&mut self, unit_id: Unit, id: DefinitionId) { let outcome = match self.graph.definitions().get(&id).unwrap() { Definition::Class(class) => { self.handle_constant_declaration(*class.name_id(), id, false, |name, owner_id| { @@ -199,81 +201,72 @@ impl<'a> Resolver<'a> { match outcome { Outcome::Retry => { // There might be dependencies we haven't figured out yet, so we need to retry - unit_queue.push_back(unit_id); + self.unit_queue.push_back(unit_id); } Outcome::Unresolved(None) => { // We couldn't resolve this name. Emit a diagnostic } Outcome::Unresolved(Some(id_needing_linearization)) => { - unit_queue.push_back(unit_id); - unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); + self.unit_queue.push_back(unit_id); + self.unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); } Outcome::Resolved(id, None) => { - unit_queue.push_back(Unit::Ancestors(id)); - *made_progress = true; + self.unit_queue.push_back(Unit::Ancestors(id)); + self.made_progress = true; } Outcome::Resolved(_, Some(id_needing_linearization)) => { - unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); - *made_progress = true; + self.unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); + self.made_progress = true; } } } /// Handles a unit of work for resolving a constant reference - fn handle_reference_unit( - &mut self, - unit_queue: &mut VecDeque, - made_progress: &mut bool, - unit_id: Unit, - id: ReferenceId, - ) { + fn handle_reference_unit(&mut self, unit_id: Unit, id: ReferenceId) { let constant_ref = self.graph.constant_references().get(&id).unwrap(); match self.resolve_constant_internal(*constant_ref.name_id()) { Outcome::Retry => { // There might be dependencies we haven't figured out yet, so we need to retry - unit_queue.push_back(unit_id); + self.unit_queue.push_back(unit_id); } Outcome::Unresolved(None) => { // We couldn't resolve this name. Emit a diagnostic } Outcome::Unresolved(Some(id_needing_linearization)) => { - unit_queue.push_back(unit_id); - unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); + self.unit_queue.push_back(unit_id); + self.unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); } Outcome::Resolved(declaration_id, None) => { self.graph.record_resolved_reference(id, declaration_id); - *made_progress = true; + self.made_progress = true; } Outcome::Resolved(resolved_id, Some(id_needing_linearization)) => { self.graph.record_resolved_reference(id, resolved_id); - *made_progress = true; - unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); + self.made_progress = true; + self.unit_queue.push_back(Unit::Ancestors(id_needing_linearization)); } } } /// Handles a unit of work for linearizing ancestors of a declaration - fn handle_ancestor_unit(&mut self, unit_queue: &mut VecDeque, made_progress: &mut bool, id: DeclarationId) { + fn handle_ancestor_unit(&mut self, id: DeclarationId) { match self.ancestors_of(id) { Ancestors::Complete(_) | Ancestors::Cyclic(_) => { // We succeeded in some capacity this time - *made_progress = true; + self.made_progress = true; } Ancestors::Partial(_) => { // We still couldn't linearize ancestors, but there's a chance that this will succeed next time. We // re-enqueue for another try, but we don't consider it as making progress - unit_queue.push_back(Unit::Ancestors(id)); + self.unit_queue.push_back(Unit::Ancestors(id)); } } } /// Handle other definitions that don't require resolution, but need to have their declarations and membership created #[allow(clippy::too_many_lines)] - fn handle_remaining_definitions( - &mut self, - other_ids: Vec>, - ) { + fn handle_remaining_definitions(&mut self, other_ids: Vec) { for id in other_ids { match self.graph.definitions().get(&id).unwrap() { Definition::Method(method_definition) => { @@ -1304,11 +1297,7 @@ impl<'a> Resolver<'a> { parent_depth + nesting_depth + 1 } - /// Returns a tuple of 2 vectors: - /// - The first one contains all constants, sorted in order for resolution (less complex constant names first) - /// - The second one contains all other definitions, in no particular order - #[must_use] - fn sorted_units(&self) -> (VecDeque, Vec) { + fn prepare_units(&mut self) -> Vec { let estimated_length = self.graph.definitions().len() / 2; let mut definitions = Vec::with_capacity(estimated_length); let mut others = Vec::with_capacity(estimated_length); @@ -1379,12 +1368,13 @@ impl<'a> Resolver<'a> { (Self::name_depth(name_a, names), uri_a, offset_a).cmp(&(Self::name_depth(name_b, names), uri_b, offset_b)) }); - let mut units = definitions.into_iter().map(|(id, _)| id).collect::>(); - units.extend(references.into_iter().map(|(id, _)| id).collect::>()); + self.unit_queue + .extend(definitions.into_iter().map(|(id, _)| id).collect::>()); + self.unit_queue + .extend(references.into_iter().map(|(id, _)| id).collect::>()); others.shrink_to_fit(); - - (units, others) + others } /// Returns the singleton parent ID for an attached object ID. A singleton class' parent depends on what the attached From d036e9575c4ad68d6f2a0b4ba7ff1395e3f58fab Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Tue, 27 Jan 2026 10:53:23 -0500 Subject: [PATCH 2/3] Introduce TodoDeclaration Used to represent namespaces we couldn't resolve but are used as parent scope of another namespace. Consider this: ```rb class Foo::Bar def bar; end end ``` While we can't resolve `Foo`, we want to create `def bar` in the right namespace: `Foo::Bar`. Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/model/declaration.rs | 5 +++++ rust/rubydex/src/model/graph.rs | 1 + 2 files changed, 6 insertions(+) diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index 3ab975de..f6c1ae88 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -58,6 +58,7 @@ macro_rules! all_declarations { Declaration::Namespace(Namespace::Class($var)) => $expr, Declaration::Namespace(Namespace::Module($var)) => $expr, Declaration::Namespace(Namespace::SingletonClass($var)) => $expr, + Declaration::Namespace(Namespace::Todo($var)) => $expr, Declaration::Constant($var) => $expr, Declaration::ConstantAlias($var) => $expr, Declaration::Method($var) => $expr, @@ -74,6 +75,7 @@ macro_rules! all_namespaces { Namespace::Class($var) => $expr, Namespace::Module($var) => $expr, Namespace::SingletonClass($var) => $expr, + Namespace::Todo($var) => $expr, } }; } @@ -372,6 +374,7 @@ pub enum Namespace { Class(Box), SingletonClass(Box), Module(Box), + Todo(Box), } impl Namespace { @@ -381,6 +384,7 @@ impl Namespace { Namespace::Class(_) => "Class", Namespace::SingletonClass(_) => "SingletonClass", Namespace::Module(_) => "Module", + Namespace::Todo(_) => "", } } @@ -473,6 +477,7 @@ impl Namespace { namespace_declaration!(Class, ClassDeclaration); namespace_declaration!(Module, ModuleDeclaration); namespace_declaration!(SingletonClass, SingletonClassDeclaration); +namespace_declaration!(Todo, TodoDeclaration); simple_declaration!(ConstantDeclaration); simple_declaration!(MethodDeclaration); simple_declaration!(GlobalVariableDeclaration); diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 16b7c9e2..67e8d3ff 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -459,6 +459,7 @@ impl Graph { Declaration::Namespace(Namespace::SingletonClass(it)) => { it.add_member(member_str_id, member_declaration_id); } + Declaration::Namespace(Namespace::Todo(it)) => it.add_member(member_str_id, member_declaration_id), Declaration::Constant(_) => { // TODO: temporary hack to avoid crashing on `Struct.new`, `Class.new` and `Module.new` } From 6259183023f5f80de506a80f2e340af3c8a2b31e Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Tue, 27 Jan 2026 14:39:13 -0500 Subject: [PATCH 3/3] Create Todo declaration for unresolved owners Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/model/graph.rs | 6 +- rust/rubydex/src/resolution.rs | 124 +++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 31 deletions(-) diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 67e8d3ff..96fda289 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -232,10 +232,8 @@ impl Graph { }?; self.declarations - .get(nesting_declaration_id) - .unwrap() - .as_namespace() - .unwrap() + .get(nesting_declaration_id)? + .as_namespace()? .member(member_str_id) } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 3aafb5ff..e8e2113e 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -7,7 +7,7 @@ use crate::model::{ declaration::{ Ancestor, Ancestors, ClassDeclaration, ClassVariableDeclaration, ConstantAliasDeclaration, ConstantDeclaration, Declaration, GlobalVariableDeclaration, InstanceVariableDeclaration, MethodDeclaration, ModuleDeclaration, - Namespace, SingletonClassDeclaration, + Namespace, SingletonClassDeclaration, TodoDeclaration, }, definitions::{Definition, Mixin}, graph::{CLASS_ID, Graph, MODULE_ID, OBJECT_ID}, @@ -69,6 +69,8 @@ pub struct Resolver<'a> { unit_queue: VecDeque, /// Whether we made any progress in the last pass of the resolution loop made_progress: bool, + /// Allow the resolver to create TODO declarations for unresolved constants + create_todo_declarations: bool, } impl<'a> Resolver<'a> { @@ -77,6 +79,7 @@ impl<'a> Resolver<'a> { graph, unit_queue: VecDeque::new(), made_progress: false, + create_todo_declarations: false, } } @@ -122,6 +125,17 @@ impl<'a> Resolver<'a> { let other_ids = self.prepare_units(); + self.process_queue(); + + // Once we've processed the queue and create all we could, what's remaining is depending on other unresolved units. + // We'll create TODO declarations for these and process the queue again to resolve what we can. + self.create_todo_declarations = true; + self.process_queue(); + + self.handle_remaining_definitions(other_ids); + } + + fn process_queue(&mut self) { loop { // Flag to ensure the end of the resolution loop. We go through all items in the queue based on its current // length. If we made any progress in this pass of the queue, we can continue because we're unlocking more work @@ -152,8 +166,6 @@ impl<'a> Resolver<'a> { break; } } - - self.handle_remaining_definitions(other_ids); } /// Resolves a single constant against the graph. This method is not meant to be used by the resolution phase, but by @@ -205,6 +217,7 @@ impl<'a> Resolver<'a> { } Outcome::Unresolved(None) => { // We couldn't resolve this name. Emit a diagnostic + self.unit_queue.push_back(unit_id); } Outcome::Unresolved(Some(id_needing_linearization)) => { self.unit_queue.push_back(unit_id); @@ -957,12 +970,47 @@ impl<'a> Resolver<'a> { fn name_owner_id(&mut self, name_id: NameId) -> Outcome { let name_ref = self.graph.names().get(&name_id).unwrap(); - if let Some(parent_scope) = name_ref.parent_scope() { + if let Some(parent_scope) = *name_ref.parent_scope() { // If we have `A::B`, the owner of `B` is whatever `A` resolves to. // If `A` is an alias, resolve through to get the actual namespace. - match self.resolve_constant_internal(*parent_scope) { + match self.resolve_constant_internal(parent_scope) { Outcome::Resolved(id, linearization) => self.resolve_to_primary_namespace(id, linearization), - other => other, + other => { + let parent_name = self.graph.names().get(&parent_scope).unwrap(); + let parent_str_id = *parent_name.str(); + + if !self.create_todo_declarations { + return other; + } + + let parent_owner_id = match self.name_owner_id(parent_scope) { + Outcome::Resolved(id, _) => id, + _ => *OBJECT_ID, + }; + + let fully_qualified_name = if parent_owner_id == *OBJECT_ID { + self.graph.strings().get(&parent_str_id).unwrap().to_string() + } else { + format!( + "{}::{}", + self.graph.declarations().get(&parent_owner_id).unwrap().name(), + self.graph.strings().get(&parent_str_id).unwrap().as_str() + ) + }; + + let declaration_id = DeclarationId::from(&fully_qualified_name); + + self.graph.declarations_mut().insert( + declaration_id, + Declaration::Namespace(Namespace::Todo(Box::new(TodoDeclaration::new( + fully_qualified_name, + parent_owner_id, + )))), + ); + self.graph.add_member(&parent_owner_id, declaration_id, parent_str_id); + + Outcome::Resolved(declaration_id, None) + } } } else if let Some(nesting_id) = name_ref.nesting() { // Lexical nesting from block structure, e.g.: @@ -1711,29 +1759,13 @@ mod tests { " }); context.resolve(); - assert!( - context - .graph() - .declarations() - .get(&DeclarationId::from("Foo")) - .is_none() - ); - assert!( - context - .graph() - .declarations() - .get(&DeclarationId::from("Foo::Bar")) - .is_none() - ); - assert!( - context - .graph() - .declarations() - .get(&DeclarationId::from("Foo::Bar::Baz")) - .is_none() - ); assert_no_diagnostics!(&context); + + assert_members_eq!(context, "Object", vec!["Foo"]); + assert_members_eq!(context, "Foo", vec!["Bar"]); + assert_members_eq!(context, "Foo::Bar", vec!["Baz"]); + assert_no_members!(context, "Foo::Bar::Baz"); } #[test] @@ -4088,4 +4120,42 @@ mod tests { assert_no_members!(context, "Foo"); assert_members_eq!(context, "Bar::Foo", vec!["FOO"]); } + + #[test] + fn resolve_missing_declaration_to_todo() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + class Foo::Bar + include Foo::Baz + + def bar; end + end + + module Foo::Baz + def baz; end + end + " + }); + context.resolve(); + + assert_no_diagnostics!(&context); + + println!( + "{:?}", + context + .graph() + .declarations() + .values() + .collect::>() + .iter() + .map(|d| d.name()) + .collect::>() + ); + + assert_members_eq!(context, "Object", vec!["Foo"]); + assert_members_eq!(context, "Foo", vec!["Bar", "Baz"]); + assert_members_eq!(context, "Foo::Bar", vec!["bar()"]); + assert_members_eq!(context, "Foo::Baz", vec!["baz()"]); + } }