Re-queue method aliases when singleton receiver is missing

`Definition::MethodAlias` inside `class << Foo` had the same drop-on-
missing-owner shape as the previously-fixed methods/attrs/instance
variables: when `resolve_lexical_owner` returned `None`, the alias was
silently skipped, so deleting and re-adding `Foo` lost
`Foo::<Foo>#new()` even though `Foo::<Foo>#old()` was restored.

Extract a `resolve_lexical_owner_or_requeue` helper and route Method,
Attr*, MethodAlias, and the singleton-body InstanceVariable path through
it so the policy is uniform. Also tightens a few of the doc comments
introduced by the previous commits.

Author: st0012

rust/rubydex/src/model/graph.rs

@@ -4323,10 +4323,7 @@ mod incremental_resolution_tests {
     /// 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.
+    /// reattached.
     #[test]
     fn singleton_definition_survives_receiver_delete_readd() {
         let mut incremental = GraphTest::new();
@@ -4357,6 +4354,34 @@ mod incremental_resolution_tests {
         assert_declaration_exists!(incremental, "Foo::<Foo>#bar()");
     }
 
+    /// Same shape as `singleton_definition_survives_receiver_delete_readd` but
+    /// for a method alias inside the singleton block. Without re-queueing the
+    /// def, `alias new old` would be silently dropped while `Foo` is missing
+    /// and `Foo::<Foo>#new()` would never be restored.
+    #[test]
+    fn singleton_body_method_alias_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 old; end; alias new old; end");
+        incremental.resolve();
+        assert_declaration_exists!(incremental, "Foo::<Foo>#old()");
+        assert_declaration_exists!(incremental, "Foo::<Foo>#new()");
+
+        incremental.delete_uri("file:///foo.rb");
+        incremental.resolve();
+
+        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 old; end; alias new old; end");
+        fresh.resolve();
+
+        assert_declaration_ids_match(&incremental, &fresh);
+        assert_declaration_exists!(incremental, "Foo::<Foo>#new()");
+    }
+
     /// Same shape as `singleton_definition_survives_receiver_delete_readd` but
     /// for a direct instance variable inside the singleton block. Without
     /// re-queueing the def, `@bar` would land on `Object::<Object>` (via the

rust/rubydex/src/resolution.rs

@@ -301,13 +301,8 @@ 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));
+                            let lexical = *method_definition.lexical_nesting_id();
+                            let Some(resolved) = self.resolve_lexical_owner_or_requeue(lexical, id) else {
                                 continue;
                             };
                             resolved
@@ -319,32 +314,35 @@ 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));
+                    let lexical = *attr.lexical_nesting_id();
+                    let str_id = *attr.str_id();
+                    let Some(owner_id) = self.resolve_lexical_owner_or_requeue(lexical, id) else {
                         continue;
                     };
 
-                    self.create_declaration(*attr.str_id(), id, owner_id, |name| {
+                    self.create_declaration(str_id, id, owner_id, |name| {
                         Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id)))
                     });
                 }
                 Definition::AttrReader(attr) => {
-                    let Some(owner_id) = self.resolve_lexical_owner(*attr.lexical_nesting_id()) else {
-                        self.graph.push_work(Unit::Definition(id));
+                    let lexical = *attr.lexical_nesting_id();
+                    let str_id = *attr.str_id();
+                    let Some(owner_id) = self.resolve_lexical_owner_or_requeue(lexical, id) else {
                         continue;
                     };
 
-                    self.create_declaration(*attr.str_id(), id, owner_id, |name| {
+                    self.create_declaration(str_id, id, owner_id, |name| {
                         Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id)))
                     });
                 }
                 Definition::AttrWriter(attr) => {
-                    let Some(owner_id) = self.resolve_lexical_owner(*attr.lexical_nesting_id()) else {
-                        self.graph.push_work(Unit::Definition(id));
+                    let lexical = *attr.lexical_nesting_id();
+                    let str_id = *attr.str_id();
+                    let Some(owner_id) = self.resolve_lexical_owner_or_requeue(lexical, id) else {
                         continue;
                     };
 
-                    self.create_declaration(*attr.str_id(), id, owner_id, |name| {
+                    self.create_declaration(str_id, id, owner_id, |name| {
                         Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id)))
                     });
                 }
@@ -534,7 +532,8 @@ impl<'a> Resolver<'a> {
                             }
                         },
                         None => {
-                            let Some(resolved) = self.resolve_lexical_owner(*alias.lexical_nesting_id()) else {
+                            let lexical = *alias.lexical_nesting_id();
+                            let Some(resolved) = self.resolve_lexical_owner_or_requeue(lexical, id) else {
                                 continue;
                             };
                             resolved
@@ -615,6 +614,22 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    /// Wraps `resolve_lexical_owner` for `handle_remaining_definitions` callers:
+    /// when the owner can't be resolved (its singleton nesting has no declaration
+    /// yet), re-queue the definition so the next resolve can place it on the
+    /// right owner instead of dropping the work.
+    fn resolve_lexical_owner_or_requeue(
+        &mut self,
+        lexical_nesting_id: Option<DefinitionId>,
+        id: DefinitionId,
+    ) -> Option<DeclarationId> {
+        let resolved = self.resolve_lexical_owner(lexical_nesting_id);
+        if resolved.is_none() {
+            self.graph.push_work(Unit::Definition(id));
+        }
+        resolved
+    }
+
     /// Resolves owner from lexical nesting.
     fn resolve_lexical_owner(&self, lexical_nesting_id: Option<DefinitionId>) -> Option<DeclarationId> {
         let Some(id) = lexical_nesting_id else {
@@ -623,13 +638,9 @@ 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.
+        // before the method definition itself. A SingletonClass with no declaration
+        // is an exception: returning the surrounding scope would attach its members to
+        // the wrong owner (e.g. `Object`) and never recover, so return None.
         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(_)) {
@@ -1050,20 +1061,13 @@ impl<'a> Resolver<'a> {
         let str_id = *name_ref.str();
 
         let outcome = match self.name_owner_id(name_id, singleton) {
-            // name_owner_id returns Unresolved(None) only when the parent scope is genuinely
-            // unknown (e.g., `class A::B::C` where `A` doesn't exist). For regular declarations
-            // we create Todo placeholders for the missing parent chain so `B::C` can still be
-            // placed; Todos get promoted when real definitions appear later.
+            // For a regular declaration, create Todo placeholders for the missing parent chain
+            // so e.g. `class A::B::C` with `A` undefined can still be placed; Todos are 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 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.
+            // Singletons attach via `set_singleton_class_id` (not `add_member`), so a Todo
+            // receiver would never gain a member. Emit Retry so the unit is preserved for a
+            // later resolve where the receiver may exist.
             Outcome::Unresolved(None) if singleton => Outcome::Retry(None),
             Outcome::Unresolved(None) => Outcome::Resolved(self.create_todo_for_parent(name_id), None),
             other => other,