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

Return semantic tokens for local vars shadowed by parameters #2509

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

vinistock
Copy link
Member

Motivation

There was an oversight from by part in #2482. There's still one scenario where we don't want to avoid returning local variable tokens for all assignment types: when the variable is being shadowed by a parameter.

For example

def foo(a)
  # This should be highlighted as a parameter, since it was first declared as one
  # inside this scope. And the developer should know that they are overriding a parameter
  a = 123
end

Implementation

Essentially, I brought back all of the methods removed in #2482, but now we only return tokens if the variable has been shadowed by a parameter.

Automated Tests

Added a fixture and expectation that captures all cases.

@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Aug 28, 2024
@vinistock vinistock self-assigned this Aug 28, 2024
@vinistock vinistock requested a review from a team as a code owner August 28, 2024 19:51
@vinistock vinistock requested review from andyw8 and st0012 August 28, 2024 19:51
@vinistock vinistock enabled auto-merge (squash) August 28, 2024 19:58
@vinistock vinistock merged commit c0b8aa1 into main Aug 28, 2024
35 checks passed
@vinistock vinistock deleted the vs-return-semantic-tokens-for-shadowed-vars branch August 28, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug 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