Track unsafe blocks, don't trigger missing unsafe diagnostic when unsafe exprs within unsafe block

7 files changed, 82 insertions, 19 deletions

diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index a7e2e0982..174bbf9f4 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -218,8 +218,12 @@ impl ExprCollector<'_> {
                     let body = self.collect_block_opt(e.block_expr());
                     self.alloc_expr(Expr::TryBlock { body }, syntax_ptr)
                 }
+                ast::Effect::Unsafe(_) => {
+                    let body = self.collect_block_opt(e.block_expr());
+                    self.alloc_expr(Expr::UnsafeBlock { body }, syntax_ptr)
+                }
                 // FIXME: we need to record these effects somewhere...
-                ast::Effect::Async(_) | ast::Effect::Label(_) | ast::Effect::Unsafe(_) => {
+                ast::Effect::Async(_) | ast::Effect::Label(_) => {
                     self.collect_block_opt(e.block_expr())
                 }
             },
diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs
index ca49b26d1..f5e3e74fb 100644
--- a/crates/ra_hir_def/src/expr.rs
+++ b/crates/ra_hir_def/src/expr.rs
@@ -150,6 +150,9 @@ pub enum Expr {
     Tuple {
         exprs: Vec<ExprId>,
     },
+    UnsafeBlock {
+        body: ExprId,
+    },
     Array(Array),
     Literal(Literal),
 }
@@ -247,7 +250,7 @@ impl Expr {
                     f(*expr);
                 }
             }
-            Expr::TryBlock { body } => f(*body),
+            Expr::TryBlock { body } | Expr::UnsafeBlock { body } => f(*body),
             Expr::Loop { body, .. } => f(*body),
             Expr::While { condition, body, .. } => {
                 f(*condition);
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs
index 1a0710e0b..d600aca21 100644
--- a/crates/ra_hir_ty/src/expr.rs
+++ b/crates/ra_hir_ty/src/expr.rs
@@ -318,46 +318,74 @@ pub fn record_pattern_missing_fields(
     Some((variant_def, missed_fields, exhaustive))
 }
 
+pub struct UnsafeExpr {
+    expr: ExprId,
+    inside_unsafe_block: bool,
+}
+
+impl UnsafeExpr {
+    fn new(expr: ExprId) -> Self {
+        Self { expr, inside_unsafe_block: false }
+    }
+}
+
 pub fn unsafe_expressions(
     db: &dyn HirDatabase,
     infer: &InferenceResult,
     def: DefWithBodyId,
-) -> Vec<ExprId> {
-    let mut unsafe_expr_ids = vec![];
+) -> Vec<UnsafeExpr> {
+    let mut unsafe_exprs = vec![];
+    let mut unsafe_block_scopes = vec![];
     let body = db.body(def);
+    let expr_scopes = db.expr_scopes(def);
     for (id, expr) in body.exprs.iter() {
         match expr {
+            Expr::UnsafeBlock { body } => {
+                if let Some(scope) = expr_scopes.scope_for(*body) {
+                    unsafe_block_scopes.push(scope);
+                }
+            }
             Expr::Call { callee, .. } => {
                 let ty = &infer.type_of_expr[*callee];
-                if let &Ty::Apply(ApplicationTy {ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), .. }) = ty {
+                if let &Ty::Apply(ApplicationTy {
+                    ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)),
+                    ..
+                }) = ty
+                {
                     if db.function_data(func).is_unsafe {
-                        unsafe_expr_ids.push(id);
+                        unsafe_exprs.push(UnsafeExpr::new(id));
                     }
-                 }
+                }
             }
             Expr::MethodCall { .. } => {
                 if infer
                     .method_resolution(id)
-                    .map(|func| {
-                        db.function_data(func).is_unsafe
-                    })
-                    .unwrap_or_else(|| {
-                        false
-                    })
+                    .map(|func| db.function_data(func).is_unsafe)
+                    .unwrap_or_else(|| false)
                 {
-                    unsafe_expr_ids.push(id);
+                    unsafe_exprs.push(UnsafeExpr::new(id));
                 }
             }
             Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
                 if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] {
-                    unsafe_expr_ids.push(id);
+                    unsafe_exprs.push(UnsafeExpr::new(id));
                 }
             }
             _ => {}
         }
     }
 
-    unsafe_expr_ids
+    'unsafe_exprs: for unsafe_expr in &mut unsafe_exprs {
+        let scope = expr_scopes.scope_for(unsafe_expr.expr);
+        for scope in expr_scopes.scope_chain(scope) {
+            if unsafe_block_scopes.contains(&scope) {
+                unsafe_expr.inside_unsafe_block = true;
+                continue 'unsafe_exprs;
+            }
+        }
+    }
+
+    unsafe_exprs
 }
 
 pub struct UnsafeValidator<'a, 'b: 'a> {
@@ -379,7 +407,13 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
         let def = self.func.into();
         let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
         let func_data = db.function_data(self.func);
-        if func_data.is_unsafe || unsafe_expressions.len() == 0 {
+        if func_data.is_unsafe
+            || unsafe_expressions
+                .into_iter()
+                .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block)
+                .count()
+                == 0
+        {
             return;
         }
 
diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs
index a9565a58d..df7bcb5fa 100644
--- a/crates/ra_hir_ty/src/infer/expr.rs
+++ b/crates/ra_hir_ty/src/infer/expr.rs
@@ -142,6 +142,7 @@ impl<'a> InferenceContext<'a> {
                 // FIXME: Breakable block inference
                 self.infer_block(statements, *tail, expected)
             }
+            Expr::UnsafeBlock { body } => self.infer_expr(*body, expected),
             Expr::TryBlock { body } => {
                 let _inner = self.infer_expr(*body, expected);
                 // FIXME should be std::result::Result<{inner}, _>
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 8889fa3ba..4bc2e8b27 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -608,10 +608,30 @@ fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() {
         r"
 //- /lib.rs
 fn nothing_to_see_move_along() {
+    let x = &5 as *const usize;
+    unsafe {
+        let y = *x;
+    }
+}
+",
+    )
+    .diagnostics()
+    .0;
+
+    assert_snapshot!(diagnostics, @"");
+}
+
+#[test]
+fn missing_unsafe_diagnostic_with_raw_ptr_outside_unsafe_block() {
+    let diagnostics = TestDB::with_files(
+        r"
+//- /lib.rs
+fn nothing_to_see_move_along() {
+    let x = &5 as *const usize;
     unsafe {
-        let x = &5 as *const usize;
         let y = *x;
     }
+    let z = *x;
 }
 ",
     )
diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs
index 7d8197f8b..cd919466f 100644
--- a/crates/ra_hir_ty/src/tests/simple.rs
+++ b/crates/ra_hir_ty/src/tests/simple.rs
@@ -1880,6 +1880,7 @@ fn main() {
         @r###"
     10..130 '{     ...2 }; }': ()
     20..21 'x': i32
+    24..37 'unsafe { 92 }': i32
     31..37 '{ 92 }': i32
     33..35 '92': i32
     47..48 'y': {unknown}
diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs
index a2366f997..58141da11 100644
--- a/crates/ra_syntax/src/ast/generated/nodes.rs
+++ b/crates/ra_syntax/src/ast/generated/nodes.rs
@@ -899,7 +899,7 @@ impl ast::LoopBodyOwner for LoopExpr {}
 impl LoopExpr {
     pub fn loop_token(&self) -> Option<SyntaxToken> { support::token(&self.syntax, T![loop]) }
 }
-/// Block expression with an optional prefix (label, try keyword,
+/// Block expression with an optional prefix (label, try ketword,
 /// unsafe keyword, async keyword...).
 ///
 /// ```