Skip to content

Index calls to private_constant and public_constant#506

Open
Morriar wants to merge 2 commits intomainfrom
at-private-const-def
Open

Index calls to private_constant and public_constant#506
Morriar wants to merge 2 commits intomainfrom
at-private-const-def

Conversation

@Morriar
Copy link
Contributor

@Morriar Morriar commented Jan 22, 2026

One more step towards #89 and #354. We start keeping track of calls to private_constant and public_constant.

For these ones I chose to keep a new definition around because we'll need to resolve the names to be able to apply the visibility to the right constant.

Take this code for example:

# file1.rb
class Foo
  Bar = 42
end

# file2.rb
Foo.private_constant(:Bar)

there is no way to handle it within the indexer.

Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar requested a review from a team as a code owner January 22, 2026 16:11
@Morriar Morriar changed the title At private const def Index calls to private_constant and public_constant Jan 22, 2026
@vinistock
Copy link
Member

I like the idea of storing the visibility calls so that we can process them correctly during resolution, but I wonder if we need a new concept here other than definition. This is a very similar case as #362, where we would also like to remember this for resolution. It seems that these cases mutate existing definitions.

What do you think of having a separate concept for this? Maybe a new directives node hashmap?

Base automatically changed from at-clean-indexer-asserts to main January 26, 2026 15:01
@Morriar
Copy link
Contributor Author

Morriar commented Jan 27, 2026

I like the idea of storing the visibility calls so that we can process them correctly during resolution, but I wonder if we need a new concept here other than definition. This is a very similar case as #362, where we would also like to remember this for resolution. It seems that these cases mutate existing definitions.

What do you think of having a separate concept for this? Maybe a new directives node hashmap?

The thing is, I want the private_const(:Foo) to be stored has one of the definitions for the declaration Foo. It makes it easy to go to the definitions and rename them without a special logic.

@vinistock
Copy link
Member

It makes it easy to go to the definitions and rename them without a special logic.

I think these should appear in find references (which is the same as rename), but not go to definition. I'd find it surprising if going to definition would take me to a private_const call. I'm thinking something like this:

Foo.private_const(:Bar)
# ^ constant reference to Foo
#                   ^ constant reference to Bar

I'd expect these to show up when finding references, but not when going to definition. I see it as something like this:

enum Operation {
  PrivateConstant,
  Mixin,
}

struct Directive {
  receiver: ReferenceId,
  target: ReferenceId,
  operation: Operation
}

This would also support private_singleton_method, where you'd have a constant reference as the receiver and a method reference as the target. What do you think?

@Morriar
Copy link
Contributor Author

Morriar commented Jan 30, 2026

enum Operation {
  PrivateConstant,
  Mixin,
}

struct Directive {
  receiver: ReferenceId,
  target: ReferenceId,
  operation: Operation
}

This would also support private_singleton_method, where you'd have a constant reference as the receiver and a method reference as the target. What do you think?

How would we link it back from the declaration? We add a new directives field to each declaration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants