Unnamed repository; edit this file 'description' to name the repository.
Diagnose missing assoc items in trait impls
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 8 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 50 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs | 40 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/trait_impl_missing_assoc_item.rs | 102 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/type_mismatch.rs | 2 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 4 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/tests.rs | 3 |
7 files changed, 191 insertions, 18 deletions
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index dd5c9b3535..cf9a2b73d9 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -54,6 +54,7 @@ diagnostics![ PrivateField, ReplaceFilterMapNextWithFindMap, TraitImplIncorrectSafety, + TraitImplMissingAssocItems, TraitImplOrphan, TypedHole, TypeMismatch, @@ -302,3 +303,10 @@ pub struct TraitImplIncorrectSafety { pub impl_: AstPtr<ast::Impl>, pub should_be_safe: bool, } + +#[derive(Debug, PartialEq, Eq)] +pub struct TraitImplMissingAssocItems { + pub file_id: HirFileId, + pub impl_: AstPtr<ast::Impl>, + pub missing: Vec<(Name, AssocItem)>, +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 066d7e635a..e0d27f5262 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -33,7 +33,7 @@ pub mod symbols; mod display; -use std::{iter, ops::ControlFlow}; +use std::{iter, mem::discriminant, ops::ControlFlow}; use arrayvec::ArrayVec; use base_db::{CrateDisplayName, CrateId, CrateOrigin, Edition, FileId, ProcMacroKind}; @@ -593,6 +593,7 @@ impl Module { let inherent_impls = db.inherent_impls_in_crate(self.id.krate()); + let mut impl_assoc_items_scratch = vec![]; for impl_def in self.impl_defs(db) { let loc = impl_def.id.lookup(db.upcast()); let tree = loc.id.item_tree(db.upcast()); @@ -661,8 +662,51 @@ impl Module { _ => (), }; - for item in impl_def.items(db) { - let def: DefWithBody = match item { + if let Some(trait_) = trait_ { + let items = &db.trait_data(trait_.into()).items; + let required_items = items.iter().filter(|&(_, assoc)| match *assoc { + AssocItemId::FunctionId(it) => !db.function_data(it).has_body(), + AssocItemId::ConstId(_) => true, + AssocItemId::TypeAliasId(it) => db.type_alias_data(it).type_ref.is_none(), + }); + impl_assoc_items_scratch.extend(db.impl_data(impl_def.id).items.iter().map( + |&item| { + ( + item, + match item { + AssocItemId::FunctionId(it) => db.function_data(it).name.clone(), + AssocItemId::ConstId(it) => { + db.const_data(it).name.as_ref().unwrap().clone() + } + AssocItemId::TypeAliasId(it) => db.type_alias_data(it).name.clone(), + }, + ) + }, + )); + + let missing: Vec<_> = required_items + .filter(|(name, id)| { + !impl_assoc_items_scratch.iter().any(|(impl_item, impl_name)| { + discriminant(impl_item) == discriminant(id) && impl_name == name + }) + }) + .map(|(name, item)| (name.clone(), AssocItem::from(*item))) + .collect(); + if !missing.is_empty() { + acc.push( + TraitImplMissingAssocItems { + impl_: ast_id_map.get(node.ast_id()), + file_id, + missing, + } + .into(), + ) + } + impl_assoc_items_scratch.clear(); + } + + for &item in &db.impl_data(impl_def.id).items { + let def: DefWithBody = match AssocItem::from(item) { AssocItem::Function(it) => it.into(), AssocItem::Const(it) => it.into(), AssocItem::TypeAlias(_) => continue, diff --git a/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs b/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs index f28298de7d..251a645292 100644 --- a/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs +++ b/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs @@ -1,6 +1,7 @@ use hir::InFile; +use syntax::ast; -use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, Severity}; +use crate::{adjusted_display_range, Diagnostic, DiagnosticCode, DiagnosticsContext, Severity}; // Diagnostic: trait-impl-incorrect-safety // @@ -9,15 +10,28 @@ pub(crate) fn trait_impl_incorrect_safety( ctx: &DiagnosticsContext<'_>, d: &hir::TraitImplIncorrectSafety, ) -> Diagnostic { - Diagnostic::new_with_syntax_node_ptr( - ctx, + Diagnostic::new( DiagnosticCode::Ra("trait-impl-incorrect-safety", Severity::Error), if d.should_be_safe { "unsafe impl for safe trait" } else { "impl for unsafe trait needs to be unsafe" }, - InFile::new(d.file_id, d.impl_.clone().into()), + adjusted_display_range::<ast::Impl>( + ctx, + InFile { file_id: d.file_id, value: d.impl_.syntax_node_ptr() }, + &|impl_| { + if d.should_be_safe { + Some(match (impl_.unsafe_token(), impl_.impl_token()) { + (None, None) => return None, + (None, Some(t)) | (Some(t), None) => t.text_range(), + (Some(t1), Some(t2)) => t1.text_range().cover(t2.text_range()), + }) + } else { + impl_.impl_token().map(|t| t.text_range()) + } + }, + ), ) } @@ -35,10 +49,10 @@ unsafe trait Unsafe {} impl Safe for () {} impl Unsafe for () {} -//^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe +//^^^^ error: impl for unsafe trait needs to be unsafe unsafe impl Safe for () {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait +//^^^^^^^^^^^ error: unsafe impl for safe trait unsafe impl Unsafe for () {} "#, @@ -57,20 +71,20 @@ struct L<'l>; impl<T> Drop for S<T> {} impl<#[may_dangle] T> Drop for S<T> {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe +//^^^^ error: impl for unsafe trait needs to be unsafe unsafe impl<T> Drop for S<T> {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait +//^^^^^^^^^^^ error: unsafe impl for safe trait unsafe impl<#[may_dangle] T> Drop for S<T> {} impl<'l> Drop for L<'l> {} impl<#[may_dangle] 'l> Drop for L<'l> {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe +//^^^^ error: impl for unsafe trait needs to be unsafe unsafe impl<'l> Drop for L<'l> {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait +//^^^^^^^^^^^ error: unsafe impl for safe trait unsafe impl<#[may_dangle] 'l> Drop for L<'l> {} "#, @@ -86,14 +100,14 @@ trait Trait {} impl !Trait for () {} unsafe impl !Trait for () {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait +//^^^^^^^^^^^ error: unsafe impl for safe trait unsafe trait UnsafeTrait {} impl !UnsafeTrait for () {} unsafe impl !UnsafeTrait for () {} -//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait +//^^^^^^^^^^^ error: unsafe impl for safe trait "#, ); @@ -108,7 +122,7 @@ struct S; impl S {} unsafe impl S {} -//^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait +//^^^^^^^^^^^ error: unsafe impl for safe trait "#, ); } diff --git a/crates/ide-diagnostics/src/handlers/trait_impl_missing_assoc_item.rs b/crates/ide-diagnostics/src/handlers/trait_impl_missing_assoc_item.rs new file mode 100644 index 0000000000..40d0b6fdd4 --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/trait_impl_missing_assoc_item.rs @@ -0,0 +1,102 @@ +use hir::InFile; +use itertools::Itertools; +use syntax::{ast, AstNode}; + +use crate::{adjusted_display_range, Diagnostic, DiagnosticCode, DiagnosticsContext}; + +// Diagnostic: trait-impl-missing-assoc_item +// +// Diagnoses missing trait items in a trait impl. +pub(crate) fn trait_impl_missing_assoc_item( + ctx: &DiagnosticsContext<'_>, + d: &hir::TraitImplMissingAssocItems, +) -> Diagnostic { + let missing = d.missing.iter().format_with(", ", |(name, item), f| { + f(&match *item { + hir::AssocItem::Function(_) => "`fn ", + hir::AssocItem::Const(_) => "`const ", + hir::AssocItem::TypeAlias(_) => "`type ", + })?; + f(&name.display(ctx.sema.db))?; + f(&"`") + }); + 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()), + ), + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn simple() { + check_diagnostics( + r#" +trait Trait { + const C: (); + type T; + fn f(); +} + +impl Trait for () { + const C: () = (); + type T = (); + fn f() {} +} + +impl Trait for () { + //^^^^^ error: not all trait items implemented, missing: `const C` + type T = (); + fn f() {} +} + +impl Trait for () { + //^^^^^ error: not all trait items implemented, missing: `const C`, `type T`, `fn f` +} + +"#, + ); + } + + #[test] + fn default() { + check_diagnostics( + r#" +trait Trait { + const C: (); + type T = (); + fn f() {} +} + +impl Trait for () { + const C: () = (); + type T = (); + fn f() {} +} + +impl Trait for () { + //^^^^^ error: not all trait items implemented, missing: `const C` + type T = (); + fn f() {} +} + +impl Trait for () { + //^^^^^ error: not all trait items implemented, missing: `const C` + type T = (); + } + +impl Trait for () { + //^^^^^ error: not all trait items implemented, missing: `const C` +} + +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index 14454fe8dc..16d8ed64ad 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -278,6 +278,7 @@ struct Foo; struct Bar; impl core::ops::Deref for Foo { type Target = Bar; + fn deref(&self) -> &Self::Target { loop {} } } fn main() { @@ -290,6 +291,7 @@ struct Foo; struct Bar; impl core::ops::Deref for Foo { type Target = Bar; + fn deref(&self) -> &Self::Target { loop {} } } fn main() { diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 99921c1107..6744895f3c 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -46,6 +46,7 @@ mod handlers { pub(crate) mod replace_filter_map_next_with_find_map; pub(crate) mod trait_impl_orphan; pub(crate) mod trait_impl_incorrect_safety; + pub(crate) mod trait_impl_missing_assoc_item; pub(crate) mod typed_hole; pub(crate) mod type_mismatch; pub(crate) mod unimplemented_builtin_macro; @@ -360,8 +361,9 @@ pub fn diagnostics( AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d), AnyDiagnostic::PrivateField(d) => handlers::private_field::private_field(&ctx, &d), AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => handlers::replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), - AnyDiagnostic::TraitImplOrphan(d) => handlers::trait_impl_orphan::trait_impl_orphan(&ctx, &d), AnyDiagnostic::TraitImplIncorrectSafety(d) => handlers::trait_impl_incorrect_safety::trait_impl_incorrect_safety(&ctx, &d), + AnyDiagnostic::TraitImplMissingAssocItems(d) => handlers::trait_impl_missing_assoc_item::trait_impl_missing_assoc_item(&ctx, &d), + AnyDiagnostic::TraitImplOrphan(d) => handlers::trait_impl_orphan::trait_impl_orphan(&ctx, &d), AnyDiagnostic::TypedHole(d) => handlers::typed_hole::typed_hole(&ctx, &d), AnyDiagnostic::TypeMismatch(d) => handlers::type_mismatch::type_mismatch(&ctx, &d), AnyDiagnostic::UndeclaredLabel(d) => handlers::undeclared_label::undeclared_label(&ctx, &d), diff --git a/crates/ide-diagnostics/src/tests.rs b/crates/ide-diagnostics/src/tests.rs index ff8f3b2686..c766a018bf 100644 --- a/crates/ide-diagnostics/src/tests.rs +++ b/crates/ide-diagnostics/src/tests.rs @@ -43,7 +43,8 @@ fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) { super::diagnostics(&db, &conf, &AssistResolveStrategy::All, file_position.file_id) .pop() .expect("no diagnostics"); - let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth]; + let fix = + &diagnostic.fixes.expect(&format!("{:?} diagnostic misses fixes", diagnostic.code))[nth]; let actual = { let source_change = fix.source_change.as_ref().unwrap(); let file_id = *source_change.source_file_edits.keys().next().unwrap(); |