Preserve singleton definitions when receiver is missing

Two related fixes for singleton class invalidation:

1. `class << Foo` definitions were dropped if Foo was temporarily missing.
   `handle_constant_declaration` now emits `Retry(None)` instead of
   `Unresolved(None)` for singletons whose receiver can't be resolved, so
   the unit survives across resolve cycles. `resolve_lexical_owner` no
   longer falls back to the surrounding scope when the immediate nesting
   is a `SingletonClass` without a declaration; methods inside the block
   are re-queued via `push_work` until the singleton is reattached.

2. Singletons kept alive only by a constant reference (e.g. `Foo::<Foo>`
   materialized by `Foo.new`) were not collected when the reference's
   file was deleted. `invalidate()` only seeds from removed definitions,
   so `remove_document_data` now queues declarations that become
   removable after reference detachment.

Author: st0012

rust/rubydex/src/model/graph.rs

@@ -634,7 +634,7 @@ impl Graph {
         self.tracked_todos.insert(decl_id);
     }
 
-    fn push_work(&mut self, unit: Unit) {
+    pub(crate) fn push_work(&mut self, unit: Unit) {
         self.pending_work.push(unit);
     }
 
@@ -1042,21 +1042,34 @@ impl Graph {
             }
         }
 
+        // Detach references and queue any declaration that becomes removable as a
+        // result. `invalidate()` only seeds from removed definitions, so a singleton
+        // kept alive purely by a reference (e.g. `Foo::<Foo>` materialized by
+        // `Foo.new`) would otherwise linger after the reference's file is deleted.
+        let mut emptied_by_ref_removal: Vec<InvalidationItem> = Vec::new();
         for ref_id in document.constant_references() {
             if let Some(constant_ref) = self.constant_references.remove(ref_id) {
                 // Detach from target declaration. References unresolved during invalidation
                 // were already detached; this catches the rest.
-                if let Some(NameRef::Resolved(resolved)) = self.names.get(constant_ref.name_id())
-                    && let Some(declaration) = self.declarations.get_mut(resolved.declaration_id())
-                {
-                    declaration.remove_constant_reference(ref_id);
+                if let Some(NameRef::Resolved(resolved)) = self.names.get(constant_ref.name_id()) {
+                    let target_id = *resolved.declaration_id();
+                    if let Some(declaration) = self.declarations.get_mut(&target_id) {
+                        declaration.remove_constant_reference(ref_id);
+                        if declaration.is_removable() {
+                            emptied_by_ref_removal.push(InvalidationItem::Declaration(target_id));
+                        }
+                    }
                 }
 
                 self.remove_name_dependent(*constant_ref.name_id(), NameDependent::Reference(*ref_id));
                 self.untrack_name(*constant_ref.name_id());
             }
         }
 
+        if !emptied_by_ref_removal.is_empty() {
+            self.invalidate_graph(emptied_by_ref_removal, IdentityHashMap::default());
+        }
+
         // Detach removed definitions from their declarations.
         // Most definitions were already detached by invalidate_declaration via
         // pending_detachments. Definitions not handled by pending_detachments are
@@ -4306,4 +4319,74 @@ mod incremental_resolution_tests {
 
         assert_declaration_ids_match(&incremental, &fresh);
     }
+
+    /// Delete the receiver of a `class << Foo` block, resolve, then re-add the
+    /// receiver. The singleton's definition unit must survive the gap so that
+    /// when `Foo` returns, `Foo::<Foo>` and any methods inside the block are
+    /// reattached. Regression for the case where dropping singleton TODO
+    /// creation also dropped the singleton's definition unit on
+    /// `Outcome::Unresolved(None)` — emitting `Retry(None)` instead keeps the
+    /// unit queued across resolve cycles.
+    #[test]
+    fn singleton_definition_survives_receiver_delete_readd() {
+        let mut incremental = GraphTest::new();
+        incremental.index_uri("file:///foo.rb", "class Foo; end");
+        incremental.index_uri("file:///singleton.rb", "class << Foo; def bar; end; end");
+        incremental.resolve();
+        assert_declaration_exists!(incremental, "Foo::<Foo>");
+        assert_declaration_exists!(incremental, "Foo::<Foo>#bar()");
+
+        incremental.delete_uri("file:///foo.rb");
+        incremental.resolve();
+        assert_declaration_does_not_exist!(incremental, "Foo");
+        assert_declaration_does_not_exist!(incremental, "Foo::<Foo>");
+        assert_declaration_does_not_exist!(incremental, "Foo::<Foo>#bar()");
+        // The method's lexical owner can't be resolved while the singleton is
+        // gone, so it must not get re-attached to `Object` as a fallback.
+        assert_declaration_does_not_exist!(incremental, "Object#bar()");
+
+        incremental.index_uri("file:///foo.rb", "class Foo; end");
+        incremental.resolve();
+
+        let mut fresh = GraphTest::new();
+        fresh.index_uri("file:///foo.rb", "class Foo; end");
+        fresh.index_uri("file:///singleton.rb", "class << Foo; def bar; end; end");
+        fresh.resolve();
+
+        assert_declaration_ids_match(&incremental, &fresh);
+        assert_declaration_exists!(incremental, "Foo::<Foo>#bar()");
+    }
+
+    /// A singleton class materialized solely by a constant reference (e.g.
+    /// `Foo.new` creating `Foo::<Foo>` even though no `class << Foo` block
+    /// exists) must be collected when its sole supporting reference is
+    /// removed. `invalidate()` only seeds from removed definitions, so the
+    /// reference detachment in `remove_document_data` queues now-removable
+    /// declarations for cleanup.
+    #[test]
+    fn singleton_kept_only_by_reference_collected_on_ref_delete() {
+        let mut context = GraphTest::new();
+        context.index_uri("file:///foo.rb", "class Foo; end");
+        context.index_uri("file:///user.rb", "Foo.new");
+        context.resolve();
+        assert_declaration_exists!(context, "Foo::<Foo>");
+
+        context.delete_uri("file:///user.rb");
+        context.resolve();
+
+        // The reference that materialized the singleton is gone, so the
+        // singleton itself must not linger and `Foo.singleton_class_id` must
+        // be cleared.
+        assert_declaration_does_not_exist!(context, "Foo::<Foo>");
+        let foo = context
+            .graph()
+            .declarations()
+            .get(&crate::model::ids::DeclarationId::from("Foo"))
+            .expect("Foo should still exist");
+        let foo_ns = foo.as_namespace().expect("Foo is a namespace");
+        assert!(
+            foo_ns.singleton_class().is_none(),
+            "Foo.singleton_class_id should be cleared after the singleton is removed"
+        );
+    }
 } // mod incremental_resolution_tests

rust/rubydex/src/resolution.rs

@@ -301,8 +301,13 @@ impl<'a> Resolver<'a> {
                             singleton_id
                         }
                         None => {
+                            // resolve_lexical_owner returns None when the immediate nesting is
+                            // a SingletonClass def whose declaration is missing (e.g. receiver
+                            // was just deleted). Re-queue so the next resolve, after the
+                            // singleton is reattached, can place the method on the right owner.
                             let Some(resolved) = self.resolve_lexical_owner(*method_definition.lexical_nesting_id())
                             else {
+                                self.graph.push_work(Unit::Definition(id));
                                 continue;
                             };
                             resolved
@@ -315,6 +320,7 @@ impl<'a> Resolver<'a> {
                 }
                 Definition::AttrAccessor(attr) => {
                     let Some(owner_id) = self.resolve_lexical_owner(*attr.lexical_nesting_id()) else {
+                        self.graph.push_work(Unit::Definition(id));
                         continue;
                     };
 
@@ -324,6 +330,7 @@ impl<'a> Resolver<'a> {
                 }
                 Definition::AttrReader(attr) => {
                     let Some(owner_id) = self.resolve_lexical_owner(*attr.lexical_nesting_id()) else {
+                        self.graph.push_work(Unit::Definition(id));
                         continue;
                     };
 
@@ -333,6 +340,7 @@ impl<'a> Resolver<'a> {
                 }
                 Definition::AttrWriter(attr) => {
                     let Some(owner_id) = self.resolve_lexical_owner(*attr.lexical_nesting_id()) else {
+                        self.graph.push_work(Unit::Definition(id));
                         continue;
                     };
 
@@ -612,8 +620,17 @@ impl<'a> Resolver<'a> {
         // If no declaration exists yet for this definition, walk up the lexical chain.
         // This handles the case where attr_* definitions inside methods are processed
         // before the method definition itself.
+        //
+        // Exception: a SingletonClass with no declaration (because its receiver was just
+        // deleted) must not fall back to the surrounding scope. Doing so would attach
+        // members like `def bar` to e.g. `Object` instead of waiting for the singleton
+        // to be reattached on a later resolve. Return `None` so the caller can skip and
+        // re-queue the definition.
         let Some(declaration_id) = self.graph.definition_id_to_declaration_id(id) else {
             let definition = self.graph.definitions().get(&id).unwrap();
+            if matches!(definition, Definition::SingletonClass(_)) {
+                return None;
+            }
             return self.resolve_lexical_owner(*definition.lexical_nesting_id());
         };
 
@@ -1035,11 +1052,16 @@ impl<'a> Resolver<'a> {
             // placed; Todos get promoted when real definitions appear later.
             //
             // Singleton classes are the exception: `class << UndefinedReceiver` attaches via
-            // `set_singleton_class_id`, not `add_member`, so a TODO receiver would never gain
-            // a member and would remain empty forever. Skip TODO creation for singletons —
-            // the singleton simply can't be placed. Retry (preserved via `preserve_retry`) lets
-            // the unit cycle back through the worklist if the receiver might resolve later.
-            Outcome::Unresolved(None) if !singleton => Outcome::Resolved(self.create_todo_for_parent(name_id), None),
+            // `set_singleton_class_id`, not `add_member`, so a TODO receiver would never gain a
+            // member and would remain empty forever. Skip TODO creation and emit Retry instead
+            // of Unresolved so the unit is preserved across resolve cycles. `made_progress` only
+            // flips on Resolved, so retrying is safe — the inner fixpoint terminates and any
+            // leftover unit is drained back to `pending_work`. This matters for delete/re-add:
+            // after deleting `class Foo`, the `class << Foo; def bar; end` definition unit
+            // re-enters resolution with Foo missing; emitting Retry keeps it queued so the next
+            // resolve (with Foo restored) can attach `Foo::<Foo>#bar` again.
+            Outcome::Unresolved(None) if singleton => Outcome::Retry(None),
+            Outcome::Unresolved(None) => Outcome::Resolved(self.create_todo_for_parent(name_id), None),
             other => other,
         };