Resolve module_function visibility#802
Open
alexcrocha wants to merge 1 commit into
Open
Conversation
`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.
f1516b0 to
f4f9a77
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Closes #89 and finishes the singleton-method visibility resolution work started in #780/#781/#782. This PR adds support for resolving
module_functionvisibility:Foo#barbecomes private andFoo.baris added as a public singleton method.Two
MethodVisibilitydefs instead of one sharedThe indexer emits two
MethodVisibilitydefs at themodule_function :barcall site: one routed to the instance side (Foo#bar), one to the singleton side (Foo::<Foo>#bar) via theSINGLETON_METHOD_VISIBILITYflag from #780. Both carryVisibility::ModuleFunction.A single shared def attached to both declarations doesn't work.
definition_to_declaration_idonly 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:With one shared def,
Foo::<Foo>#bar()lingers aftera.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::ModuleFunctionandSINGLETON_METHOD_VISIBILITYtogether 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.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()translatesVisibility::ModuleFunctiontoPrivate(instance) orPublic(singleton) based on the flag.This keeps the two-def model internal. Callers query visibility and get
Public/Private/Protectedas expected. TheSINGLETON_METHOD_VISIBILITYflag, theVisibility::ModuleFunctionenum value, and the two-def emission are implementation details that don't leak throughGraph's public API.Defer singleton class creation until the target is confirmed
private_class_methodlooks 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 (Foodirectly) and only attaches the resulting declaration to the singleton class. Creating the singleton class up front for the singleton-side def would leave an emptyFoo::<Foo>namespace when the target is missing:The instance-side def emits the
undefined methoddiagnostic. The singleton-side def stays silent and creates nothing, so no synthetic singleton class is left behind.MethodVisibilityDefinition::id()includes flag bitsTwo defs at the same source location share the same
uri_id,offset, andstr_id. The ID hash now includes flag bits so the siblings get distinctDefinitionIds. Without this,add_definitionpanics on collision.In this PR
MethodVisibilitydefs formodule_function :sym(one per side; singleton-side carriesSINGLETON_METHOD_VISIBILITY)Visibility::ModuleFunctioninsideresolve_method_visibilitiesfor the cross-namespace lookup: target on the lexical owner, declaration on the singleton classget_or_create_singleton_classfor the singleton-side def until the target is confirmedVisibility::ModuleFunctiontoPublic(singleton) orPrivate(instance) inGraph::visibility()based on the flagDefinitionFlagsbits inMethodVisibilityDefinition::id()so siblings get distinct IDsNot in this PR
The inline form (
module_function def bar; end) already works through the indexer's pre-existing two-MethodDefinitionpath; no changes needed here.Ruby's exact runtime behavior for
module_function :a, :missingis not modeled. Ruby raises on:missingonly afterFoo#ahas been made private, and commits no singleton copies for the call. Rubydex resolves each argument independently, somodule_function :a, :missingmakesFoo#aprivate AND addsFoo.aas 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.