Skip to content

Commit c1e65aa

Browse files
borsbors
committed
Auto merge of #15895 - Veykril:unsafe-impls-diagnostic, r=Veykril
Diagnose missing assoc items in trait impls
2 parents 989000b + 723d799 commit c1e65aa

File tree

7 files changed

+191
-18
lines changed

7 files changed

+191
-18
lines changed

‎crates/hir/src/diagnostics.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ diagnostics![
5454
PrivateField,
5555
ReplaceFilterMapNextWithFindMap,
5656
TraitImplIncorrectSafety,
57+
TraitImplMissingAssocItems,
5758
TraitImplOrphan,
5859
TypedHole,
5960
TypeMismatch,
@@ -302,3 +303,10 @@ pub struct TraitImplIncorrectSafety {
302303
pub impl_: AstPtr<ast::Impl>,
303304
pub should_be_safe: bool,
304305
}
306+
307+
#[derive(Debug, PartialEq, Eq)]
308+
pub struct TraitImplMissingAssocItems {
309+
pub file_id: HirFileId,
310+
pub impl_: AstPtr<ast::Impl>,
311+
pub missing: Vec<(Name, AssocItem)>,
312+
}

‎crates/hir/src/lib.rs

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub mod symbols;
3333

3434
mod display;
3535

36-
use std::{iter, ops::ControlFlow};
36+
use std::{iter, mem::discriminant, ops::ControlFlow};
3737

3838
use arrayvec::ArrayVec;
3939
use base_db::{CrateDisplayName, CrateId, CrateOrigin, Edition, FileId, ProcMacroKind};
@@ -593,6 +593,7 @@ impl Module {
593593

594594
let inherent_impls = db.inherent_impls_in_crate(self.id.krate());
595595

596+
let mut impl_assoc_items_scratch = vec![];
596597
for impl_def in self.impl_defs(db) {
597598
let loc = impl_def.id.lookup(db.upcast());
598599
let tree = loc.id.item_tree(db.upcast());
@@ -661,8 +662,51 @@ impl Module {
661662
_ => (),
662663
};
663664

664-
for item in impl_def.items(db) {
665-
let def: DefWithBody = match item {
665+
if let Some(trait_) = trait_ {
666+
let items = &db.trait_data(trait_.into()).items;
667+
let required_items = items.iter().filter(|&(_, assoc)| match *assoc {
668+
AssocItemId::FunctionId(it) => !db.function_data(it).has_body(),
669+
AssocItemId::ConstId(_) => true,
670+
AssocItemId::TypeAliasId(it) => db.type_alias_data(it).type_ref.is_none(),
671+
});
672+
impl_assoc_items_scratch.extend(db.impl_data(impl_def.id).items.iter().map(
673+
|&item| {
674+
(
675+
item,
676+
match item {
677+
AssocItemId::FunctionId(it) => db.function_data(it).name.clone(),
678+
AssocItemId::ConstId(it) => {
679+
db.const_data(it).name.as_ref().unwrap().clone()
680+
}
681+
AssocItemId::TypeAliasId(it) => db.type_alias_data(it).name.clone(),
682+
},
683+
)
684+
},
685+
));
686+
687+
let missing: Vec<_> = required_items
688+
.filter(|(name, id)| {
689+
!impl_assoc_items_scratch.iter().any(|(impl_item, impl_name)| {
690+
discriminant(impl_item) == discriminant(id) && impl_name == name
691+
})
692+
})
693+
.map(|(name, item)| (name.clone(), AssocItem::from(*item)))
694+
.collect();
695+
if !missing.is_empty() {
696+
acc.push(
697+
TraitImplMissingAssocItems {
698+
impl_: ast_id_map.get(node.ast_id()),
699+
file_id,
700+
missing,
701+
}
702+
.into(),
703+
)
704+
}
705+
impl_assoc_items_scratch.clear();
706+
}
707+
708+
for &item in &db.impl_data(impl_def.id).items {
709+
let def: DefWithBody = match AssocItem::from(item) {
666710
AssocItem::Function(it) => it.into(),
667711
AssocItem::Const(it) => it.into(),
668712
AssocItem::TypeAlias(_) => continue,

‎crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use hir::InFile;
2+
use syntax::ast;
23

3-
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, Severity};
4+
use crate::{adjusted_display_range, Diagnostic, DiagnosticCode, DiagnosticsContext, Severity};
45

56
// Diagnostic: trait-impl-incorrect-safety
67
//
@@ -9,15 +10,28 @@ pub(crate) fn trait_impl_incorrect_safety(
910
ctx: &DiagnosticsContext<'_>,
1011
d: &hir::TraitImplIncorrectSafety,
1112
) -> Diagnostic {
12-
Diagnostic::new_with_syntax_node_ptr(
13-
ctx,
13+
Diagnostic::new(
1414
DiagnosticCode::Ra("trait-impl-incorrect-safety", Severity::Error),
1515
if d.should_be_safe {
1616
"unsafe impl for safe trait"
1717
} else {
1818
"impl for unsafe trait needs to be unsafe"
1919
},
20-
InFile::new(d.file_id, d.impl_.clone().into()),
20+
adjusted_display_range::<ast::Impl>(
21+
ctx,
22+
InFile { file_id: d.file_id, value: d.impl_.syntax_node_ptr() },
23+
&|impl_| {
24+
if d.should_be_safe {
25+
Some(match (impl_.unsafe_token(), impl_.impl_token()) {
26+
(None, None) => return None,
27+
(None, Some(t)) | (Some(t), None) => t.text_range(),
28+
(Some(t1), Some(t2)) => t1.text_range().cover(t2.text_range()),
29+
})
30+
} else {
31+
impl_.impl_token().map(|t| t.text_range())
32+
}
33+
},
34+
),
2135
)
2236
}
2337

@@ -35,10 +49,10 @@ unsafe trait Unsafe {}
3549
impl Safe for () {}
3650
3751
impl Unsafe for () {}
38-
//^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe
52+
//^^^^ error: impl for unsafe trait needs to be unsafe
3953
4054
unsafe impl Safe for () {}
41-
//^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait
55+
//^^^^^^^^^^^ error: unsafe impl for safe trait
4256
4357
unsafe impl Unsafe for () {}
4458
"#,
@@ -57,20 +71,20 @@ struct L<'l>;
5771
impl<T> Drop for S<T> {}
5872
5973
impl<#[may_dangle] T> Drop for S<T> {}
60-
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe
74+
//^^^^ error: impl for unsafe trait needs to be unsafe
6175
6276
unsafe impl<T> Drop for S<T> {}
63-
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait
77+
//^^^^^^^^^^^ error: unsafe impl for safe trait
6478
6579
unsafe impl<#[may_dangle] T> Drop for S<T> {}
6680
6781
impl<'l> Drop for L<'l> {}
6882
6983
impl<#[may_dangle] 'l> Drop for L<'l> {}
70-
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe
84+
//^^^^ error: impl for unsafe trait needs to be unsafe
7185
7286
unsafe impl<'l> Drop for L<'l> {}
73-
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait
87+
//^^^^^^^^^^^ error: unsafe impl for safe trait
7488
7589
unsafe impl<#[may_dangle] 'l> Drop for L<'l> {}
7690
"#,
@@ -86,14 +100,14 @@ trait Trait {}
86100
impl !Trait for () {}
87101
88102
unsafe impl !Trait for () {}
89-
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait
103+
//^^^^^^^^^^^ error: unsafe impl for safe trait
90104
91105
unsafe trait UnsafeTrait {}
92106
93107
impl !UnsafeTrait for () {}
94108
95109
unsafe impl !UnsafeTrait for () {}
96-
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait
110+
//^^^^^^^^^^^ error: unsafe impl for safe trait
97111
98112
"#,
99113
);
@@ -108,7 +122,7 @@ struct S;
108122
impl S {}
109123
110124
unsafe impl S {}
111-
//^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait
125+
//^^^^^^^^^^^ error: unsafe impl for safe trait
112126
"#,
113127
);
114128
}

‎crates/ide-diagnostics/src/handlers/trait_impl_missing_assoc_item.rs

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use hir::InFile;
2+
use itertools::Itertools;
3+
use syntax::{ast, AstNode};
4+
5+
use crate::{adjusted_display_range, Diagnostic, DiagnosticCode, DiagnosticsContext};
6+
7+
// Diagnostic: trait-impl-missing-assoc_item
8+
//
9+
// Diagnoses missing trait items in a trait impl.
10+
pub(crate) fn trait_impl_missing_assoc_item(
11+
ctx: &DiagnosticsContext<'_>,
12+
d: &hir::TraitImplMissingAssocItems,
13+
) -> Diagnostic {
14+
let missing = d.missing.iter().format_with(", ", |(name, item), f| {
15+
f(&match *item {
16+
hir::AssocItem::Function(_) => "`fn ",
17+
hir::AssocItem::Const(_) => "`const ",
18+
hir::AssocItem::TypeAlias(_) => "`type ",
19+
})?;
20+
f(&name.display(ctx.sema.db))?;
21+
f(&"`")
22+
});
23+
Diagnostic::new(
24+
DiagnosticCode::RustcHardError("E0046"),
25+
format!("not all trait items implemented, missing: {missing}"),
26+
adjusted_display_range::<ast::Impl>(
27+
ctx,
28+
InFile { file_id: d.file_id, value: d.impl_.syntax_node_ptr() },
29+
&|impl_| impl_.trait_().map(|t| t.syntax().text_range()),
30+
),
31+
)
32+
}
33+
34+
#[cfg(test)]
35+
mod tests {
36+
use crate::tests::check_diagnostics;
37+
38+
#[test]
39+
fn simple() {
40+
check_diagnostics(
41+
r#"
42+
trait Trait {
43+
const C: ();
44+
type T;
45+
fn f();
46+
}
47+
48+
impl Trait for () {
49+
const C: () = ();
50+
type T = ();
51+
fn f() {}
52+
}
53+
54+
impl Trait for () {
55+
//^^^^^ error: not all trait items implemented, missing: `const C`
56+
type T = ();
57+
fn f() {}
58+
}
59+
60+
impl Trait for () {
61+
//^^^^^ error: not all trait items implemented, missing: `const C`, `type T`, `fn f`
62+
}
63+
64+
"#,
65+
);
66+
}
67+
68+
#[test]
69+
fn default() {
70+
check_diagnostics(
71+
r#"
72+
trait Trait {
73+
const C: ();
74+
type T = ();
75+
fn f() {}
76+
}
77+
78+
impl Trait for () {
79+
const C: () = ();
80+
type T = ();
81+
fn f() {}
82+
}
83+
84+
impl Trait for () {
85+
//^^^^^ error: not all trait items implemented, missing: `const C`
86+
type T = ();
87+
fn f() {}
88+
}
89+
90+
impl Trait for () {
91+
//^^^^^ error: not all trait items implemented, missing: `const C`
92+
type T = ();
93+
}
94+
95+
impl Trait for () {
96+
//^^^^^ error: not all trait items implemented, missing: `const C`
97+
}
98+
99+
"#,
100+
);
101+
}
102+
}

‎crates/ide-diagnostics/src/handlers/type_mismatch.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ struct Foo;
278278
struct Bar;
279279
impl core::ops::Deref for Foo {
280280
type Target = Bar;
281+
fn deref(&self) -> &Self::Target { loop {} }
281282
}
282283
283284
fn main() {
@@ -290,6 +291,7 @@ struct Foo;
290291
struct Bar;
291292
impl core::ops::Deref for Foo {
292293
type Target = Bar;
294+
fn deref(&self) -> &Self::Target { loop {} }
293295
}
294296
295297
fn main() {

‎crates/ide-diagnostics/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ mod handlers {
4646
pub(crate) mod replace_filter_map_next_with_find_map;
4747
pub(crate) mod trait_impl_orphan;
4848
pub(crate) mod trait_impl_incorrect_safety;
49+
pub(crate) mod trait_impl_missing_assoc_item;
4950
pub(crate) mod typed_hole;
5051
pub(crate) mod type_mismatch;
5152
pub(crate) mod unimplemented_builtin_macro;
@@ -360,8 +361,9 @@ pub fn diagnostics(
360361
AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d),
361362
AnyDiagnostic::PrivateField(d) => handlers::private_field::private_field(&ctx, &d),
362363
AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => handlers::replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d),
363-
AnyDiagnostic::TraitImplOrphan(d) => handlers::trait_impl_orphan::trait_impl_orphan(&ctx, &d),
364364
AnyDiagnostic::TraitImplIncorrectSafety(d) => handlers::trait_impl_incorrect_safety::trait_impl_incorrect_safety(&ctx, &d),
365+
AnyDiagnostic::TraitImplMissingAssocItems(d) => handlers::trait_impl_missing_assoc_item::trait_impl_missing_assoc_item(&ctx, &d),
366+
AnyDiagnostic::TraitImplOrphan(d) => handlers::trait_impl_orphan::trait_impl_orphan(&ctx, &d),
365367
AnyDiagnostic::TypedHole(d) => handlers::typed_hole::typed_hole(&ctx, &d),
366368
AnyDiagnostic::TypeMismatch(d) => handlers::type_mismatch::type_mismatch(&ctx, &d),
367369
AnyDiagnostic::UndeclaredLabel(d) => handlers::undeclared_label::undeclared_label(&ctx, &d),

‎crates/ide-diagnostics/src/tests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) {
4343
super::diagnostics(&db, &conf, &AssistResolveStrategy::All, file_position.file_id)
4444
.pop()
4545
.expect("no diagnostics");
46-
let fix = &diagnostic.fixes.expect("diagnostic misses fixes")[nth];
46+
let fix =
47+
&diagnostic.fixes.expect(&format!("{:?} diagnostic misses fixes", diagnostic.code))[nth];
4748
let actual = {
4849
let source_change = fix.source_change.as_ref().unwrap();
4950
let file_id = *source_change.source_file_edits.keys().next().unwrap();

0 commit comments

Comments
 (0)