Auto merge of #15895 - Veykril:unsafe-impls-diagnostic, r=Veykril

Diagnose missing assoc items in trait impls

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)>,
+}

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) {
+            if let Some(trait_) = trait_ {
-                let def: DefWithBody = match item {
+                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,

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(
+    Diagnostic::new(
-        ctx,
         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
 "#,
         );
     }

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`
+}
+
+"#,
+        );
+    }
+}

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() {

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),

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();