From 82780d8caf7bd034d4ef0e3a62789f61a047eb81 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Thu, 9 Mar 2023 15:46:54 +0800 Subject: [PATCH] feat: add an autofix for inserting an unsafe block to missing unsafe diagnostic --- .../src/handlers/missing_unsafe.rs | 196 +++++++++++++++++- 1 file changed, 189 insertions(+), 7 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index ea1ea5a216..60086ed4a4 100644 --- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -1,4 +1,11 @@ -use crate::{Diagnostic, DiagnosticsContext}; +use hir::db::AstDatabase; +use ide_db::{assists::Assist, source_change::SourceChange}; +use syntax::ast::{ExprStmt, LetStmt}; +use syntax::AstNode; +use syntax::{ast, SyntaxNode}; +use text_edit::TextEdit; + +use crate::{fix, Diagnostic, DiagnosticsContext}; // Diagnostic: missing-unsafe // @@ -9,11 +16,60 @@ pub(crate) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsaf "this operation is unsafe and requires an unsafe function or block", ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, ) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; + let expr = d.expr.value.to_node(&root); + + let node_to_add_unsafe_block = pick_best_node_to_add_unsafe_block(ctx, &expr); + + let replacement = format!("unsafe {{ {} }}", node_to_add_unsafe_block.text()); + let edit = TextEdit::replace(node_to_add_unsafe_block.text_range(), replacement); + let source_change = + SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + Some(vec![fix("add_unsafe", "Add unsafe block", source_change, expr.syntax().text_range())]) +} + +// Find the let statement or expression statement closest to the `expr` in the +// ancestor chain. +// +// Why don't we just add an unsafe block around the `expr`? +// +// Consider this example: +// ``` +// STATIC_MUT += 1; +// ``` +// We can't add an unsafe block to the left-hand side of an assignment. +// ``` +// unsafe { STATIC_MUT } += 1; +// ``` +// +// Or this example: +// ``` +// let z = STATIC_MUT.a; +// ``` +// We can't add an unsafe block like this: +// ``` +// let z = unsafe { STATIC_MUT } .a; +// ``` +fn pick_best_node_to_add_unsafe_block( + ctx: &DiagnosticsContext<'_>, + expr: &ast::Expr, +) -> SyntaxNode { + let Some(let_or_expr_stmt) = ctx.sema.ancestors_with_macros(expr.syntax().clone()).find(|node| { + LetStmt::can_cast(node.kind()) || ExprStmt::can_cast(node.kind()) + }) else { + // Is this reachable? + return expr.syntax().clone(); + }; + let_or_expr_stmt } #[cfg(test)] mod tests { - use crate::tests::check_diagnostics; + use crate::tests::{check_diagnostics, check_fix}; #[test] fn missing_unsafe_diagnostic_with_raw_ptr() { @@ -23,7 +79,7 @@ fn main() { let x = &5 as *const usize; unsafe { let y = *x; } let z = *x; -} //^^ error: this operation is unsafe and requires an unsafe function or block +} //^^💡 error: this operation is unsafe and requires an unsafe function or block "#, ) } @@ -48,9 +104,9 @@ unsafe fn unsafe_fn() { fn main() { unsafe_fn(); - //^^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block HasUnsafe.unsafe_fn(); - //^^^^^^^^^^^^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block unsafe { unsafe_fn(); HasUnsafe.unsafe_fn(); @@ -72,7 +128,7 @@ static mut STATIC_MUT: Ty = Ty { a: 0 }; fn main() { let x = STATIC_MUT.a; - //^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block unsafe { let x = STATIC_MUT.a; } @@ -94,9 +150,135 @@ extern "rust-intrinsic" { fn main() { let _ = bitreverse(12); let _ = floorf32(12.0); - //^^^^^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block } "#, ); } + + #[test] + fn add_unsafe_block_when_dereferencing_a_raw_pointer() { + check_fix( + r#" +fn main() { + let x = &5 as *const usize; + let z = *x$0; +} +"#, + r#" +fn main() { + let x = &5 as *const usize; + unsafe { let z = *x; } +} +"#, + ); + } + + #[test] + fn add_unsafe_block_when_calling_unsafe_function() { + check_fix( + r#" +unsafe fn func() { + let x = &5 as *const usize; + let z = *x; +} +fn main() { + func$0(); +} +"#, + r#" +unsafe fn func() { + let x = &5 as *const usize; + let z = *x; +} +fn main() { + unsafe { func(); } +} +"#, + ) + } + + #[test] + fn add_unsafe_block_when_calling_unsafe_method() { + check_fix( + r#" +struct S(usize); +impl S { + unsafe fn func(&self) { + let x = &self.0 as *const usize; + let z = *x; + } +} +fn main() { + let s = S(5); + s.func$0(); +} +"#, + r#" +struct S(usize); +impl S { + unsafe fn func(&self) { + let x = &self.0 as *const usize; + let z = *x; + } +} +fn main() { + let s = S(5); + unsafe { s.func(); } +} +"#, + ) + } + + #[test] + fn add_unsafe_block_when_accessing_mutable_static() { + check_fix( + r#" +struct Ty { + a: u8, +} + +static mut STATIC_MUT: Ty = Ty { a: 0 }; + +fn main() { + let x = STATIC_MUT$0.a; +} +"#, + r#" +struct Ty { + a: u8, +} + +static mut STATIC_MUT: Ty = Ty { a: 0 }; + +fn main() { + unsafe { let x = STATIC_MUT.a; } +} +"#, + ) + } + + #[test] + fn add_unsafe_block_when_calling_unsafe_intrinsic() { + check_fix( + r#" +extern "rust-intrinsic" { + pub fn floorf32(x: f32) -> f32; +} + +fn main() { + let _ = floorf32$0(12.0); +} +"#, + r#" +extern "rust-intrinsic" { + pub fn floorf32(x: f32) -> f32; +} + +fn main() { + unsafe { let _ = floorf32(12.0); } +} +"#, + ) + } }