Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide constant type differentiation in semantic highlighting request #1148

Closed
wants to merge 0 commits into from

Conversation

mateusdeap
Copy link
Contributor

@mateusdeap mateusdeap commented Oct 27, 2023

Motivation

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.

Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping track of the nesting is looking good. Now, when we find a constant reference, we need to use the constant name, the nesting and the index to determine what the constant is declared as.

def on_constant_read_node_enter(node)
  entries = @index.resolve(node.name, @nesting)
  # if entries is nil, we didn't find the declaration for it
  # it might be defined with meta-programming or using C code
  unless entries
    # Push a constant semantic token
  end

  # Otherwise, we can check the type of entry to determine the
  # type of constant
  first_entry = T.must(entries.first)

  case first_entry
  when RubyIndexer::Entry::Class
    # push a class.declaration token
  when RubyIndexer::Entry::Module
    # push a namespace.declaration token
  when RubyIndexer::Entry::Constant
    # push a constant.declaration token
  end
end

@mateusdeap
Copy link
Contributor Author

Awesome. Will try to implement this as soon as I can.

@mateusdeap
Copy link
Contributor Author

Closed it by accident while trying to update my main branch. Finally found some time to squeeze out some work here, so I'll be making updates soon

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.

Semantic highlighting marking a class usage as a constant
2 participants