Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions crates/hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ diagnostics![AnyDiagnostic<'db> ->
FruInDestructuringAssignment,
FunctionalRecordUpdateOnNonStruct,
GenericDefaultRefersToSelf,
ImplIncorrectSafety,
InactiveCode,
IncoherentImpl,
IncorrectCase,
Expand Down Expand Up @@ -148,7 +149,6 @@ diagnostics![AnyDiagnostic<'db> ->
RemoveUnnecessaryElse,
UnusedMustUse<'db>,
ReplaceFilterMapNextWithFindMap,
TraitImplIncorrectSafety,
TraitImplMissingAssocItems,
TraitImplOrphan,
TraitImplRedundantAssocItems,
Expand Down Expand Up @@ -495,14 +495,6 @@ pub struct TraitImplOrphan {
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,
}

#[derive(Debug, PartialEq, Eq)]
pub struct TraitImplMissingAssocItems {
pub file_id: HirFileId,
Expand All @@ -528,6 +520,22 @@ pub struct RemoveUnnecessaryElse {
pub if_expr: InFile<AstPtr<ast::IfExpr>>,
}

#[derive(Debug)]
pub struct ImplIncorrectSafety {
pub file_id: HirFileId,
pub impl_: AstPtr<ast::Impl>,
pub kind: ImplIncorrectSafetyKind,
}

#[derive(Debug)]
pub enum ImplIncorrectSafetyKind {
UnsafeInherentImpl,
UnsafeNegativeImpl,
UnsafeImplOfSafeTrait(Trait),
SafeImplOfUnsafeTrait(Trait),
SafeImplOfDanglingDrop,
}

#[derive(Debug)]
pub struct UnusedMustUse<'db> {
pub expr: InFile<ExprOrPatPtr>,
Expand Down
67 changes: 61 additions & 6 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,13 +895,68 @@ impl Module {

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(loc.id.value), file_id, should_be_safe: true }.into()),
// safe impl for unsafe trait
(false, true, false, _) |
(true, _, true, _) => {
acc.push(
ImplIncorrectSafety {
file_id,
impl_: ast_id_map.get(loc.id.value),
kind: ImplIncorrectSafetyKind::UnsafeNegativeImpl,
}
.into(),
);
}

(true, false, _, false) => {
let impl_ = ast_id_map.get(loc.id.value);

if let Some(trait_) = trait_ {
// unsafe impl of safe trait
acc.push(
ImplIncorrectSafety {
file_id,
impl_,
kind: ImplIncorrectSafetyKind::UnsafeImplOfSafeTrait(trait_),
}
.into(),
);
} else {
// unsafe inherent impl
acc.push(
ImplIncorrectSafety {
file_id,
impl_,
kind: ImplIncorrectSafetyKind::UnsafeInherentImpl,
}
.into(),
);
}
}
Comment thread
ada4a marked this conversation as resolved.

// safe impl of unsafe trait
(false, true, false, _) => {
if let Some(trait_) = trait_ {
acc.push(
ImplIncorrectSafety {
file_id,
impl_: ast_id_map.get(loc.id.value),
kind: ImplIncorrectSafetyKind::SafeImplOfUnsafeTrait(trait_),
}
.into(),
);
}
}

// safe impl of dangling drop
(false, false, _, true) => acc.push(TraitImplIncorrectSafety { impl_: ast_id_map.get(loc.id.value), file_id, should_be_safe: false }.into()),
(false, false, _, true) => {
acc.push(
ImplIncorrectSafety {
file_id,
impl_: ast_id_map.get(loc.id.value),
kind: ImplIncorrectSafetyKind::SafeImplOfDanglingDrop,
}
.into(),
);
}
_ => (),
};

Expand Down
221 changes: 221 additions & 0 deletions crates/ide-diagnostics/src/handlers/impl_incorrect_safety.rs
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I split the file into 5 as well?

Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
use hir::InFile;
use syntax::{AstNode, AstPtr, ast};

use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, adjusted_display_range};

pub(crate) fn impl_incorrect_safety(
ctx: &DiagnosticsContext<'_, '_>,
d: &hir::ImplIncorrectSafety,
) -> Diagnostic {
use hir::ImplIncorrectSafetyKind as Kind;

let impl_ = InFile { file_id: d.file_id, value: d.impl_ };
match d.kind {
Kind::UnsafeInherentImpl => unsafe_inherent_impl(ctx, impl_),
Kind::UnsafeNegativeImpl => unsafe_negative_impl(ctx, impl_),
Kind::UnsafeImplOfSafeTrait(trait_) => unsafe_impl_of_safe_trait(ctx, impl_, trait_),
Kind::SafeImplOfUnsafeTrait(trait_) => safe_impl_of_unsafe_trait(ctx, impl_, trait_),
Kind::SafeImplOfDanglingDrop => safe_impl_of_dangling_drop(ctx, impl_),
}
}

// Diagnostic: unsafe-inherent-impl
//
// This diagnostic is triggered when an inherent implementation is marked `unsafe`
fn unsafe_inherent_impl(
ctx: &DiagnosticsContext<'_, '_>,
impl_: InFile<AstPtr<ast::Impl>>,
) -> Diagnostic {
Diagnostic::new(
DiagnosticCode::RustcHardError("E0197"),
"inherent impls cannot be unsafe",
adjusted_display_range(ctx, impl_, &|impl_| {
Some((impl_.unsafe_token()?.text_range()).cover(impl_.self_ty()?.syntax().text_range()))
}),
)
.with_main_node(impl_.map(Into::into))
.stable()
}

// Diagnostic: unsafe-negative-impl
//
// This diagnostic is triggered when a negative implementation is marked `unsafe`
fn unsafe_negative_impl(
ctx: &DiagnosticsContext<'_, '_>,
impl_: InFile<AstPtr<ast::Impl>>,
) -> Diagnostic {
Diagnostic::new(
DiagnosticCode::RustcHardError("E0198"),
"negative impls cannot be unsafe",
adjusted_display_range(ctx, impl_, &|impl_| {
Some((impl_.unsafe_token()?.text_range()).cover(impl_.self_ty()?.syntax().text_range()))
}),
)
.with_main_node(impl_.map(Into::into))
.stable()
}

// Diagnostic: unsafe-impl-of-safe-trait
//
// This diagnostic is triggered when an implementation of a safe trait is marked `unsafe`
fn unsafe_impl_of_safe_trait(
ctx: &DiagnosticsContext<'_, '_>,
impl_: InFile<AstPtr<ast::Impl>>,
trait_: hir::Trait,
) -> Diagnostic {
let trait_name = trait_.name(ctx.db());
let trait_name = trait_name.display(ctx.db(), ctx.edition);

Diagnostic::new(
DiagnosticCode::RustcHardError("E0199"),
format!("implementing the trait `{trait_name}` is not unsafe"),
adjusted_display_range(ctx, impl_, &|impl_| {
Some(impl_.unsafe_token()?.text_range().cover(impl_.self_ty()?.syntax().text_range()))
}),
)
.with_main_node(impl_.map(Into::into))
.stable()
}

// Diagnostic: safe-impl-of-unsafe-trait
//
// This diagnostic is triggered when an implementation of an unsafe trait is missing `unsafe`
fn safe_impl_of_unsafe_trait(
ctx: &DiagnosticsContext<'_, '_>,
impl_: InFile<AstPtr<ast::Impl>>,
trait_: hir::Trait,
) -> Diagnostic {
let trait_name = trait_.name(ctx.db());
let trait_name = trait_name.display(ctx.db(), ctx.edition);

Diagnostic::new(
DiagnosticCode::RustcHardError("E0200"),
format!("the trait `{trait_name}` requires an `unsafe impl` declaration"),
adjusted_display_range(ctx, impl_, &|impl_| {
Some(impl_.impl_token()?.text_range().cover(impl_.self_ty()?.syntax().text_range()))
}),
)
.with_main_node(impl_.map(Into::into))
.stable()
}

// Diagnostic: safe-impl-of-dangling-drop
//
// This diagnostic is triggered when an implementation of `Drop` using `#[may_dangle]` is missing `unsafe`
fn safe_impl_of_dangling_drop(
ctx: &DiagnosticsContext<'_, '_>,
impl_: InFile<AstPtr<ast::Impl>>,
) -> Diagnostic {
Diagnostic::new(
DiagnosticCode::RustcHardError("E0569"),
"requires an `unsafe impl` declaration due to `#[may_dangle]` attribute",
adjusted_display_range(ctx, impl_, &|impl_| {
Some(impl_.impl_token()?.text_range().cover(impl_.self_ty()?.syntax().text_range()))
}),
)
.with_main_node(impl_.map(Into::into))
.stable()
}

#[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: the trait `Unsafe` requires an `unsafe impl` declaration

unsafe impl Safe for () {}
//^^^^^^^^^^^^^^^^^^^^^^^ error: implementing the trait `Safe` is not unsafe

unsafe impl Unsafe for () {}
"#,
);
}

#[test]
fn drop_may_dangle() {
check_diagnostics(
r#"
#![feature(lang_items)]
#[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: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute

unsafe impl<T> Drop for S<T> {}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: implementing the trait `Drop` is not unsafe

unsafe impl<#[may_dangle] T> Drop for S<T> {}

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

impl<#[may_dangle] 'l> Drop for L<'l> {}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute

unsafe impl<'l> Drop for L<'l> {}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: implementing the trait `Drop` is not unsafe

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: negative impls cannot be unsafe

unsafe trait UnsafeTrait {}

impl !UnsafeTrait for () {}

unsafe impl !UnsafeTrait for () {}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: negative impls cannot be unsafe

"#,
);
}

#[test]
fn inherent() {
check_diagnostics(
r#"
struct S;

impl S {}

unsafe impl S {}
//^^^^^^^^^^^^^ error: inherent impls cannot be unsafe
"#,
);
}

#[test]
fn unsafe_unresolved_trait() {
check_diagnostics(
r#"
unsafe impl TestTrait for u32 {}
"#,
);
}
}
Loading
Loading