Resolve retroactive singleton method visibility changes (#781)

Part of #89 and follows #780, which added the `SINGLETON_METHOD_VISIBILITY` bit flag and indexed singleton-flagged `MethodVisibilityDefinition`s.

This PR routes singleton-flagged defs through the singleton class and folds the singleton path into the existing `resolve_method_visibilities` pass. Visibility definitions are processed after the main convergence loop, so the target method declaration is guaranteed to exist by the time we attach visibility.

For inherited targets like `private_class_method :foo` where `foo` comes from a parent class's singleton, we create a child-owned `MethodDeclaration` on the child's singleton class. This matches what #738 did for instance methods, and what Ruby reports when you ask `Child.singleton_class.instance_method(:foo).owner`:

```ruby
class Parent
  def self.foo; end
end

class Child < Parent
  private_class_method :foo
end

Child.singleton_class.instance_method(:foo).owner
# => #<Class:Child>   (Ruby copies the method to Child when visibility changes)

Parent.singleton_class.instance_method(:foo).owner
# => #<Class:Parent>  (untouched on the parent)
```

### Why document-scoped diagnostics for the singleton path

When a singleton target doesn't resolve, the diagnostic attaches to the document, not to the singleton declaration. Walking ancestors via `get_or_create_singleton_class(Foo, ...)` can synthesize `Foo::<Foo>` as a side effect, even when it has no real members. For `class Foo; private_class_method :missing; end` where `Foo` has no other singleton methods, that synthetic singleton class only exists to host the diagnostic. Attaching to it would leak the declaration on file delete. Document-scoped clears with the file. Instance-method diagnostics keep their existing declaration scope: their owner can't be synthetic.

### Source-order processing of visibility defs

Ruby applies visibility statements in the order they appear in source, so the later one wins:

```ruby
class Foo
  def bar; end
  private :bar
  public :bar          # Ruby: bar is Public
end
```

This was already broken on `main` for instance methods; surfaced while wiring up the singleton path, fixed here for both. `prepare_units` sorted `definitions` and `const_refs` by `(uri_id, offset)` for determinism but left `others` (which holds the visibility defs queued by `handle_remaining_definitions`) in `IdentityHashMap` iteration order, which is bucket-hash order. So `Foo#bar` could end up `Private` because the resolver happened to apply `public` first and `private` second. Same sort applied to `others` so the override-order invariant holds for both instance and singleton paths.

### In this PR

- branch on `is_singleton_method_visibility()` inside `resolve_method_visibilities`: resolve through `get_or_create_singleton_class` for the singleton path, render `"class method"` vs `"method"` in the diagnostic, attach document-scoped (singleton) vs declaration-scoped (instance)
- add `Graph::find_singleton_method_visibility_declaration` and route through it from the existing `Definition::MethodVisibility` arm in `definition_to_declaration_id` when the flag is set
- sort `others` by `(uri_id, offset)` in `prepare_units` so visibility overrides apply in source order

Author: alexcrocha

rust/rubydex/src/model/graph.rs

@@ -6,7 +6,7 @@ use crate::diagnostic::Diagnostic;
 use crate::indexing::local_graph::LocalGraph;
 use crate::model::built_in::{OBJECT_ID, add_built_in_data};
 use crate::model::declaration::{Ancestor, Declaration, Namespace};
-use crate::model::definitions::{Definition, Receiver};
+use crate::model::definitions::{Definition, MethodVisibilityDefinition, Receiver};
 use crate::model::document::Document;
 use crate::model::encoding::Encoding;
 use crate::model::identity_maps::{IdentityHashMap, IdentityHashSet};
@@ -333,10 +333,15 @@ impl Graph {
                     .or_else(|| self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref())),
                 it.target(),
             ),
-            Definition::MethodVisibility(it) => (
-                self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
-                it.str_id(),
-            ),
+            Definition::MethodVisibility(it) => {
+                if it.flags().is_singleton_method_visibility() {
+                    return self.find_singleton_method_visibility_declaration(it);
+                }
+                (
+                    self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
+                    it.str_id(),
+                )
+            }
             Definition::GlobalVariable(it) => (
                 self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
                 it.str_id(),
@@ -413,6 +418,27 @@ impl Graph {
         None
     }
 
+    /// Looks up the declaration for a singleton method visibility through the singleton class.
+    fn find_singleton_method_visibility_declaration(
+        &self,
+        definition: &MethodVisibilityDefinition,
+    ) -> Option<&DeclarationId> {
+        let nesting_name_id = self.find_enclosing_namespace_name_id(definition.lexical_nesting_id().as_ref());
+        let nesting_declaration_id = match nesting_name_id {
+            Some(name_id) => self.name_id_to_declaration_id(*name_id),
+            None => Some(&*OBJECT_ID),
+        }?;
+        let singleton_id = self
+            .declarations
+            .get(nesting_declaration_id)?
+            .as_namespace()?
+            .singleton_class()?;
+        self.declarations
+            .get(singleton_id)?
+            .as_namespace()?
+            .member(definition.str_id())
+    }
+
     /// Looks up the declaration for a `SelfReceiver` method/alias through the singleton class.
     fn find_self_receiver_declaration(&self, def_id: DefinitionId, member_str_id: StringId) -> Option<&DeclarationId> {
         let owner_decl_id = self.definition_id_to_declaration_id(def_id)?;

rust/rubydex/src/resolution.rs

@@ -604,10 +604,8 @@ impl<'a> Resolver<'a> {
                         self.graph.add_document_diagnostic(uri_id, diagnostic);
                     }
                 }
-                Definition::MethodVisibility(visibility_def) => {
-                    if !visibility_def.flags().is_singleton_method_visibility() {
-                        method_visibility_ids.push(id);
-                    }
+                Definition::MethodVisibility(_) => {
+                    method_visibility_ids.push(id);
                 }
                 Definition::Class(_)
                 | Definition::SingletonClass(_)
@@ -622,7 +620,8 @@ impl<'a> Resolver<'a> {
         self.resolve_method_visibilities(method_visibility_ids);
     }
 
-    /// Resolves retroactive method visibility changes (`private :foo`, `protected :foo`, `public :foo`).
+    /// Resolves retroactive method visibility changes (`private :foo`, `protected :foo`, `public :foo`,
+    /// `private_class_method :foo`, `public_class_method :foo`).
     ///
     /// Runs as a second pass after all methods/attrs are declared, so `private :bar` works
     /// regardless of whether `def bar` appeared before or after it in source.
@@ -638,11 +637,21 @@ impl<'a> Resolver<'a> {
             let uri_id = *method_visibility.uri_id();
             let offset = method_visibility.offset().clone();
             let lexical_nesting_id = *method_visibility.lexical_nesting_id();
+            let is_singleton = method_visibility.flags().is_singleton_method_visibility();
 
-            let Some(owner_id) = self.resolve_lexical_owner(lexical_nesting_id, id) else {
+            let Some(lexical_owner_id) = self.resolve_lexical_owner(lexical_nesting_id, id) else {
                 continue;
             };
 
+            let owner_id = if is_singleton {
+                let Some(singleton_id) = self.get_or_create_singleton_class(lexical_owner_id, true) else {
+                    continue;
+                };
+                singleton_id
+            } else {
+                lexical_owner_id
+            };
+
             let Some(Declaration::Namespace(namespace)) = self.graph.declarations().get(&owner_id) else {
                 continue;
             };
@@ -691,13 +700,20 @@ impl<'a> Resolver<'a> {
                     Rule::UndefinedMethodVisibilityTarget,
                     uri_id,
                     offset,
-                    format!("undefined method `{method_name}` for visibility change in `{owner_name}`"),
+                    format!("undefined method `{owner_name}#{method_name}` for visibility change"),
                 );
-                self.graph
-                    .declarations_mut()
-                    .get_mut(&owner_id)
-                    .unwrap()
-                    .add_diagnostic(diagnostic);
+                if is_singleton {
+                    // Document-scoped: the singleton class may be synthetic (created by this
+                    // visibility resolution) and won't be cleaned up on file delete, so attaching
+                    // the diagnostic to the declaration would leave it orphaned.
+                    self.graph.add_document_diagnostic(uri_id, diagnostic);
+                } else {
+                    self.graph
+                        .declarations_mut()
+                        .get_mut(&owner_id)
+                        .unwrap()
+                        .add_diagnostic(diagnostic);
+                }
             }
         }
 
@@ -1882,7 +1898,7 @@ impl<'a> Resolver<'a> {
                             singleton_methods.push(Unit::Definition(id));
                         }
                         _ => {
-                            others.push(id);
+                            others.push((id, (*definition.uri_id(), definition.offset())));
                         }
                     }
                 }
@@ -1922,14 +1938,15 @@ impl<'a> Resolver<'a> {
             (depths.get(name_a).unwrap(), uri_a, offset_a).cmp(&(depths.get(name_b).unwrap(), uri_b, offset_b))
         });
 
+        others.sort_unstable_by_key(|(_, key)| *key);
+
         // Definitions first, then constant refs, then singleton methods, then ancestors
         self.unit_queue.extend(definitions.into_iter().map(|(id, _)| id));
         self.unit_queue.extend(const_refs.into_iter().map(|(id, _)| id));
         self.unit_queue.extend(singleton_methods);
         self.unit_queue.extend(ancestors.into_iter().map(Unit::Ancestors));
 
-        others.shrink_to_fit();
-        others
+        others.into_iter().map(|(id, _)| id).collect()
     }
 
     /// Returns the singleton parent ID for an attached object ID. A singleton class' parent depends on what the attached

rust/rubydex/src/resolution_tests.rs

@@ -4926,6 +4926,25 @@ mod visibility_resolution_tests {
         };
     }
 
+    #[test]
+    fn retroactive_visibility_override_applies_in_source_order() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+              def bar; end
+              private :bar
+              public :bar
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+        assert_visibility_eq!(context, "Foo#bar()", Visibility::Public);
+    }
+
     #[test]
     fn retroactive_visibility_on_direct_method() {
         let mut context = GraphTest::new();
@@ -5066,7 +5085,7 @@ mod visibility_resolution_tests {
         assert_diagnostics_eq!(
             context,
             &[
-                "undefined-method-visibility-target: undefined method `nonexistent()` for visibility change in `Foo` (2:12-2:23)"
+                "undefined-method-visibility-target: undefined method `Foo#nonexistent()` for visibility change (2:12-2:23)"
             ]
         );
     }
@@ -5306,4 +5325,139 @@ mod visibility_resolution_tests {
 
         assert_visibility_eq!(context, "Foo::BAR", Visibility::Private);
     }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_on_direct_member() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+              def self.bar; end
+              def self.baz; end
+
+              private_class_method :bar
+              private_class_method :baz
+              public_class_method :baz
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+        assert_visibility_eq!(context, "Foo::<Foo>#bar()", Visibility::Private);
+        assert_visibility_eq!(context, "Foo::<Foo>#baz()", Visibility::Public);
+    }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_on_inherited_method() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Parent
+              def self.foo; end
+            end
+
+            class Child < Parent
+              private_class_method :foo
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+        assert_declaration_exists!(context, "Child::<Child>#foo()");
+        assert_visibility_eq!(context, "Child::<Child>#foo()", Visibility::Private);
+        assert_visibility_eq!(context, "Parent::<Parent>#foo()", Visibility::Public);
+    }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_on_undefined_method_emits_diagnostic() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+              private_class_method :nonexistent
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_diagnostics_eq!(
+            context,
+            &[
+                "undefined-method-visibility-target: undefined method `Foo::<Foo>#nonexistent()` for visibility change (2:25-2:36)"
+            ]
+        );
+    }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_undefined_target_diagnostic_clears_when_file_deleted() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+            end
+            ",
+        );
+        context.index_uri(
+            "file:///bad.rb",
+            r"
+            class Foo
+              private_class_method :missing
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_diagnostics_eq!(
+            context,
+            &[
+                "undefined-method-visibility-target: undefined method `Foo::<Foo>#missing()` for visibility change (2:25-2:32)"
+            ]
+        );
+
+        context.delete_uri("file:///bad.rb");
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+    }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_undefined_target_diagnostic_clears_when_target_added() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+              private_class_method :missing
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_diagnostics_eq!(
+            context,
+            &[
+                "undefined-method-visibility-target: undefined method `Foo::<Foo>#missing()` for visibility change (2:25-2:32)"
+            ]
+        );
+
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+              def self.missing; end
+              private_class_method :missing
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+        assert_visibility_eq!(context, "Foo::<Foo>#missing()", Visibility::Private);
+    }
 }

test/declaration_test.rb

@@ -789,6 +789,41 @@ class Foo
     end
   end
 
+  def test_class_method_visibility_via_private_class_method
+    with_context do |context|
+      context.write!("file1.rb", <<~RUBY)
+        class Foo
+          def self.bar; end
+          private_class_method :bar
+        end
+      RUBY
+
+      graph = Rubydex::Graph.new
+      graph.index_all(context.glob("**/*.rb"))
+      graph.resolve
+
+      assert_equal(:private, graph["Foo::<Foo>#bar()"].visibility)
+    end
+  end
+
+  def test_class_method_visibility_via_public_class_method
+    with_context do |context|
+      context.write!("file1.rb", <<~RUBY)
+        class Foo
+          def self.bar; end
+          private_class_method :bar
+          public_class_method :bar
+        end
+      RUBY
+
+      graph = Rubydex::Graph.new
+      graph.index_all(context.glob("**/*.rb"))
+      graph.resolve
+
+      assert_equal(:public, graph["Foo::<Foo>#bar()"].visibility)
+    end
+  end
+
   def test_constant_alias_visibility
     with_context do |context|
       context.write!("file1.rb", <<~RUBY)