Defer cleanup of declarations emptied during invalidation

During invalidation, declarations can lose all their definitions via two
paths that don't immediately remove them:

1. Name cascade (unresolve_dependent_name): a surviving file's definition
   is detached and re-queued — the declaration may be repopulated after
   re-resolution.

2. Document data cleanup (remove_document_data): the broad retain sweep
   detaches definitions that definition_to_declaration_id can't map back
   (e.g. singleton members like def self.x or @ivar in class body, where
   the lookup resolves to Foo but the member lives under <Foo>).

In both cases, immediately removing the declaration would destroy
accumulated state (singleton, members) that the resolver can rebuild.
Instead, push emptied declarations to pending_declaration_cleanup and
run cleanup after resolution — only removing those still empty.

Add has_no_backing_definitions to Declaration, which combines the
synthetic check with the empty-definitions check. Synthetic declarations
(SingletonClass, Object, Module, Class) are never cleaned up this way —
they're only removed when their owner is removed.

Author: st0012

rust/rubydex/src/model/declaration.rs

@@ -345,6 +345,18 @@ impl Declaration {
         all_declarations!(self, it => it.definition_ids.is_empty())
     }
 
+    /// Returns true if this declaration has no backing definitions and is eligible
+    /// for removal. Synthetic declarations (singleton classes, bootstrap Object/Module/Class)
+    /// are never definition-backed — they should only be removed when their owner is removed.
+    #[must_use]
+    pub fn has_no_backing_definitions(&self, decl_id: &DeclarationId) -> bool {
+        let is_synthetic = matches!(self, Declaration::Namespace(Namespace::SingletonClass(_)))
+            || *decl_id == *super::graph::OBJECT_ID
+            || *decl_id == *super::graph::MODULE_ID
+            || *decl_id == *super::graph::CLASS_ID;
+        !is_synthetic && self.has_no_definitions()
+    }
+
     pub fn add_definition(&mut self, definition_id: DefinitionId) {
         all_declarations!(self, it => {
             debug_assert!(

rust/rubydex/src/model/graph.rs

@@ -87,6 +87,11 @@ pub struct Graph {
 
     /// Paths to exclude from file discovery during indexing.
     excluded_paths: HashSet<PathBuf>,
+
+    /// Declarations that became empty during name cascade and may need removal.
+    /// Deferred until after resolution — if the declaration was repopulated, skip it.
+    /// Drained by `take_pending_declaration_cleanup()`.
+    pending_declaration_cleanup: Vec<DeclarationId>,
 }
 
 impl Graph {
@@ -129,6 +134,7 @@ impl Graph {
             name_dependents: IdentityHashMap::default(),
             pending_work: Vec::default(),
             excluded_paths: HashSet::new(),
+            pending_declaration_cleanup: Vec::default(),
         }
     }
 
@@ -628,6 +634,11 @@ impl Graph {
         std::mem::take(&mut self.pending_work)
     }
 
+    /// Drains declarations that need post-resolution cleanup.
+    pub(crate) fn take_pending_declaration_cleanup(&mut self) -> Vec<DeclarationId> {
+        std::mem::take(&mut self.pending_declaration_cleanup)
+    }
+
     fn push_work(&mut self, unit: Unit) {
         self.pending_work.push(unit);
     }
@@ -636,6 +647,28 @@ impl Graph {
         self.pending_work.extend(units);
     }
 
+    /// Post-resolution cleanup: removes declarations that were emptied during name
+    /// cascade and never repopulated by re-resolution. Runs `invalidate_graph` on
+    /// any that are still empty, which cascades to their members and singletons.
+    /// Post-resolution cleanup: removes declarations that were emptied during
+    /// invalidation cascade and never repopulated by re-resolution. Cascades
+    /// to members, singletons, and descendants via `invalidate_graph`.
+    pub(crate) fn cleanup_empty_declarations(&mut self, candidates: Vec<DeclarationId>) {
+        let items: Vec<InvalidationItem> = candidates
+            .into_iter()
+            .filter(|decl_id| {
+                self.declarations
+                    .get(decl_id)
+                    .is_some_and(Declaration::has_no_definitions)
+            })
+            .map(InvalidationItem::Declaration)
+            .collect();
+
+        if !items.is_empty() {
+            self.invalidate_graph(items, IdentityHashMap::default());
+        }
+    }
+
     /// Converts a `Resolved` `NameRef` back to `Unresolved`, preserving the original `Name` data.
     /// Returns the `DeclarationId` it was previously resolved to, if any.
     fn unresolve_name(&mut self, name_id: NameId) -> Option<DeclarationId> {
@@ -1038,10 +1071,14 @@ impl Graph {
             .collect();
 
         if !missed_def_ids.is_empty() {
-            for declaration in self.declarations.values_mut() {
+            for (decl_id, declaration) in &mut self.declarations {
+                let had_definitions = !declaration.definitions().is_empty();
                 for def_id in &missed_def_ids {
                     declaration.remove_definition(def_id);
                 }
+                if had_definitions && declaration.has_no_backing_definitions(decl_id) {
+                    self.pending_declaration_cleanup.push(*decl_id);
+                }
             }
         }
 
@@ -1056,8 +1093,11 @@ impl Graph {
         }
     }
 
-    /// Unified invalidation worklist. Processes declaration and name items in a single loop,
+    /// Invalidation worklist. Processes declaration, name, and reference items in a loop,
     /// where processing one item can push new items back onto the queue.
+    ///
+    /// `pending_detachments` maps declarations to definitions that need to be detached
+    /// before deciding whether to remove or update each declaration.
     fn invalidate_graph(
         &mut self,
         items: Vec<InvalidationItem>,
@@ -1125,10 +1165,13 @@ impl Graph {
         let Some(decl) = self.declarations.get(&decl_id) else {
             return;
         };
-        let should_remove = decl.has_no_definitions() || !self.declarations.contains_key(decl.owner_id());
+        let should_remove =
+            decl.has_no_backing_definitions(&decl_id) || !self.declarations.contains_key(decl.owner_id());
 
         if should_remove {
-            // Queue members + singleton for removal
+            // Remove path: definitions came from a deleted/changed file and won't
+            // re-resolve to this declaration. Removal must be immediate so child
+            // declarations see a missing owner and cascade correctly.
             if let Some(ns) = decl.as_namespace() {
                 if let Some(singleton_id) = ns.singleton_class() {
                     queue.push(InvalidationItem::Declaration(*singleton_id));
@@ -1236,12 +1279,18 @@ impl Graph {
                             decl.remove_definition(def_id);
                         }
 
+                        // Defer cleanup: unlike the remove path in invalidate_declaration
+                        // (where definitions are from a deleted file and won't return),
+                        // this definition is from a surviving file — it was re-queued and
+                        // will likely re-resolve to the same declaration. Removing now
+                        // would destroy accumulated state (singleton, members) that can't
+                        // be rebuilt. Post-resolution cleanup removes it if still empty.
                         if self
                             .declarations
                             .get(&old_decl_id)
-                            .is_some_and(Declaration::has_no_definitions)
+                            .is_some_and(|decl| decl.has_no_backing_definitions(&old_decl_id))
                         {
-                            queue.push(InvalidationItem::Declaration(old_decl_id));
+                            self.pending_declaration_cleanup.push(old_decl_id);
                         }
                     }
                     NameDependent::ChildName(_) | NameDependent::NestedName(_) => {}
@@ -3670,6 +3719,79 @@ mod incremental_resolution_tests {
         assert_no_dangling_definitions(context.graph());
     }
 
+    #[test]
+    fn singleton_class_survives_when_reopener_is_deleted() {
+        let mut context = GraphTest::new();
+        context.index_uri("file:///foo.rb", "class Foo; def self.bar; end; end");
+        context.index_uri("file:///reopener.rb", "class Foo; end");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Foo");
+        assert_declaration_exists!(context, "Foo::<Foo>");
+
+        context.delete_uri("file:///reopener.rb");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Foo");
+        assert_declaration_exists!(context, "Foo::<Foo>");
+    }
+
+    #[test]
+    fn singleton_survives_when_singleton_definition_deleted_but_caller_remains() {
+        let mut context = GraphTest::new();
+        context.index_uri("file:///foo.rb", "class Foo; end");
+        context.index_uri("file:///foo_singleton.rb", "class Foo; class << self; end; end");
+        context.index_uri("file:///whatever.rb", "Foo.new");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Foo");
+        assert_declaration_exists!(context, "Foo::<Foo>");
+
+        // Remove the file with the explicit `class << self`. Foo::<Foo> should
+        // survive because Foo.new in whatever.rb created an Attached reference
+        // that also triggers singleton creation.
+        context.delete_uri("file:///foo_singleton.rb");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Foo");
+        assert_declaration_exists!(context, "Foo::<Foo>");
+    }
+
+    #[test]
+    fn nested_class_inside_singleton_scope_survives_reopener_deletion() {
+        let mut context = GraphTest::new();
+
+        context.index_uri(
+            "file:///main.rb",
+            r"
+            module Outer
+              class << self
+                class Inner
+                  def initialize; end
+                end
+
+                def run
+                  Inner.new
+                end
+              end
+            end
+            ",
+        );
+        context.index_uri("file:///reopener.rb", "module Outer; end");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Outer");
+        assert_declaration_exists!(context, "Outer::<Outer>::Inner");
+        assert_declaration_exists!(context, "Outer::<Outer>::Inner::<Inner>");
+
+        context.delete_uri("file:///reopener.rb");
+        context.resolve();
+
+        assert_declaration_exists!(context, "Outer");
+        assert_declaration_exists!(context, "Outer::<Outer>::Inner");
+        assert_declaration_exists!(context, "Outer::<Outer>::Inner::<Inner>");
+    }
+
     #[test]
     fn singleton_class_preserved_after_delete_and_reindex() {
         let mut context = GraphTest::new();
@@ -3692,27 +3814,29 @@ mod incremental_resolution_tests {
     }
 
     #[test]
-    fn singleton_recreated_when_reference_nested_in_compact_class() {
+    fn singleton_members_cleaned_up_after_file_deletion() {
         let mut context = GraphTest::new();
-
-        context.index_uri("file:///parent.rb", "module Parent; end");
-        context.index_uri("file:///target.rb", "class Parent::Target; end");
-        context.index_uri("file:///caller.rb", "class Parent::Caller; Parent::Target.new; end");
+        context.index_uri(
+            "file:///a.rb",
+            "
+            class Foo
+              @x = 1
+              def self.bar; end
+            end
+            ",
+        );
+        context.index_uri("file:///b.rb", "class Foo; end");
         context.resolve();
 
-        assert_declaration_exists!(context, "Parent::Target");
-        assert_declaration_exists!(context, "Parent::Target::<Target>");
+        assert_declaration_exists!(context, "Foo::<Foo>#@x");
+        assert_declaration_exists!(context, "Foo::<Foo>#bar()");
 
-        context.delete_uri("file:///parent.rb");
-        context.delete_uri("file:///target.rb");
-        context.resolve();
-
-        context.index_uri("file:///parent.rb", "module Parent; end");
-        context.index_uri("file:///target.rb", "class Parent::Target; end");
+        context.delete_uri("file:///a.rb");
         context.resolve();
 
-        assert_declaration_exists!(context, "Parent::Target");
-        assert_declaration_exists!(context, "Parent::Target::<Target>");
+        assert_declaration_exists!(context, "Foo");
+        assert_declaration_does_not_exist!(context, "Foo::<Foo>#@x");
+        assert_declaration_does_not_exist!(context, "Foo::<Foo>#bar()");
     }
 
     #[test]

rust/rubydex/src/resolution.rs

@@ -124,6 +124,15 @@ impl<'a> Resolver<'a> {
         self.graph.extend_work(std::mem::take(&mut self.unit_queue));
 
         self.handle_remaining_definitions(other_ids);
+
+        // Post-resolution cleanup: declarations emptied during name cascade
+        // (unresolve_dependent_name) and never repopulated should be removed.
+        let mut cleanup_candidates = self.graph.take_pending_declaration_cleanup();
+        if !cleanup_candidates.is_empty() {
+            cleanup_candidates.sort_unstable();
+            cleanup_candidates.dedup();
+            self.graph.cleanup_empty_declarations(cleanup_candidates);
+        }
     }
 
     /// Resolves a single constant against the graph. This method is not meant to be used by the resolution phase, but by