Skip to content

Resolve module_function visibility#802

Open
alexcrocha wants to merge 1 commit into
mainfrom
05-11-resolve_module_function_visibility
Open

Resolve module_function visibility#802
alexcrocha wants to merge 1 commit into
mainfrom
05-11-resolve_module_function_visibility

Conversation

@alexcrocha
Copy link
Copy Markdown
Contributor

@alexcrocha alexcrocha commented May 12, 2026

Closes #89 and finishes the singleton-method visibility resolution work started in #780/#781/#782. This PR adds support for resolving module_function visibility:

module Foo
  def bar; end
  module_function :bar
end

Foo.bar           # public singleton method
Foo.new.bar       # NoMethodError (private instance method)

Foo#bar becomes private and Foo.bar is added as a public singleton method.

Two MethodVisibility defs instead of one shared

The indexer emits two MethodVisibility defs at the module_function :bar call site: one routed to the instance side (Foo#bar), one to the singleton side (Foo::<Foo>#bar) via the SINGLETON_METHOD_VISIBILITY flag from #780. Both carry Visibility::ModuleFunction.

A single shared def attached to both declarations doesn't work. definition_to_declaration_id only returns one declaration per def, so the singleton-side declaration would be unreachable from reverse-lookup invalidation. When the source method was removed in a different file, the singleton companion would survive in the graph:

# a.rb
module Foo
  def bar; end
  module_function :bar
end

# b.rb (re-indexed without `def bar`)
module Foo
end

With one shared def, Foo::<Foo>#bar() lingers after a.rb's re-index. Two defs side-step this: each routes to exactly one declaration, and the existing invalidation paths handle both correctly.

Reusing existing primitives

Visibility::ModuleFunction and SINGLETON_METHOD_VISIBILITY together fully describe the singleton-side def. No new flag is needed:

  • Visibility::ModuleFunction + no flag: instance-side def, resolves on the lexical owner.
  • Visibility::ModuleFunction + SINGLETON_METHOD_VISIBILITY: singleton-side def, resolves on the lexical owner but the resulting declaration lives on the singleton class.
  • Any other visibility + SINGLETON_METHOD_VISIBILITY: private_class_method / public_class_method, already handled in Resolve retroactive singleton method visibility changes #781.

Internals don't leak through Graph::visibility()

Graph::visibility() translates Visibility::ModuleFunction to Private (instance) or Public (singleton) based on the flag.

This keeps the two-def model internal. Callers query visibility and get Public/Private/Protected as expected. The SINGLETON_METHOD_VISIBILITY flag, the Visibility::ModuleFunction enum value, and the two-def emission are implementation details that don't leak through Graph's public API.

Defer singleton class creation until the target is confirmed

private_class_method looks up the target on the singleton class, so it has to create the singleton class up front. module_function's singleton-side def looks up on the instance side (Foo directly) and only attaches the resulting declaration to the singleton class. Creating the singleton class up front for the singleton-side def would leave an empty Foo::<Foo> namespace when the target is missing:

module Foo
  module_function :missing
end

The instance-side def emits the undefined method diagnostic. The singleton-side def stays silent and creates nothing, so no synthetic singleton class is left behind.

MethodVisibilityDefinition::id() includes flag bits

Two defs at the same source location share the same uri_id, offset, and str_id. The ID hash now includes flag bits so the siblings get distinct DefinitionIds. Without this, add_definition panics on collision.

In this PR

  • emit two MethodVisibility defs for module_function :sym (one per side; singleton-side carries SINGLETON_METHOD_VISIBILITY)
  • branch on Visibility::ModuleFunction inside resolve_method_visibilities for the cross-namespace lookup: target on the lexical owner, declaration on the singleton class
  • defer get_or_create_singleton_class for the singleton-side def until the target is confirmed
  • silence the singleton-side diagnostic for missing targets; the instance-side def owns the diagnostic
  • translate Visibility::ModuleFunction to Public (singleton) or Private (instance) in Graph::visibility() based on the flag
  • include DefinitionFlags bits in MethodVisibilityDefinition::id() so siblings get distinct IDs

Not in this PR

The inline form (module_function def bar; end) already works through the indexer's pre-existing two-MethodDefinition path; no changes needed here.

Ruby's exact runtime behavior for module_function :a, :missing is not modeled. Ruby raises on :missing only after Foo#a has been made private, and commits no singleton copies for the call. Rubydex resolves each argument independently, so module_function :a, :missing makes Foo#a private AND adds Foo.a as a public singleton method, with a diagnostic for :missing. Editors regularly see code in flux, so modeling each argument independently lets the user keep useful information that Ruby's all-or-nothing raise would discard.

Copy link
Copy Markdown
Contributor Author

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

`module_function :bar` makes `Foo#bar` private and adds `Foo.bar` as
a public singleton method. The indexer emits two `MethodVisibility`
defs at the call site (one per side, the singleton-side flagged with
`SINGLETON_METHOD_VISIBILITY`) so the existing resolution and
reverse-lookup invalidation paths handle each side independently.
This makes multi-file cleanup correct when either the source method
or the visibility call is removed.

The singleton-side declaration is created only after confirming the
target instance method exists, so `module_function :missing` doesn't
leave behind an empty `Foo::<Foo>` namespace.
@alexcrocha alexcrocha force-pushed the 05-11-resolve_module_function_visibility branch from f1516b0 to f4f9a77 Compare May 12, 2026 16:53
@alexcrocha alexcrocha self-assigned this May 12, 2026
@alexcrocha alexcrocha added the enhancement New feature or request label May 12, 2026
@alexcrocha alexcrocha marked this pull request as ready for review May 12, 2026 16:54
@alexcrocha alexcrocha requested a review from a team as a code owner May 12, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Start keeping track of visibility for applicable definitions

1 participant