Index private_class_method / public_class_method calls by alexcrocha · Pull Request #780 · Shopify/rubydex

alexcrocha · 2026-05-05T03:44:59Z

Part of #89

This PR adds indexer support for private_class_method and public_class_method calls. Each call becomes a MethodVisibilityDefinition with the SINGLETON_METHOD_VISIBILITY flag set, which the resolver applies to the singleton method declaration in a follow-up PR. This is the base of a Graphite stack.

We need a definition because the indexer can't yet attach the visibility change to the singleton method:

class Foo
  def self.bar; end
  private_class_method :bar
end

The method declaration for Foo::<Foo>#bar() only exists after resolution.

Why a bit flag

I added a DefinitionFlags::SINGLETON_METHOD_VISIBILITY bit on the existing MethodVisibilityDefinition rather than introducing a parallel SingletonMethodVisibilityDefinition type. The two would have carried the same fields and the indexer treats instance and singleton visibility calls almost identically. A bit flag captures the singleton-vs-instance distinction without doubling the definition variants and the FFI plumbing that comes with them.

Why no explicit receiver support

Foo.private_class_method :bar is legal Ruby but rare in practice. Supporting it would require a new definition type to carry the receiver. Ignoring it lets us fold the singleton case into the existing MethodVisibilityDefinition and get resolution-time differentiation from a single bit flag, which keeps the blast radius of the change small. Calls with an explicit receiver bypass the visibility logic, though the call is still visited so nested references stay indexed.

In this PR

add DefinitionFlags::SINGLETON_METHOD_VISIBILITY
index private_class_method / public_class_method calls with literal symbol or string targets, with an implicit receiver only
skip singleton-flagged definitions from resolve_method_visibilities so the new flag stays inert until the follow-up PR wires up resolution
diagnose top-level calls and non-literal arguments

Not in this PR

A few things are intentionally deferred to follow-up PRs in the stack:

applying the visibility to the singleton method declaration
extracting the literal-extraction helper we duplicate from handle_constant_visibility
supporting array arguments (private_class_method [:a, :b])
supporting inline-def arguments (private_class_method def self.foo; end)

alexcrocha · 2026-05-05T03:45:16Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

st0012 · 2026-05-07T06:53:03Z

+        call_name: &str,
+    ) {
+        match node.receiver() {
+            Some(ruby_prism::Node::SelfNode { .. }) | None => match self.nesting_stack.last() {


Should the branches under this condition call visit_call_node_parts before returning/continuing?

I think that's already happening. When we process the arguments below, if it's a string or a symbol, we consider it as a valid call, otherwise we visit the argument and add a diagnostic.

vinistock

General implementation is good, just a few fixes

vinistock · 2026-05-07T14:38:00Z

    pub struct DefinitionFlags: u8 {
        const DEPRECATED = 0b0001;
        const PROMOTABLE = 0b0010;
+        const SINGLETON_METHOD_VISIBILITY = 0b0100;


Note for the future: we're using DefinitionFlags for all definitions, but the reality is that most flags are only applicable to certain types. For example, deprecated doesn't make much sense for these visibility related definitions, promotable is only applicable to constants and this new one is only applicable to visibility operations.

Right now, this is not a problem because we didn't run out of space yet, but it would be nice to specialize a bunch of different flags.

vinistock · 2026-05-07T14:42:14Z

+        }
+
+        let Some(arguments) = node.arguments() else {
+            return;


For a future PR, we can add a diagnostic here. Invoking private|public_class_method with no arguments does nothing.

vinistock · 2026-05-07T14:45:06Z

+        call_name: &str,
+    ) {
+        match node.receiver() {
+            Some(ruby_prism::Node::SelfNode { .. }) | None => match self.nesting_stack.last() {


I think that's already happening. When we process the arguments below, if it's a string or a symbol, we consider it as a valid call, otherwise we visit the argument and add a diagnostic.

alexcrocha · 2026-05-07T19:57:46Z

Merge activity

May 7, 7:57 PM UTC: @alexcrocha merged this pull request with Graphite.

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

This was referenced May 5, 2026

Extract shared helpers for visibility-call processing #783

Draft

Support array and inline-def arguments to private_class_method #782

Merged

alexcrocha mentioned this pull request May 5, 2026

Resolve retroactive singleton method visibility changes #781

Merged

alexcrocha self-assigned this May 5, 2026

alexcrocha added the enhancement New feature or request label May 5, 2026

alexcrocha marked this pull request as ready for review May 5, 2026 21:50

alexcrocha requested a review from a team as a code owner May 5, 2026 21:50

alexcrocha force-pushed the 05-04-index_private_class_method___public_class_method_calls branch 5 times, most recently from 85b078d to 16d134d Compare May 6, 2026 23:37

st0012 reviewed May 7, 2026

View reviewed changes

vinistock approved these changes May 7, 2026

View reviewed changes

alexcrocha force-pushed the 05-04-index_private_class_method___public_class_method_calls branch from 16d134d to 27da4dd Compare May 7, 2026 18:15

Index private_class_method / public_class_method calls

ce07ec6

alexcrocha force-pushed the 05-04-index_private_class_method___public_class_method_calls branch from 27da4dd to ce07ec6 Compare May 7, 2026 19:53

alexcrocha merged commit f1bda68 into main May 7, 2026
36 checks passed

alexcrocha deleted the 05-04-index_private_class_method___public_class_method_calls branch May 7, 2026 19:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index private_class_method / public_class_method calls#780

Index private_class_method / public_class_method calls#780
alexcrocha merged 1 commit intomainfrom
05-04-index_private_class_method___public_class_method_calls

alexcrocha commented May 5, 2026 •

edited

Loading

Uh oh!

alexcrocha commented May 5, 2026 •

edited

Loading

Uh oh!

st0012 May 7, 2026

Uh oh!

vinistock May 7, 2026

Uh oh!

Uh oh!

Uh oh!

vinistock left a comment

Uh oh!

vinistock May 7, 2026

Uh oh!

vinistock May 7, 2026

Uh oh!

Uh oh!

vinistock May 7, 2026

Uh oh!

Uh oh!

Uh oh!

alexcrocha commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alexcrocha commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why a bit flag

Why no explicit receiver support

In this PR

Not in this PR

Uh oh!

alexcrocha commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

st0012 May 7, 2026

Choose a reason for hiding this comment

Uh oh!

vinistock May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vinistock left a comment

Choose a reason for hiding this comment

Uh oh!

vinistock May 7, 2026

Choose a reason for hiding this comment

Uh oh!

vinistock May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vinistock May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alexcrocha commented May 7, 2026

Merge activity

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexcrocha commented May 5, 2026 •

edited

Loading

alexcrocha commented May 5, 2026 •

edited

Loading