Address review comments, have MissingUnsafe diagnostic point to each unsafe use, update tests

3 files changed, 19 insertions, 25 deletions

diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index 0b7bcdc9d..a59efb347 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -3,10 +3,7 @@
 use std::any::Any;
 
 use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile};
-use ra_syntax::{
-    ast::{self, NameOwner},
-    AstNode, AstPtr, SyntaxNodePtr,
-};
+use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
 use stdx::format_to;
 
 pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path};
@@ -176,15 +173,15 @@ impl AstDiagnostic for BreakOutsideOfLoop {
 #[derive(Debug)]
 pub struct MissingUnsafe {
     pub file: HirFileId,
-    pub fn_def: AstPtr<ast::FnDef>,
+    pub expr: AstPtr<ast::Expr>,
 }
 
 impl Diagnostic for MissingUnsafe {
     fn message(&self) -> String {
-        format!("Missing unsafe keyword on fn")
+        format!("This operation is unsafe and requires an unsafe function or block")
     }
     fn source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.fn_def.clone().into() }
+        InFile { file_id: self.file, value: self.expr.clone().into() }
     }
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
         self
@@ -192,11 +189,11 @@ impl Diagnostic for MissingUnsafe {
 }
 
 impl AstDiagnostic for MissingUnsafe {
-    type AST = ast::Name;
+    type AST = ast::Expr;
 
     fn ast(&self, db: &impl AstDatabase) -> Self::AST {
         let root = db.parse_or_expand(self.source().file_id).unwrap();
         let node = self.source().value.to_node(&root);
-        ast::FnDef::cast(node).unwrap().name().unwrap()
+        ast::Expr::cast(node).unwrap()
     }
 }
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs
index ce73251b8..5f332aadb 100644
--- a/crates/ra_hir_ty/src/expr.rs
+++ b/crates/ra_hir_ty/src/expr.rs
@@ -2,9 +2,7 @@
 
 use std::sync::Arc;
 
-use hir_def::{
-    path::path, resolver::HasResolver, src::HasSource, AdtId, DefWithBodyId, FunctionId, Lookup,
-};
+use hir_def::{path::path, resolver::HasResolver, AdtId, DefWithBodyId, FunctionId};
 use hir_expand::diagnostics::DiagnosticSink;
 use ra_syntax::{ast, AstPtr};
 use rustc_hash::FxHashSet;
@@ -346,7 +344,7 @@ pub fn unsafe_expressions(
                 }
             }
             Expr::Call { callee, .. } => {
-                let ty = &infer.type_of_expr[*callee];
+                let ty = &infer[*callee];
                 if let &Ty::Apply(ApplicationTy {
                     ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)),
                     ..
@@ -361,7 +359,7 @@ pub fn unsafe_expressions(
                 if infer
                     .method_resolution(id)
                     .map(|func| db.function_data(func).is_unsafe)
-                    .unwrap_or_else(|| false)
+                    .unwrap_or(false)
                 {
                     unsafe_exprs.push(UnsafeExpr::new(id));
                 }
@@ -409,7 +407,7 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
         let func_data = db.function_data(self.func);
         if func_data.is_unsafe
             || unsafe_expressions
-                .into_iter()
+                .iter()
                 .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block)
                 .count()
                 == 0
@@ -417,12 +415,11 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
             return;
         }
 
-        let loc = self.func.lookup(db.upcast());
-        let in_file = loc.source(db.upcast());
-
-        let file = in_file.file_id;
-        let fn_def = AstPtr::new(&in_file.value);
-
-        self.sink.push(MissingUnsafe { file, fn_def })
+        let (_, body_source) = db.body_with_source_map(def);
+        for unsafe_expr in unsafe_expressions {
+            if let Ok(in_file) = body_source.as_ref().expr_syntax(unsafe_expr.expr) {
+                self.sink.push(MissingUnsafe { file: in_file.file_id, expr: in_file.value })
+            }
+        }
     }
 }
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 4bc2e8b27..26b3aeb50 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -552,7 +552,7 @@ fn missing_unsafe() {
     .diagnostics()
     .0;
 
-    assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n    let x = &5 as *const usize;\n    let y = *x;\n}": Missing unsafe keyword on fn"#);
+    assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#);
 }
 
 #[test]
@@ -573,7 +573,7 @@ fn missing_unsafe() {
     .diagnostics()
     .0;
 
-    assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n    unsafe_fn();\n}": Missing unsafe keyword on fn"#);
+    assert_snapshot!(diagnostics, @r#""unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#);
 }
 
 #[test]
@@ -599,7 +599,7 @@ fn missing_unsafe() {
     .diagnostics()
     .0;
 
-    assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n    HasUnsafe.unsafe_fn();\n}": Missing unsafe keyword on fn"#);
+    assert_snapshot!(diagnostics, @r#""HasUnsafe.unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#);
 }
 
 #[test]