diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index f3188c9aada5..7038709431d0 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -113,6 +113,7 @@ diagnostics![AnyDiagnostic<'db> -> FruInDestructuringAssignment, FunctionalRecordUpdateOnNonStruct, GenericDefaultRefersToSelf, + ImplIncorrectSafety, InactiveCode, IncoherentImpl, IncorrectCase, @@ -148,7 +149,6 @@ diagnostics![AnyDiagnostic<'db> -> RemoveUnnecessaryElse, UnusedMustUse<'db>, ReplaceFilterMapNextWithFindMap, - TraitImplIncorrectSafety, TraitImplMissingAssocItems, TraitImplOrphan, TraitImplRedundantAssocItems, @@ -495,14 +495,6 @@ pub struct TraitImplOrphan { pub impl_: AstPtr, } -// FIXME: Split this off into the corresponding 4 rustc errors -#[derive(Debug, PartialEq, Eq)] -pub struct TraitImplIncorrectSafety { - pub file_id: HirFileId, - pub impl_: AstPtr, - pub should_be_safe: bool, -} - #[derive(Debug, PartialEq, Eq)] pub struct TraitImplMissingAssocItems { pub file_id: HirFileId, @@ -528,6 +520,22 @@ pub struct RemoveUnnecessaryElse { pub if_expr: InFile>, } +#[derive(Debug)] +pub struct ImplIncorrectSafety { + pub file_id: HirFileId, + pub impl_: AstPtr, + pub kind: ImplIncorrectSafetyKind, +} + +#[derive(Debug)] +pub enum ImplIncorrectSafetyKind { + UnsafeInherentImpl, + UnsafeNegativeImpl, + UnsafeImplOfSafeTrait(Trait), + SafeImplOfUnsafeTrait(Trait), + SafeImplOfDanglingDrop, +} + #[derive(Debug)] pub struct UnusedMustUse<'db> { pub expr: InFile, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d187763151a2..7f8707ba34a4 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -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(), + ); + } + } + + // 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(), + ); + } _ => (), }; diff --git a/crates/ide-diagnostics/src/handlers/impl_incorrect_safety.rs b/crates/ide-diagnostics/src/handlers/impl_incorrect_safety.rs new file mode 100644 index 000000000000..aad71ecfaec0 --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/impl_incorrect_safety.rs @@ -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>, +) -> 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>, +) -> 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>, + 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>, + 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>, +) -> 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; +struct L<'l>; + + impl Drop for S {} + + impl<#[may_dangle] T> Drop for S {} +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: requires an `unsafe impl` declaration due to `#[may_dangle]` attribute + + unsafe impl Drop for S {} +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: implementing the trait `Drop` is not unsafe + + unsafe impl<#[may_dangle] T> Drop for S {} + + 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 {} + "#, + ); + } +} diff --git a/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs b/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs deleted file mode 100644 index 9e7393c89c15..000000000000 --- a/crates/ide-diagnostics/src/handlers/trait_impl_incorrect_safety.rs +++ /dev/null @@ -1,140 +0,0 @@ -use hir::InFile; -use syntax::ast; - -use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, Severity, adjusted_display_range}; - -// 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( - 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" - }, - adjusted_display_range::( - ctx, - InFile { file_id: d.file_id, value: d.impl_ }, - &|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()) - } - }, - ), - ) - .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: 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#" -#![feature(lang_items)] -#[lang = "drop"] -trait Drop {} -struct S; -struct L<'l>; - - impl Drop for S {} - - impl<#[may_dangle] T> Drop for S {} -//^^^^ error: impl for unsafe trait needs to be unsafe - - unsafe impl Drop for S {} -//^^^^^^^^^^^ error: unsafe impl for safe trait - - unsafe impl<#[may_dangle] T> Drop for S {} - - 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 -"#, - ); - } - - #[test] - fn unsafe_unresolved_trait() { - check_diagnostics( - r#" -unsafe impl TestTrait for u32 {} - "#, - ); - } -} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index aec68b55c78d..8279cc2495f7 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -45,6 +45,7 @@ mod handlers { pub(crate) mod functional_record_update_on_non_struct; pub(crate) mod generic_args_prohibited; pub(crate) mod generic_default_refers_to_self; + pub(crate) mod impl_incorrect_safety; pub(crate) mod inactive_code; pub(crate) mod incoherent_impl; pub(crate) mod incorrect_case; @@ -77,7 +78,6 @@ mod handlers { pub(crate) mod remove_trailing_return; pub(crate) mod remove_unnecessary_else; pub(crate) mod replace_filter_map_next_with_find_map; - pub(crate) mod trait_impl_incorrect_safety; pub(crate) mod trait_impl_missing_assoc_item; pub(crate) mod trait_impl_orphan; pub(crate) mod trait_impl_redundant_assoc_item; @@ -448,6 +448,7 @@ pub fn semantic_diagnostics( AnyDiagnostic::ExpectedArrayOrSlicePat(d) => handlers::expected_array_or_slice_pat::expected_array_or_slice_pat(&ctx, &d), AnyDiagnostic::ExpectedFunction(d) => handlers::expected_function::expected_function(&ctx, &d), AnyDiagnostic::FunctionalRecordUpdateOnNonStruct(d) => handlers::functional_record_update_on_non_struct::functional_record_update_on_non_struct(&ctx, &d), + AnyDiagnostic::ImplIncorrectSafety(d) => handlers::impl_incorrect_safety::impl_incorrect_safety(&ctx, &d), AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { Some(it) => it, None => continue, @@ -494,7 +495,6 @@ pub fn semantic_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::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::TraitImplRedundantAssocItems(d) => handlers::trait_impl_redundant_assoc_item::trait_impl_redundant_assoc_item(&ctx, &d), AnyDiagnostic::TraitImplOrphan(d) => handlers::trait_impl_orphan::trait_impl_orphan(&ctx, &d),