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

Add find references support for constants #2632

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Conversation

vinistock
Copy link
Member

Motivation

Closes #202

With the same infrastructure added in #2626, we can easily support finding constant references. Instead of building an array of text edits, we just return an array of locations.

Implementation

The idea is to use the same reference finder implementation and then return all of the hits back as locations.

I also had to fix a small duplication issue to be able to respect the LSP spec's includeDeclaration parameter.

Automated Tests

Added tests.

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Sep 30, 2024
@vinistock vinistock self-assigned this Sep 30, 2024
@vinistock vinistock requested a review from a team as a code owner September 30, 2024 14:12
@vinistock vinistock requested review from andyw8 and st0012 September 30, 2024 14:12
@andyw8
Copy link
Contributor

andyw8 commented Sep 30, 2024

Do we need any safeguards for someone running Find References on a large codebase for some constant with a high number of occurrences?

Copy link
Contributor

@andyw8 andyw8 left a comment

Choose a reason for hiding this comment

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

Very exciting to have this.

jekyll/index.markdown Outdated Show resolved Hide resolved
jekyll/index.markdown Outdated Show resolved Hide resolved
lib/ruby_lsp/requests/references.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/internal.rb Outdated Show resolved Hide resolved
lib/ruby_indexer/lib/ruby_indexer/reference_finder.rb Outdated Show resolved Hide resolved
@vinistock vinistock force-pushed the vs-add-references-support branch from 8d1136c to 6b96c8c Compare October 1, 2024 12:58
@vinistock vinistock requested a review from andyw8 October 1, 2024 13:00
@andyw8
Copy link
Contributor

andyw8 commented Oct 1, 2024

I found one issue:

Create a new file, but don't save it:

class Widget2
  def bar
  end
end

Widget2

Right-click on Widget2 in the first line then Find All References

The LSP will error with:

Ruby LSP error NoMethodError: undefined method `full_name' for an instance of Prism::ProgramNode /Users/andyw8/src/github.com/Shopify/ruby-lsp/lib/ruby_lsp/requests/support/common.rb:152:in `constant_name'

@vinistock
Copy link
Member Author

The issue was related to not finding a target. I fixed it in the latest push.

@vinistock vinistock force-pushed the vs-add-rename-support branch 5 times, most recently from 21f56af to 377e988 Compare October 2, 2024 20:18
Base automatically changed from vs-add-rename-support to main October 2, 2024 20:39
@vinistock vinistock force-pushed the vs-add-references-support branch from 933a025 to 4274afd Compare October 2, 2024 22:04
@vinistock vinistock enabled auto-merge (squash) October 2, 2024 22:05
@vinistock vinistock merged commit 970b466 into main Oct 2, 2024
35 checks passed
@vinistock vinistock deleted the vs-add-references-support branch October 2, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants