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: Diagnose missing assoc items in trait impls #15895

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Nov 14, 2023

No description provided.

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

Veykril commented Nov 14, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 14, 2023

📌 Commit 723d799 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 14, 2023

⌛ Testing commit 723d799 with merge c1e65aa...

@bors
Copy link
Collaborator

bors commented Nov 14, 2023

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

@bors bors merged commit c1e65aa into rust-lang:master Nov 14, 2023
10 checks passed
@Veykril Veykril changed the title Diagnose missing assoc items in trait impls feat: Diagnose missing assoc items in trait impls Nov 15, 2023
match item {
AssocItemId::FunctionId(it) => db.function_data(it).name.clone(),
AssocItemId::ConstId(it) => {
db.const_data(it).name.as_ref().unwrap().clone()
Copy link
Member Author

Choose a reason for hiding this comment

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

Just realized I forgot to replace the unwrap here. @Young-Flash since you are looking into #15909 already, do you mind changing this to ? and make this map a filter_map?

Copy link
Member

Choose a reason for hiding this comment

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

sure

@lnicola
Copy link
Member

lnicola commented Nov 20, 2023

image

bors added a commit that referenced this pull request Nov 20, 2023
fix: handle default constant values in `trait_impl_missing_assoc_item` diagnostic

A patch of #15895, close #15909

cc `@Veykril`
@jnicholls
Copy link

jnicholls commented Nov 21, 2023

This feature seems like a regression to me, or at the very least is not representative of compiler behavior. An associated const can have a default value on the trait, allowing implementations to inherit the default value. Since updating to v0.3.1740 I'm getting some false positive errors due to this feature.

Example of error-free (rustc) code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=4a8dad8097690736315ab7e64f19d0c9

trait MyTrait {
    const VALUE: &'static str = "Hello World";
}

struct MyType1;
impl MyTrait for MyType1 {}

struct MyType2;
impl MyTrait for MyType2 {
    const VALUE: &'static str = "Goodbye World";
}

fn main() {
    println!("{}", MyType1::VALUE);
    println!("{}", MyType2::VALUE);
}

The above code results in a rust-analyzer error on the impl MyTrait for MyType1 for not providing a value for the associated const VALUE.

@lnicola
Copy link
Member

lnicola commented Nov 21, 2023

@jnicholls can you use the pre-release version until next week?

@jnicholls
Copy link

@lnicola Thanks the the wicked fast reply! Yes I can use the pre-release version, no problem.

@jnicholls
Copy link

Apologies, I didn't notice #15911 up above. Good deal! Have a great rest of your week.

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.

6 participants