Skip to content

feat: Don't suggest unstable items on stable toolchain #14549

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

Merged
merged 3 commits into from
Apr 11, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Apr 11, 2023

Closes #3020

This PR implements stability check in ide-completion so that unstable items are only suggested if you're on nightly toolchain.

It's a bit unfortunate CompletionContext::check_stability() is spammed all over the crate, but we should call it before building CompletionItem as you cannot get attributes on the item it's completing from that struct. I looked up every callsite of Builder::add_to(), Completions::add[_opt](), andCompletions::add_all() and inserted the check wherever necessary.

The tests are admittedly incomplete in that I didn't add tests for every kind of item as I thought that would be too big and not worthwhile. I copy-pasted some existing basic tests in every test module and adjusted them.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 11, 2023
@Veykril
Copy link
Member

Veykril commented Apr 11, 2023

It's a bit unfortunate CompletionContext::check_stability() is spammed all over the crate, but we should call it before building CompletionItem as you cannot get attributes on the item it's completing from that struct.

This happens with a lot of additions we'd want, its a bit unfortunate but I am unsure how we can restructure things to prevent that so it's fine.

Also related issue to the PR #3021

@Veykril
Copy link
Member

Veykril commented Apr 11, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Apr 11, 2023

📌 Commit e6e4872 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 11, 2023

⌛ Testing commit e6e4872 with merge fd27621...

@bors
Copy link
Contributor

bors commented Apr 11, 2023

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

!ctx.is_item_hidden(&import.item_to_import)
&& !ctx.is_item_hidden(&import.original_item)
let item = &import.item_to_import;
!ctx.is_item_hidden(item)
Copy link
Contributor

@hecatia-elegua hecatia-elegua Apr 11, 2023

Choose a reason for hiding this comment

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

Did you want to do !ctx.is_item_hidden(&import.original_item) here?
The preview window doesn't show it but you're doing !ctx.is_item_hidden(item) twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, you're right! Thanks for catching this. Will file another PR shortly!

&& !ctx.is_item_hidden(&import.original_item)
let item = &import.item_to_import;
!ctx.is_item_hidden(item)
&& !ctx.is_item_hidden(item)
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well

bors added a commit that referenced this pull request Apr 11, 2023
@lnicola lnicola changed the title Don't suggest unstable items on stable toolchain feat: Don't suggest unstable items on stable toolchain Apr 12, 2023
@lnicola
Copy link
Member

lnicola commented Apr 17, 2023

Um, how do I make this work?

image

@lowr
Copy link
Contributor Author

lowr commented Apr 17, 2023

@lnicola Looks like our release channel detection procedure is wrong. I'm sorry I didn't notice this before landing, when I tested in my local env it worked, but I guess that was before I added None here.

Fix on the way!

@lnicola
Copy link
Member

lnicola commented Apr 17, 2023

@lowr no idea why, I have a vanilla rustup setup.

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.

Do not propose unstable features on autocomplete for stable toolchain
6 participants