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

Suggest type completions for type arguments and constant completions for constant arguments #15383

Merged
merged 15 commits into from Aug 15, 2023

Conversation

max-heller
Copy link
Contributor

When determining completions for generic arguments, suggest only types or only constants if the corresponding generic parameter is a type parameter or constant parameter.

Closes #12568

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 2, 2023
crates/ide-completion/src/context/analysis.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/tests/type_pos.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context/analysis.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context/analysis.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/completions/type.rs Outdated Show resolved Hide resolved
@Veykril
Copy link
Member

Veykril commented Aug 5, 2023

The test failures seem correct given the changes, you'll just have to update their outputs

@max-heller max-heller requested a review from Veykril August 6, 2023 16:09
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/tests/type_pos.rs Outdated Show resolved Hide resolved
Comment on lines +731 to +747
check(
r#"
struct Foo;
const X: usize = 0;
fn foo<T, const N: usize>() {}
fn main() {
foo::<_, $0>();
}
"#,
expect![[r#"
ct CONST
ct X
ma makro!(…) macro_rules! makro
kw crate::
kw self::
"#]],
);
Copy link
Member

Choose a reason for hiding this comment

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

Here we actually still want to show the Foo struct because it could potentially have an associated const taht we want to complete. Its fine to put a FIXME on this for now as to not overload this PR with too many things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we complete associated constants e.g. Foo::X if they exist but not Foo itself?

Copy link
Contributor Author

@max-heller max-heller Aug 9, 2023

Choose a reason for hiding this comment

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

Added a FIXME. Also not sure how this should interact with the fact that foo::<usize::MAX>() is invalid (it should be foo::<{ usize::MAX }>()). It'd be nice if we could get the completion to suggest { usize::MAX } instead of just usize, the latter making less sense if you don't know it has associated constants

Copy link
Member

Choose a reason for hiding this comment

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

Could we complete associated constants e.g. Foo::X if they exist but not Foo itself?

We probably could, likewise for const function calls

max-heller and others added 2 commits August 8, 2023 20:05
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
@max-heller max-heller requested a review from Veykril August 9, 2023 01:02
@Veykril
Copy link
Member

Veykril commented Aug 15, 2023

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Aug 15, 2023

📌 Commit fb98f52 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 15, 2023

⌛ Testing commit fb98f52 with merge f73cd39...

@bors
Copy link
Collaborator

bors commented Aug 15, 2023

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

@bors bors merged commit f73cd39 into rust-lang:master Aug 15, 2023
10 checks passed
@max-heller max-heller deleted the issue-12568 branch August 15, 2023 15:11
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.

When completing generic arguments we shoul differentiate between completing const and type arguments
4 participants