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: add trait_impl_reduntant_assoc_item diagnostic #15990

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

Young-Flash
Copy link
Member

part of #15958, will try to add quickfix for the diagnostic if this PR is ok with you guys

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2023
crates/hir/src/diagnostics.rs Outdated Show resolved Hide resolved
crates/hir/src/lib.rs Outdated Show resolved Hide resolved
@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 30, 2023
@Young-Flash Young-Flash force-pushed the trait_impl_reduntant_assoc_item branch from d18ead5 to 647eb2b Compare December 1, 2023 11:12
@Young-Flash
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 1, 2023
@Young-Flash Young-Flash requested a review from Veykril December 3, 2023 05:00
@Young-Flash
Copy link
Member Author

@Veykril @lnicola could you help review this when you are free?

@Veykril
Copy link
Member

Veykril commented Dec 5, 2023

Small nit but otherwise lgtm. We should definitely look into adding some more utility functions for creating better diagnostic text ranges for things (currently most assists still create the ranges in some random ways).

@bors delegate+

@bors
Copy link
Contributor

bors commented Dec 5, 2023

✌️ @Young-Flash, you can now approve this pull request!

If @Veykril told you to "r=me" after making some further change, please make that change, then do @bors r=@Veykril

@Young-Flash Young-Flash force-pushed the trait_impl_reduntant_assoc_item branch from 731a3df to fbe494a Compare December 7, 2023 12:46
@Young-Flash
Copy link
Member Author

@bors r=@Veykril

@bors
Copy link
Contributor

bors commented Dec 7, 2023

📌 Commit fbe494a has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 7, 2023

⌛ Testing commit fbe494a with merge 421a0a4...

@bors
Copy link
Contributor

bors commented Dec 7, 2023

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

@bors bors merged commit 421a0a4 into rust-lang:master Dec 7, 2023
9 checks passed
@Young-Flash Young-Flash deleted the trait_impl_reduntant_assoc_item branch December 8, 2023 02:13
@lnicola
Copy link
Member

lnicola commented Dec 11, 2023

image

@saiintbrisson
Copy link
Contributor

saiintbrisson commented Jan 5, 2024

Hi! This feature introduced a bug when unwrapping this file_id:

FileRange { file_id: d.file_id.file_id().unwrap(), range: diagnostic_range },

The function returns None if the file is a macro file:

#[inline]
pub fn file_id(self) -> Option<FileId> {
match self.0 & Self::MACRO_FILE_TAG_MASK {
0 => Some(FileId::from_raw(self.0)),
_ => None,
}
}

Maybe changing it to something like trait_impl_missing_assoc_item will work?

Diagnostic::new(
DiagnosticCode::RustcHardError("E0046"),
format!("not all trait items implemented, missing: {missing}"),
adjusted_display_range::<ast::Impl>(
ctx,
InFile { file_id: d.file_id, value: d.impl_.syntax_node_ptr() },
&|impl_| impl_.trait_().map(|t| t.syntax().text_range()),
),
)

bors added a commit that referenced this pull request Jan 5, 2024
…eykril

feat: add quickfix for redundant_assoc_item diagnostic

Happy New Year 😊

follow up #15990, now it's time to close #15958, closes #16269

![demo](https://github.com/rust-lang/rust-analyzer/assets/71162630/74022c52-1566-49a0-9be8-03b82f3e730f)

EDIT: add a demo.git would be more illustrated when making release change log.
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