Ensure correct clean up for on-demand namespaces

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!(

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<Ancestor> = 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::<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. `<Object>` and `<BasicObject>` are only created to
+                // support `<Foo>`'s ancestor chain and must go when `<Foo>` 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::<Foo>");
         assert_declaration_exists!(context, "Bar::<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::<Foo>");
+        assert_declaration_exists!(context, "Foo::<Foo>#m()");
+        assert_declaration_exists!(context, "Object::<Object>");
+        assert_declaration_exists!(context, "BasicObject::<BasicObject>");
+
+        context.index_uri("file:///foo.rb", "class Foo; def m; end; end");
+        context.resolve();
+
+        assert_declaration_does_not_exist!(context, "Foo::<Foo>");
+        assert_declaration_does_not_exist!(context, "Foo::<Foo>#m()");
+        assert_declaration_does_not_exist!(context, "Object::<Object>");
+        assert_declaration_does_not_exist!(context, "BasicObject::<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::<Foo>");
+
+        context.index_uri("file:///foo.rb", "class Foo; end");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Foo::<Foo>");
+    }
 } // mod incremental_resolution_tests