rust-analyzer

diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs
index 93fd741aff..72ccc17486 100644
--- a/crates/hir-def/src/data.rs
+++ b/crates/hir-def/src/data.rs

@@ -327,6 +327,7 @@ pub struct ImplData {

pub self_ty: Interned<TypeRef>,

pub items: Vec<AssocItemId>,

pub is_negative: bool,

+ pub is_unsafe: bool,

// box it as the vec is usually empty anyways

pub attribute_calls: Option<Box<Vec<(AstId<ast::Item>, MacroCallId)>>>,

}

@@ -348,6 +349,7 @@ impl ImplData {

let target_trait = impl_def.target_trait.clone();

let self_ty = impl_def.self_ty.clone();

let is_negative = impl_def.is_negative;

+ let is_unsafe = impl_def.is_unsafe;

let mut collector =

AssocItemCollector::new(db, module_id, tree_id.file_id(), ItemContainerId::ImplId(id));

@@ -357,7 +359,14 @@ impl ImplData {

let items = items.into_iter().map(|(_, item)| item).collect();

(

- Arc::new(ImplData { target_trait, self_ty, items, is_negative, attribute_calls }),

+ Arc::new(ImplData {

+ target_trait,

+ self_ty,

+ items,

+ is_negative,

+ is_unsafe,

+ attribute_calls,

+ }),

diagnostics.into(),

)

}

diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs
index 3c4f21d5c6..70b96b2573 100644
--- a/crates/hir-def/src/item_tree.rs
+++ b/crates/hir-def/src/item_tree.rs

@@ -709,6 +709,7 @@ pub struct Impl {

pub target_trait: Option<Interned<TraitRef>>,

pub self_ty: Interned<TypeRef>,

pub is_negative: bool,

+ pub is_unsafe: bool,

pub items: Box<[AssocItem]>,

pub ast_id: FileAstId<ast::Impl>,

}

diff --git a/crates/hir-def/src/item_tree/lower.rs b/crates/hir-def/src/item_tree/lower.rs
index 30b649d2f3..c898eb5f92 100644
--- a/crates/hir-def/src/item_tree/lower.rs
+++ b/crates/hir-def/src/item_tree/lower.rs

@@ -492,6 +492,7 @@ impl<'a> Ctx<'a> {

let target_trait = impl_def.trait_().and_then(|tr| self.lower_trait_ref(&tr));

let self_ty = self.lower_type_ref(&impl_def.self_ty()?);

let is_negative = impl_def.excl_token().is_some();

+ let is_unsafe = impl_def.unsafe_token().is_some();

// We cannot use `assoc_items()` here as that does not include macro calls.

let items = impl_def

@@ -506,7 +507,8 @@ impl<'a> Ctx<'a> {

})

.collect();

let ast_id = self.source_ast_id_map.ast_id(impl_def);

- let res = Impl { generic_params, target_trait, self_ty, is_negative, items, ast_id };

+ let res =

+ Impl { generic_params, target_trait, self_ty, is_negative, is_unsafe, items, ast_id };

Some(id(self.data().impls.alloc(res)))

}

diff --git a/crates/hir-def/src/item_tree/pretty.rs b/crates/hir-def/src/item_tree/pretty.rs
index 5036c2b882..ca3785bf28 100644
--- a/crates/hir-def/src/item_tree/pretty.rs
+++ b/crates/hir-def/src/item_tree/pretty.rs

@@ -388,8 +388,18 @@ impl Printer<'_> {

wln!(self);

}

ModItem::Impl(it) => {

- let Impl { target_trait, self_ty, is_negative, items, generic_params, ast_id: _ } =

- &self.tree[it];

+ let Impl {

+ target_trait,

+ self_ty,

+ is_negative,

+ is_unsafe,

+ items,

+ generic_params,

+ ast_id: _,

+ } = &self.tree[it];

+ if *is_unsafe {

+ w!(self, "unsafe");

+ }

w!(self, "impl");

self.print_generic_params(generic_params);

w!(self, " ");

diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index 3a45622675..dd5c9b3535 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs

@@ -38,7 +38,6 @@ diagnostics![

IncorrectCase,

InvalidDeriveTarget,

IncoherentImpl,

- TraitImplOrphan,

MacroDefError,

MacroError,

MacroExpansionParseError,

@@ -54,6 +53,8 @@ diagnostics![

PrivateAssocItem,

PrivateField,

ReplaceFilterMapNextWithFindMap,

+ TraitImplIncorrectSafety,

+ TraitImplOrphan,

TypedHole,

TypeMismatch,

UndeclaredLabel,

@@ -293,3 +294,11 @@ pub struct TraitImplOrphan {

pub file_id: HirFileId,

pub impl_: AstPtr<ast::Impl>,

}

+// FIXME: Split this off into the corresponding 4 rustc errors

+#[derive(Debug, PartialEq, Eq)]

+pub struct TraitImplIncorrectSafety {

+ pub file_id: HirFileId,

+ pub impl_: AstPtr<ast::Impl>,

+ pub should_be_safe: bool,

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 8d82e88da9..066d7e635a 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs

@@ -53,10 +53,10 @@ use hir_def::{

resolver::{HasResolver, Resolver},

src::HasSource as _,

AssocItemId, AssocItemLoc, AttrDefId, ConstId, ConstParamId, CrateRootModuleId, DefWithBodyId,

- EnumId, EnumVariantId, ExternCrateId, FunctionId, GenericDefId, HasModule, ImplId,

- InTypeConstId, ItemContainerId, LifetimeParamId, LocalEnumVariantId, LocalFieldId, Lookup,

- MacroExpander, MacroId, ModuleId, StaticId, StructId, TraitAliasId, TraitId, TypeAliasId,

- TypeOrConstParamId, TypeParamId, UnionId,

+ EnumId, EnumVariantId, ExternCrateId, FunctionId, GenericDefId, GenericParamId, HasModule,

+ ImplId, InTypeConstId, ItemContainerId, LifetimeParamId, LocalEnumVariantId, LocalFieldId,

+ Lookup, MacroExpander, MacroId, ModuleId, StaticId, StructId, TraitAliasId, TraitId,

+ TypeAliasId, TypeOrConstParamId, TypeParamId, UnionId,

};

use hir_expand::{name::name, MacroCallKind};

use hir_ty::{

@@ -89,17 +89,7 @@ use crate::db::{DefDatabase, HirDatabase};

pub use crate::{

attrs::{resolve_doc_path_on, HasAttrs},

- diagnostics::{

- AnyDiagnostic, BreakOutsideOfLoop, CaseType, ExpectedFunction, InactiveCode,

- IncoherentImpl, IncorrectCase, InvalidDeriveTarget, MacroDefError, MacroError,

- MacroExpansionParseError, MalformedDerive, MismatchedArgCount,

- MismatchedTupleStructPatArgCount, MissingFields, MissingMatchArms, MissingUnsafe,

- MovedOutOfRef, NeedMut, NoSuchField, PrivateAssocItem, PrivateField,

- ReplaceFilterMapNextWithFindMap, TraitImplOrphan, TypeMismatch, TypedHole, UndeclaredLabel,

- UnimplementedBuiltinMacro, UnreachableLabel, UnresolvedExternCrate, UnresolvedField,

- UnresolvedImport, UnresolvedMacroCall, UnresolvedMethodCall, UnresolvedModule,

- UnresolvedProcMacro, UnusedMut, UnusedVariable,

- },

+ diagnostics::*,

has_source::HasSource,

semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},

};

@@ -613,22 +603,64 @@ impl Module {

// FIXME: Once we diagnose the inputs to builtin derives, we should at least extract those diagnostics somehow

continue;

}

+ let ast_id_map = db.ast_id_map(file_id);

for diag in db.impl_data_with_diagnostics(impl_def.id).1.iter() {

emit_def_diagnostic(db, acc, diag);

}

if inherent_impls.invalid_impls().contains(&impl_def.id) {

- let ast_id_map = db.ast_id_map(file_id);

acc.push(IncoherentImpl { impl_: ast_id_map.get(node.ast_id()), file_id }.into())

}

if !impl_def.check_orphan_rules(db) {

- let ast_id_map = db.ast_id_map(file_id);

acc.push(TraitImplOrphan { impl_: ast_id_map.get(node.ast_id()), file_id }.into())

}

+ let trait_ = impl_def.trait_(db);

+ let trait_is_unsafe = trait_.map_or(false, |t| t.is_unsafe(db));

+ let impl_is_negative = impl_def.is_negative(db);

+ let impl_is_unsafe = impl_def.is_unsafe(db);

+ let drop_maybe_dangle = (|| {

+ // FIXME: This can be simplified a lot by exposing hir-ty's utils.rs::Generics helper

+ let trait_ = trait_?;

+ let drop_trait = db.lang_item(self.krate().into(), LangItem::Drop)?.as_trait()?;

+ if drop_trait != trait_.into() {

+ return None;

+ }

+ let parent = impl_def.id.into();

+ let generic_params = db.generic_params(parent);

+ let lifetime_params = generic_params.lifetimes.iter().map(|(local_id, _)| {

+ GenericParamId::LifetimeParamId(LifetimeParamId { parent, local_id })

+ });

+ let type_params = generic_params

+ .iter()

+ .filter(|(_, it)| it.type_param().is_some())

+ .map(|(local_id, _)| {

+ GenericParamId::TypeParamId(TypeParamId::from_unchecked(

+ TypeOrConstParamId { parent, local_id },

+ ))

+ });

+ let res = type_params

+ .chain(lifetime_params)

+ .any(|p| db.attrs(AttrDefId::GenericParamId(p)).by_key("may_dangle").exists());

+ Some(res)

+ })()

+ .unwrap_or(false);

+ match (impl_is_unsafe, trait_is_unsafe, impl_is_negative, drop_maybe_dangle) {

+ // unsafe negative impl

+ (true, _, true, _) |

+ // unsafe impl for safe trait

+ (true, false, _, false) => acc.push(TraitImplIncorrectSafety { impl_: ast_id_map.get(node.ast_id()), file_id, should_be_safe: true }.into()),

+ // safe impl for unsafe trait

+ (false, true, false, _) |

+ // safe impl of dangling drop

+ (false, false, _, true) => acc.push(TraitImplIncorrectSafety { impl_: ast_id_map.get(node.ast_id()), file_id, should_be_safe: false }.into()),

+ _ => (),

+ };

for item in impl_def.items(db) {

let def: DefWithBody = match item {

AssocItem::Function(it) => it.into(),

@@ -3404,7 +3436,7 @@ impl Impl {

}

pub fn is_unsafe(self, db: &dyn HirDatabase) -> bool {

- db.impl_data(self.id).is_unique()

+ db.impl_data(self.id).is_unsafe

}

pub fn module(self, db: &dyn HirDatabase) -> Module {

diff --git a/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs b/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs
new file mode 100644
index 0000000000..f28298de7d
--- /dev/null
+++ b/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs

@@ -0,0 +1,115 @@

+use hir::InFile;

+use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, Severity};

+// Diagnostic: trait-impl-incorrect-safety

+//

+// Diagnoses incorrect safety annotations of trait impls.

+pub(crate) fn trait_impl_incorrect_safety(

+ ctx: &DiagnosticsContext<'_>,

+ d: &hir::TraitImplIncorrectSafety,

+) -> Diagnostic {

+ Diagnostic::new_with_syntax_node_ptr(

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

+ )

+#[cfg(test)]

+mod tests {

+ use crate::tests::check_diagnostics;

+ #[test]

+ fn simple() {

+ check_diagnostics(

+ r#"

+trait Safe {}

+unsafe trait Unsafe {}

+ impl Safe for () {}

+ impl Unsafe for () {}

+//^^^^^^^^^^^^^^^^^^^^^ error: impl for unsafe trait needs to be unsafe

+ unsafe impl Safe for () {}

+//^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait

+ unsafe impl Unsafe for () {}

+"#,

+ );

+ }

+ #[test]

+ fn drop_may_dangle() {

+ check_diagnostics(

+ r#"

+#[lang = "drop"]

+trait Drop {}

+struct S<T>;

+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

+ unsafe impl<T> Drop for S<T> {}

+//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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

+ unsafe impl<'l> Drop for L<'l> {}

+//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait

+ unsafe impl<#[may_dangle] 'l> Drop for L<'l> {}

+"#,

+ );

+ }

+ #[test]

+ fn negative() {

+ check_diagnostics(

+ r#"

+trait Trait {}

+ impl !Trait for () {}

+ unsafe impl !Trait for () {}

+//^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait

+unsafe trait UnsafeTrait {}

+ impl !UnsafeTrait for () {}

+ unsafe impl !UnsafeTrait for () {}

+//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait

+"#,

+ );

+ }

+ #[test]

+ fn inherent() {

+ check_diagnostics(

+ r#"

+struct S;

+ impl S {}

+ unsafe impl S {}

+//^^^^^^^^^^^^^^^^ error: unsafe impl for safe trait

+"#,

+ );

+ }

diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
index 3226dd29d2..99921c1107 100644
--- a/crates/ide-diagnostics/src/lib.rs
+++ b/crates/ide-diagnostics/src/lib.rs

@@ -45,6 +45,7 @@ mod handlers {

pub(crate) mod private_field;

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 typed_hole;

pub(crate) mod type_mismatch;

pub(crate) mod unimplemented_builtin_macro;

@@ -360,6 +361,7 @@ pub fn diagnostics(

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