From 145bbcaad855af1f5f05d19b1efaeaf728fad52f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Efe=20G=C3=B6kdemir?= Date: Sun, 25 Oct 2026 18:36:06 +0300 Subject: [PATCH] fix(resolution): refresh descendants on namespace promotion --- docs/ruby-behaviors.md | 17 ++++++++++ rust/rubydex/src/model/declaration.rs | 3 ++ rust/rubydex/src/model/graph.rs | 25 +++++++++++++++ rust/rubydex/src/resolution.rs | 46 +++++++++++++++++++++++++++ rust/rubydex/src/resolution_tests.rs | 44 +++++++++++++++++++++++++ 5 files changed, 135 insertions(+) diff --git a/docs/ruby-behaviors.md b/docs/ruby-behaviors.md index cc2451e0d..afce0fd78 100644 --- a/docs/ruby-behaviors.md +++ b/docs/ruby-behaviors.md @@ -157,6 +157,23 @@ This creates: **Important:** `Foo::Bar::Baz::Qux::Quuux` does **not** exist because `::Quuux` resets to top level. +A compact class definition does not add each qualified prefix to lexical nesting. For example, `Module.nesting` inside +`class Outer::Base::Child` contains only `Outer::Base::Child`. Unqualified constants are searched through that class's +ancestors after lexical lookup, so a top-level constant can be found through `Object` when the class inherits from +`Outer::Base`: + +```ruby +module Outer; end +class Record; LIMIT = 4; end +class Outer::Base; end + +class Outer::Base::Child < Outer::Base + def limit + Record::LIMIT # Record is found through Outer::Base and Object + end +end +``` + ## Definitions vs Declarations A fundamental concept in Ruby is that classes and modules can be reopened and defined multiple times. diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index fa4d6db83..f72490b32 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -94,6 +94,9 @@ macro_rules! namespace_declaration { match other { Declaration::Namespace(namespace) => { self.namespace_store.members.extend(namespace.members()); + self.namespace_store + .descendants + .extend(namespace.descendants().iter().copied()); self.core.references.extend(namespace.references()); } Declaration::Constant(it) | Declaration::ConstantAlias(it) => { diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 5d02610a1..a5efea3f4 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -717,6 +717,31 @@ impl Graph { std::mem::take(&mut self.pending_work) } + /// Invalidates a declaration's descendants, returning the work created by that invalidation. + /// + /// Unlike graph updates, callers may need to process this work in the current resolution cycle. + pub(crate) fn invalidate_descendants_and_collect_work(&mut self, declaration_id: DeclarationId) -> Vec { + let descendants: Vec<_> = self + .declarations + .get(&declaration_id) + .and_then(Declaration::as_namespace) + .map(|namespace| { + namespace + .descendants() + .iter() + .copied() + .filter(|descendant_id| *descendant_id != declaration_id) + .collect() + }) + .unwrap_or_default(); + let pending_work_len = self.pending_work.len(); + self.invalidate_graph( + descendants.into_iter().map(InvalidationItem::Declaration).collect(), + IdentityHashMap::default(), + ); + self.pending_work.split_off(pending_work_len) + } + pub(crate) fn push_work(&mut self, unit: Unit) { self.pending_work.push(unit); } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 44dc40dc3..8c6165c50 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -315,6 +315,39 @@ impl<'a> Resolver<'a> { } } + /// Adds work created during resolution without duplicating units that are already queued for retry. + fn extend_unique_units(&mut self, units: impl IntoIterator) { + let mut definitions = IdentityHashSet::default(); + let mut references = IdentityHashSet::default(); + let mut ancestors = IdentityHashSet::default(); + + for unit in &self.unit_queue { + match unit { + Unit::Definition(id) => { + definitions.insert(*id); + } + Unit::ConstantRef(id) => { + references.insert(*id); + } + Unit::Ancestors(id) => { + ancestors.insert(*id); + } + } + } + + for unit in units { + let is_new = match &unit { + Unit::Definition(id) => definitions.insert(*id), + Unit::ConstantRef(id) => references.insert(*id), + Unit::Ancestors(id) => ancestors.insert(*id), + }; + + if is_new { + self.unit_queue.push_back(unit); + } + } + } + /// 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) { @@ -1311,6 +1344,15 @@ impl<'a> Resolver<'a> { fully_qualified_name.insert_str(0, owner.name()); } + let declaration_id = DeclarationId::from(&fully_qualified_name); + let promotes_todo = matches!( + self.graph.definitions().get(&definition_id), + Some(Definition::Class(_) | Definition::Module(_) | Definition::SingletonClass(_)) + ) && matches!( + self.graph.declarations().get(&declaration_id), + Some(Declaration::Namespace(Namespace::Todo(_))) + ); + let declaration_id = self.graph .add_declaration(definition_id, fully_qualified_name, |fully_qualified_name| { @@ -1332,6 +1374,10 @@ impl<'a> Resolver<'a> { } self.graph.record_resolved_name(name_id, declaration_id); + if promotes_todo { + let work = self.graph.invalidate_descendants_and_collect_work(declaration_id); + self.extend_unique_units(work); + } Outcome::Resolved(declaration_id) } other => other, diff --git a/rust/rubydex/src/resolution_tests.rs b/rust/rubydex/src/resolution_tests.rs index e810aac31..094c440cb 100644 --- a/rust/rubydex/src/resolution_tests.rs +++ b/rust/rubydex/src/resolution_tests.rs @@ -20,6 +20,50 @@ fn graph_test() -> GraphTest { mod constant_resolution_tests { use super::*; + fn nested_in_superclass(explicit_namespace: bool, superclass: &str) -> GraphTest { + let mut context = graph_test(); + context.index_uri("file:///record.rb", "class Record\n LIMIT = 4\nend\n"); + if explicit_namespace { + context.index_uri("file:///outer.rb", "module Outer\nend\n"); + } + context.index_uri("file:///base.rb", "class Outer::Base\nend\n"); + context.index_uri("file:///other.rb", "class Outer::Other\nend\n"); + context.index_uri( + "file:///child.rb", + &format!("class Outer::Base::Child < {superclass}\n def limit\n Record::LIMIT\n end\nend\n"), + ); + context.resolve(); + context + } + + #[test] + fn resolves_top_level_constant_in_class_nested_under_its_superclass_with_implicit_namespace() { + let context = nested_in_superclass(false, "Outer::Base"); + assert_ancestors_eq!( + context, + "Outer::Base", + ["Outer::Base", "Object", "Kernel", "BasicObject"] + ); + assert_ancestors_eq!( + context, + "Outer::Base::Child", + ["Outer::Base::Child", "Outer::Base", "Object", "Kernel", "BasicObject"] + ); + assert_constant_reference_to!(context, "Record", "file:///child.rb:3:5-3:11"); + } + + #[test] + fn resolves_top_level_constant_in_class_nested_under_its_superclass_with_explicit_namespace() { + let context = nested_in_superclass(true, "Outer::Base"); + assert_constant_reference_to!(context, "Record", "file:///child.rb:3:5-3:11"); + } + + #[test] + fn resolves_top_level_constant_in_class_nested_under_sibling_superclass() { + let context = nested_in_superclass(false, "Outer::Other"); + assert_constant_reference_to!(context, "Record", "file:///child.rb:3:5-3:11"); + } + #[test] fn resolving_top_level_references() { let mut context = graph_test();