Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #18135 - ChayimFriedman2:unsafe-op-in-unsafe-fn, r=Veykril
feat: Add diagnostics for `unsafe_op_in_unsafe_fn`
Turns out it's pretty easy, but I did have to add support for allowed-by-default lints.
| -rw-r--r-- | crates/hir-ty/src/diagnostics/unsafe_check.rs | 10 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 2 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 5 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/missing_unsafe.rs | 31 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 17 |
5 files changed, 55 insertions, 10 deletions
diff --git a/crates/hir-ty/src/diagnostics/unsafe_check.rs b/crates/hir-ty/src/diagnostics/unsafe_check.rs index 309e208a9a..ff45c725c7 100644 --- a/crates/hir-ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir-ty/src/diagnostics/unsafe_check.rs @@ -13,7 +13,10 @@ use crate::{ db::HirDatabase, utils::is_fn_unsafe_to_call, InferenceResult, Interner, TyExt, TyKind, }; -pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> { +/// Returns `(unsafe_exprs, fn_is_unsafe)`. +/// +/// If `fn_is_unsafe` is false, `unsafe_exprs` are hard errors. If true, they're `unsafe_op_in_unsafe_fn`. +pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec<ExprId>, bool) { let _p = tracing::info_span!("missing_unsafe").entered(); let mut res = Vec::new(); @@ -24,9 +27,6 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> { | DefWithBodyId::VariantId(_) | DefWithBodyId::InTypeConstId(_) => false, }; - if is_unsafe { - return res; - } let body = db.body(def); let infer = db.infer(def); @@ -36,7 +36,7 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> { } }); - res + (res, is_unsafe) } pub struct UnsafeExpr { diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 4d90df63b9..0b3cdb2f37 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -258,6 +258,8 @@ pub struct PrivateField { #[derive(Debug)] pub struct MissingUnsafe { pub expr: InFile<AstPtr<ast::Expr>>, + /// If true, the diagnostics is an `unsafe_op_in_unsafe_fn` lint instead of a hard error. + pub only_lint: bool, } #[derive(Debug)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 176c059bf6..d18a36bdbd 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1884,9 +1884,10 @@ impl DefWithBody { ); } - for expr in hir_ty::diagnostics::missing_unsafe(db, self.into()) { + let (unafe_exprs, only_lint) = hir_ty::diagnostics::missing_unsafe(db, self.into()); + for expr in unafe_exprs { match source_map.expr_syntax(expr) { - Ok(expr) => acc.push(MissingUnsafe { expr }.into()), + Ok(expr) => acc.push(MissingUnsafe { expr, only_lint }.into()), Err(SyntheticSyntax) => { // FIXME: Here and elsewhere in this file, the `expr` was // desugared, report or assert that this doesn't happen. diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index 9a9bab091d..5b43f4b2af 100644 --- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -11,9 +11,14 @@ use crate::{fix, Diagnostic, DiagnosticCode, DiagnosticsContext}; // // This diagnostic is triggered if an operation marked as `unsafe` is used outside of an `unsafe` function or block. pub(crate) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Diagnostic { + let code = if d.only_lint { + DiagnosticCode::RustcLint("unsafe_op_in_unsafe_fn") + } else { + DiagnosticCode::RustcHardError("E0133") + }; Diagnostic::new_with_syntax_node_ptr( ctx, - DiagnosticCode::RustcHardError("E0133"), + code, "this operation is unsafe and requires an unsafe function or block", d.expr.map(|it| it.into()), ) @@ -566,4 +571,28 @@ fn main() { "#, ) } + + #[test] + fn unsafe_op_in_unsafe_fn_allowed_by_default() { + check_diagnostics( + r#" +unsafe fn foo(p: *mut i32) { + *p = 123; +} + "#, + ) + } + + #[test] + fn unsafe_op_in_unsafe_fn() { + check_diagnostics( + r#" +#![warn(unsafe_op_in_unsafe_fn)] +unsafe fn foo(p: *mut i32) { + *p = 123; + //^^💡 warn: this operation is unsafe and requires an unsafe function or block +} + "#, + ) + } } diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 69da744e66..7edc55a743 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -491,7 +491,7 @@ pub fn semantic_diagnostics( // The edition isn't accurate (each diagnostics may have its own edition due to macros), // but it's okay as it's only being used for error recovery. - handle_lint_attributes( + handle_lints( &ctx.sema, &mut FxHashMap::default(), &mut lints, @@ -551,7 +551,12 @@ fn build_group_dict( map_with_prefixes.into_iter().map(|(k, v)| (k.strip_prefix(prefix).unwrap(), v)).collect() } -fn handle_lint_attributes( +/// Thd default severity for lints that are not warn by default. +// FIXME: Autogenerate this instead of write manually. +static LINTS_DEFAULT_SEVERITY: LazyLock<FxHashMap<&str, Severity>> = + LazyLock::new(|| FxHashMap::from_iter([("unsafe_op_in_unsafe_fn", Severity::Allow)])); + +fn handle_lints( sema: &Semantics<'_, RootDatabase>, cache: &mut FxHashMap<HirFileId, FxHashMap<SmolStr, SeverityAttr>>, diagnostics: &mut [(InFile<SyntaxNode>, &mut Diagnostic)], @@ -559,6 +564,14 @@ fn handle_lint_attributes( edition: Edition, ) { for (node, diag) in diagnostics { + let lint = match diag.code { + DiagnosticCode::RustcLint(lint) | DiagnosticCode::Clippy(lint) => lint, + _ => panic!("non-lint passed to `handle_lints()`"), + }; + if let Some(&default_severity) = LINTS_DEFAULT_SEVERITY.get(lint) { + diag.severity = default_severity; + } + let mut diag_severity = fill_lint_attrs(sema, node, cache, cache_stack, diag, edition); if let outline_diag_severity @ Some(_) = |