Remove UnnecessaryUnsafe diagnostic, Fix Expr::Call unsafe analysis

crates/ra_hir_ty/src/diagnostics.rs

@@ -3,7 +3,10 @@
 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::{self, NameOwner},
+    AstNode, AstPtr, SyntaxNodePtr,
+};
 use stdx::format_to;
 
 pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path};
@@ -197,31 +200,3 @@ impl AstDiagnostic for MissingUnsafe {
         ast::FnDef::cast(node).unwrap().name().unwrap()
     }
 }
-
-#[derive(Debug)]
-pub struct UnnecessaryUnsafe {
-    pub file: HirFileId,
-    pub fn_def: AstPtr<ast::FnDef>,
-}
-
-impl Diagnostic for UnnecessaryUnsafe {
-    fn message(&self) -> String {
-        format!("Unnecessary unsafe keyword on fn")
-    }
-    fn source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.fn_def.clone().into() }
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
-impl AstDiagnostic for UnnecessaryUnsafe {
-    type AST = ast::Name;
-
-    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()
-    }
-}

crates/ra_hir_ty/src/expr.rs

@@ -13,8 +13,8 @@ use crate::{
     db::HirDatabase,
     diagnostics::{
         MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, MissingUnsafe,
-        UnnecessaryUnsafe,
     },
+    lower::CallableDef,
     utils::variant_data,
     ApplicationTy, InferenceResult, Ty, TypeCtor,
     _match::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
@@ -328,17 +328,14 @@ pub fn unsafe_expressions(
     for (id, expr) in body.exprs.iter() {
         match expr {
             Expr::Call { callee, .. } => {
-                if infer
+                let ty = &infer.type_of_expr[*callee];
-                    .method_resolution(/* id */ *callee)
+                if let &Ty::Apply(ApplicationTy {ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), .. }) = ty {
-                    .map(|func| db.function_data(func).is_unsafe)
+                    if db.function_data(func).is_unsafe {
-                    .unwrap_or(false)
-                {
                         unsafe_expr_ids.push(id);
                     }
                  }
-            Expr::MethodCall {/*_receiver, method_name,*/ .. } => {
+            }
-                // let receiver_ty = &infer.type_of_expr[*receiver];
+            Expr::MethodCall { .. } => {
-                // receiver_ty
                 if infer
                     .method_resolution(id)
                     .map(|func| {
@@ -382,9 +379,7 @@ 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);
-        let unnecessary = func_data.is_unsafe && unsafe_expressions.len() == 0;
+        if func_data.is_unsafe || unsafe_expressions.len() == 0 {
-        let missing = !func_data.is_unsafe && unsafe_expressions.len() > 0;
-        if !(unnecessary || missing) {
             return;
         }
 
@@ -394,10 +389,6 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
         let file = in_file.file_id;
         let fn_def = AstPtr::new(&in_file.value);
 
-        if unnecessary {
-            self.sink.push(UnnecessaryUnsafe { file, fn_def })
-        } else {
         self.sink.push(MissingUnsafe { file, fn_def })
     }
-    }
 }

crates/ra_hir_ty/src/tests.rs

@@ -544,7 +544,7 @@ fn missing_unsafe_diagnostic_with_raw_ptr() {
         r"
 //- /lib.rs
 fn missing_unsafe() {
-    let x = &5 as *usize;
+    let x = &5 as *const usize;
     let y = *x;
 }
 ",
@@ -552,7 +552,7 @@ fn missing_unsafe() {
     .diagnostics()
     .0;
 
-    assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n    let x = &5 as *usize;\n    let y = *x;\n}": Missing unsafe keyword on fn"#);
+    assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n    let x = &5 as *const usize;\n    let y = *x;\n}": Missing unsafe keyword on fn"#);
 }
 
 #[test]
@@ -561,7 +561,7 @@ fn missing_unsafe_diagnostic_with_unsafe_call() {
         r"
 //- /lib.rs
 unsafe fn unsafe_fn() {
-    let x = &5 as *usize;
+    let x = &5 as *const usize;
     let y = *x;
 }
 
@@ -585,7 +585,7 @@ struct HasUnsafe;
 
 impl HasUnsafe {
     unsafe fn unsafe_fn() {
-        let x = &5 as *usize;
+        let x = &5 as *const usize;
         let y = *x;
     }
 }
@@ -609,7 +609,7 @@ fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() {
 //- /lib.rs
 fn nothing_to_see_move_along() {
     unsafe {
-        let x = &5 as *usize;
+        let x = &5 as *const usize;
         let y = *x;
     }
 }
@@ -627,7 +627,7 @@ fn no_missing_unsafe_diagnostic_with_unsafe_call_in_unsafe_block() {
         r"
 //- /lib.rs
 unsafe fn unsafe_fn() {
-    let x = &5 as *usize;
+    let x = &5 as *const usize;
     let y = *x;
 }
 
@@ -653,7 +653,7 @@ struct HasUnsafe;
 
 impl HasUnsafe {
     unsafe fn unsafe_fn() {
-        let x = &5 as *usize;
+        let x = &5 as *const usize;
         let y = *x;
     }
 }
@@ -672,20 +672,6 @@ fn nothing_to_see_move_along() {
     assert_snapshot!(diagnostics, @"");
 }
 
-#[test]
-fn unnecessary_unsafe_diagnostic() {
-    let diagnostics = TestDB::with_files(
-        r"
-//- /lib.rs
-unsafe fn actually_safe_fn() {}
-",
-    )
-    .diagnostics()
-    .0;
-
-    assert_snapshot!(diagnostics, @r#""unsafe fn actually_safe_fn() {}": Unnecessary unsafe keyword on fn"#);
-}
-
 #[test]
 fn break_outside_of_loop() {
     let diagnostics = TestDB::with_files(

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 ketword,
+/// Block expression with an optional prefix (label, try keyword,
 /// unsafe keyword, async keyword...).
 ///
 /// ```