Skip to content

Commit

Permalink
Auto merge of rust-lang#18135 - ChayimFriedman2:unsafe-op-in-unsafe-f…
Browse files Browse the repository at this point in the history
…n, 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.
  • Loading branch information
bors committed Sep 18, 2024
2 parents 44f8545 + 319bc52 commit e700b48
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -36,7 +36,7 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> {
}
});

res
(res, is_unsafe)
}

pub struct UnsafeExpr {
Expand Down
2 changes: 2 additions & 0 deletions src/tools/rust-analyzer/crates/hir/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
5 changes: 3 additions & 2 deletions src/tools/rust-analyzer/crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
)
Expand Down Expand Up @@ -562,6 +567,30 @@ fn main() {
ed2021::safe();
ed2024::not_safe();
//^^^^^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
}
"#,
)
}

#[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
}
"#,
)
Expand Down
17 changes: 15 additions & 2 deletions src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -551,14 +551,27 @@ 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)],
cache_stack: &mut Vec<HirFileId>,
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(_) =
Expand Down

0 comments on commit e700b48

Please sign in to comment.