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

feat: goto definition on range operators #18362

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Conversation

duncpro
Copy link
Contributor

@duncpro duncpro commented Oct 21, 2024

Closes #18342

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 21, 2024
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

(shouldve said that in the issue as a mentoring instruction sorry)
I think we should add a couple new OperatorClasses (OperatorClass::Range(Struct), etc) here for the range types https://github.com/rust-analyzer/rust-analyzer/blob/fb832ff2ba338105dda1c8172f89d580cb0a570f/crates/ide-db/src/defs.rs#L548-L554 and populate them here https://github.com/rust-analyzer/rust-analyzer/blob/fb832ff2ba338105dda1c8172f89d580cb0a570f/crates/ide-db/src/defs.rs#L309-L330

WIth that I think the go to def code needs no changes in fact (aside from tests) and this should automatically give us support for ranges in other ide features as well (like hover and stuff)

Comment on lines 356 to 359
let path = match range_expr.op_kind()? {
RangeOp::Exclusive => path![core::ops::Range],
RangeOp::Inclusive => path![core::ops::RangeInclusive],
};
Copy link
Member

Choose a reason for hiding this comment

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

There is a bit more to check, we need to check if the first or second element is missing or not as well, so this should ultimately be able to produce one of Range, RangeFrom, RangeInclusive, RangeFull, RangeTo and RangeToInclusive

@Veykril
Copy link
Member

Veykril commented Oct 22, 2024

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Oct 22, 2024

📌 Commit 2f6923b has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 22, 2024

⌛ Testing commit 2f6923b with merge 17055aa...

@bors
Copy link
Contributor

bors commented Oct 22, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 17055aa to master...

@bors bors merged commit 17055aa into rust-lang:master Oct 22, 2024
11 checks passed
@duncpro
Copy link
Contributor Author

duncpro commented Oct 22, 2024

@Veykril Thank you for helping me along. This was my first real OSS contribution.

@Veykril
Copy link
Member

Veykril commented Oct 22, 2024

Ah, if you want there is a follow up task. We can do the same for patterns I just realized, not just expressions since there are also range patterns :)

@duncpro
Copy link
Contributor Author

duncpro commented Oct 22, 2024

I'm down. I'll open an issue for it right now

@rust-lang rust-lang deleted a comment Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go to definition on ../..= should go to the corresponding Range* type
4 participants