Provide constant type differentiation in semantic highlighting request

[mateusdeap]

Motivation

This is a new version of #1148 which closed when I tried to update my fork of the repo. Also making this a draft to gather feedback on my solution and give me time to do proper testing, since I use neovim and I'll probably need to figure some things out in that area.

Fixes #1128 (Potentially)

This PR aims to modify the SemanticHighlighting class so that it can properly determine what type of constant is being referenced in a given piece of code. The details and discussion can be found in the issue, but we want syntax highlighting to know that Foo in Foo.new below is referencing the class Foo and isn't just some other constant Foo, for example:

class Foo
  THIS_IS_A_CONSTANT = 1

  def self.create_instance
    Foo.new
  end
end

Implementation

Following @vinistock's recommendations, I made sure an instance of RubyIndexer::Index is given to the request and created a variable to keep track of nesting. Additionally subscribed to events needed to keep track of this nesting.

At the time of submission, the PR is incomplete, however. I wasn't able to figure out how to properly set up the index in the tests, so the lines where I call index_single are definitely something that needs fixing.

Finally, one thing discussed in the issue is that, even if this is implemented correctly, it might be too resource intensive, so if this gets finished, benchmarks will be required

Automated Tests

Haven't added any new tests so far but only tried to modify the current test to reflect the needed changes.

Manual Tests

The issue was opened referencing neovim, so I'm not sure if this affects VS Code. However the request doesn't have this specific capability so I think it's surely possible to replicate this by opening the example file provided in the issue.

[vinistock]

This token type doesn't exist in the specification. We can't add it here because editors won't know what to do with that type.

We have two options here. We can use static variable as the token type for constants or we can propose the addition of constant as a token type to the specification.

[mateusdeap]

Interesting. I was completely unaware of this.

Particularly, I'll just use the static variable, but do you think there is value in adding the constant token type to the spec? I'm assuming this might be an issue that the spec is built thinking of wording that doesn't exactly match Ruby concepts. So maybe static variable would be the equivalent of a Ruby constant. Despite the glaring contradiction in naming.

[vinistock]

Constant reads are references to constants, not declarations. So you need to remove the modifiers on all of these add_token calls.

[vinistock]

Let's not commit these changes.

[vinistock]

Instead of indexing every fixture here, we should use the source we receive in this method instead. Something like

index.index_single(
  RubyIndexer::IndexablePath.new(
    "fake",
    "fake/path.rb",
  ),
  source
)

Otherwise, we'd be indexing every fixture file in the codebase every time we're running the test for a single fixture.

[github-actions]

This pull request is being marked as stale because there was no activity in the last 2 months