diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index d821c7d62..c96ea3649 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -349,6 +349,22 @@ impl Declaration { all_declarations!(self, it => it.definition_ids.is_empty()) } + /// Returns true when this declaration is a purely on-demand namespace whose last anchor was just removed: it has no + /// definitions of its own, no members, no singleton child, and no descendants beyond itself. A namespace's + /// `descendants` set always contains itself, so "no real descendants" means len 0 or only self. + #[must_use] + pub fn is_on_demand_orphan(&self, decl_id: DeclarationId) -> bool { + if !self.has_no_definitions() { + return false; + } + + let Some(ns) = self.as_namespace() else { + return false; + }; + + ns.members().is_empty() && ns.singleton_class().is_none() && ns.descendants().iter().all(|id| *id == decl_id) + } + pub fn add_definition(&mut self, definition_id: DefinitionId) { all_declarations!(self, it => { debug_assert!( diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index d866fa107..185b79862 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -1138,6 +1138,7 @@ impl Graph { /// without changing ancestors (e.g. adding a method in a new file). In that case /// the ancestor re-resolution is redundant — a future optimization could skip it /// by tracking why the declaration was seeded. + #[allow(clippy::too_many_lines)] fn invalidate_declaration( &mut self, decl_id: DeclarationId, @@ -1199,6 +1200,10 @@ impl Graph { let unqualified_str_id = StringId::from(&decl.unqualified_name()); let owner_id = *decl.owner_id(); let is_singleton_class = matches!(decl, Declaration::Namespace(Namespace::SingletonClass(_))); + let ancestors_to_detach: Vec = decl + .as_namespace() + .map(|ns| ns.ancestors().iter().copied().collect()) + .unwrap_or_default(); for def_id in def_ids { self.push_work(Unit::Definition(def_id)); @@ -1213,6 +1218,42 @@ impl Graph { ns.remove_member(&unqualified_str_id); } } + + // If the owner was purely on-demand (no definitions of its own) and this removal left it with nothing, + // queue it for cleanup. Typical case: a singleton class like `Foo::` is created only to host `def + // self.m`; removing `m` leaves it empty. + if self + .declarations + .get(&owner_id) + .is_some_and(|d| d.is_on_demand_orphan(owner_id)) + { + queue.push(InvalidationItem::Declaration(owner_id)); + } + + // Detach from each complete ancestor's descendant set so we don't leave a stale id in descendants. + // If an ancestor was only kept alive on-demand (no definitions of its own) and now has no + // remaining descendants, it is orphaned and must be removed too. This cascades through + // implicit singleton-class chains: e.g. `` and `` are only created to + // support ``'s ancestor chain and must go when `` goes. + for ancestor in ancestors_to_detach { + let Ancestor::Complete(ancestor_id) = ancestor else { + continue; + }; + + if let Some(anc_decl) = self.declarations.get_mut(&ancestor_id) + && let Some(ns) = anc_decl.as_namespace_mut() + { + ns.remove_descendant(&decl_id); + } + + if self + .declarations + .get(&ancestor_id) + .is_some_and(|d| d.is_on_demand_orphan(ancestor_id)) + { + queue.push(InvalidationItem::Declaration(ancestor_id)); + } + } } self.declarations.remove(&decl_id); @@ -2393,6 +2434,7 @@ mod tests { #[cfg(test)] mod incremental_resolution_tests { + use crate::model::ids::DeclarationId; use crate::model::name::NameRef; use crate::test_utils::GraphTest; use crate::{ @@ -4020,4 +4062,85 @@ mod incremental_resolution_tests { assert_declaration_exists!(context, "Foo::"); assert_declaration_exists!(context, "Bar::"); } + + #[test] + fn constant_references_through_object_inheritance_are_invalidated() { + let mut context = GraphTest::new(); + context.index_uri("file:///a.rb", "class Foo; end"); + context.index_uri( + "file:///b.rb", + r" + module Wrap + class Foo; end + ::Foo + Foo + end + ", + ); + context.resolve(); + + context.delete_uri("file:///a.rb"); + context.resolve(); + + let kernel = context + .graph() + .declarations() + .get(&DeclarationId::from("Kernel")) + .unwrap(); + + for id in kernel.as_namespace().unwrap().descendants() { + assert!( + context.graph().declarations().contains_key(id), + "Kernel has stale descendant id {id:?} with no backing declaration" + ); + } + } + + #[test] + fn singleton_class_not_cleaned_up_when_def_self_removed() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", "class Foo; def self.m; end; end"); + context.resolve(); + + assert_declaration_exists!(context, "Foo::"); + assert_declaration_exists!(context, "Foo::#m()"); + assert_declaration_exists!(context, "Object::"); + assert_declaration_exists!(context, "BasicObject::"); + + context.index_uri("file:///foo.rb", "class Foo; def m; end; end"); + context.resolve(); + + assert_declaration_does_not_exist!(context, "Foo::"); + assert_declaration_does_not_exist!(context, "Foo::#m()"); + assert_declaration_does_not_exist!(context, "Object::"); + assert_declaration_does_not_exist!(context, "BasicObject::"); + } + + #[test] + fn singleton_classes_are_not_cleaned_up_when_there_are_still_references() { + let mut context = GraphTest::new(); + context.index_uri( + "file:///foo.rb", + " + class Foo + class << self + end + end + ", + ); + context.index_uri( + "file:///foo2.rb", + " + Foo.methods + ", + ); + context.resolve(); + + assert_declaration_exists!(context, "Foo::"); + + context.index_uri("file:///foo.rb", "class Foo; end"); + context.resolve(); + + assert_declaration_exists!(context, "Foo::"); + } } // mod incremental_resolution_tests