internal: move missing unsafe diagnostic to hir

Author: matklad

crates/hir/src/diagnostics.rs

@@ -301,3 +301,27 @@ impl Diagnostic for BreakOutsideOfLoop {
         self
     }
 }
+
+// Diagnostic: missing-unsafe
+//
+// This diagnostic is triggered if an operation marked as `unsafe` is used outside of an `unsafe` function or block.
+#[derive(Debug)]
+pub struct MissingUnsafe {
+    pub file: HirFileId,
+    pub expr: AstPtr<ast::Expr>,
+}
+
+impl Diagnostic for MissingUnsafe {
+    fn code(&self) -> DiagnosticCode {
+        DiagnosticCode("missing-unsafe")
+    }
+    fn message(&self) -> String {
+        format!("This operation is unsafe and requires an unsafe function or block")
+    }
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
+        InFile { file_id: self.file, value: self.expr.clone().into() }
+    }
+    fn as_any(&self) -> &(dyn Any + Send + 'static) {
+        self
+    }
+}

crates/hir/src/lib.rs

@@ -36,14 +36,14 @@ use std::{iter, sync::Arc};
 use arrayvec::ArrayVec;
 use base_db::{CrateDisplayName, CrateId, Edition, FileId};
 use diagnostics::{
-    BreakOutsideOfLoop, InactiveCode, MacroError, NoSuchField, UnimplementedBuiltinMacro,
-    UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule,
-    UnresolvedProcMacro,
+    BreakOutsideOfLoop, InactiveCode, MacroError, MissingUnsafe, NoSuchField,
+    UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
+    UnresolvedModule, UnresolvedProcMacro,
 };
 use either::Either;
 use hir_def::{
     adt::{ReprKind, VariantData},
-    body::BodyDiagnostic,
+    body::{BodyDiagnostic, SyntheticSyntax},
     expr::{BindingAnnotation, LabelId, Pat, PatId},
     item_tree::ItemTreeNode,
     lang_item::LangItemTarget,
@@ -1060,6 +1060,18 @@ impl Function {
             }
         }
 
+        for expr in hir_ty::diagnostics::missing_unsafe(db, self.id.into()) {
+            match source_map.as_ref().expr_syntax(expr) {
+                Ok(in_file) => {
+                    sink.push(MissingUnsafe { file: in_file.file_id, expr: in_file.value })
+                }
+                Err(SyntheticSyntax) => {
+                    // FIXME: The `expr` was desugared, report or assert that
+                    // this dosen't happen.
+                }
+            }
+        }
+
         hir_ty::diagnostics::validate_module_item(db, krate, self.id.into(), sink);
         hir_ty::diagnostics::validate_body(db, self.id.into(), sink);
     }

crates/hir_ty/src/diagnostics.rs

@@ -17,7 +17,10 @@ use crate::{
     diagnostics_sink::{Diagnostic, DiagnosticCode, DiagnosticSink},
 };
 
-pub use crate::diagnostics::expr::{record_literal_missing_fields, record_pattern_missing_fields};
+pub use crate::diagnostics::{
+    expr::{record_literal_missing_fields, record_pattern_missing_fields},
+    unsafe_check::missing_unsafe,
+};
 
 pub fn validate_module_item(
     db: &dyn HirDatabase,
@@ -35,8 +38,6 @@ pub fn validate_body(db: &dyn HirDatabase, owner: DefWithBodyId, sink: &mut Diag
     let infer = db.infer(owner);
     let mut validator = expr::ExprValidator::new(owner, infer.clone(), sink);
     validator.validate_body(db);
-    let mut validator = unsafe_check::UnsafeValidator::new(owner, infer, sink);
-    validator.validate_body(db);
 }
 
 // Diagnostic: missing-structure-fields
@@ -219,30 +220,6 @@ impl Diagnostic for RemoveThisSemicolon {
     }
 }
 
-// Diagnostic: missing-unsafe
-//
-// This diagnostic is triggered if an operation marked as `unsafe` is used outside of an `unsafe` function or block.
-#[derive(Debug)]
-pub struct MissingUnsafe {
-    pub file: HirFileId,
-    pub expr: AstPtr<ast::Expr>,
-}
-
-impl Diagnostic for MissingUnsafe {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("missing-unsafe")
-    }
-    fn message(&self) -> String {
-        format!("This operation is unsafe and requires an unsafe function or block")
-    }
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.expr.clone().into() }
-    }
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
-}
-
 // Diagnostic: mismatched-arg-count
 //
 // This diagnostic is triggered if a function is invoked with an incorrect amount of arguments.

crates/hir_ty/src/diagnostics/unsafe_check.rs

@@ -1,61 +1,32 @@
 //! Provides validations for unsafe code. Currently checks if unsafe functions are missing
 //! unsafe blocks.
 
-use std::sync::Arc;
-
 use hir_def::{
     body::Body,
     expr::{Expr, ExprId, UnaryOp},
     resolver::{resolver_for_expr, ResolveValueResult, ValueNs},
     DefWithBodyId,
 };
 
-use crate::{
-    db::HirDatabase, diagnostics::MissingUnsafe, diagnostics_sink::DiagnosticSink, InferenceResult,
-    Interner, TyExt, TyKind,
-};
+use crate::{db::HirDatabase, InferenceResult, Interner, TyExt, TyKind};
 
-pub(super) struct UnsafeValidator<'a, 'b: 'a> {
-    owner: DefWithBodyId,
-    infer: Arc<InferenceResult>,
-    sink: &'a mut DiagnosticSink<'b>,
-}
+pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> Vec<ExprId> {
+    let infer = db.infer(def);
 
-impl<'a, 'b> UnsafeValidator<'a, 'b> {
-    pub(super) fn new(
-        owner: DefWithBodyId,
-        infer: Arc<InferenceResult>,
-        sink: &'a mut DiagnosticSink<'b>,
-    ) -> UnsafeValidator<'a, 'b> {
-        UnsafeValidator { owner, infer, sink }
+    // let unsafe_expressions = ;
+    let is_unsafe = match def {
+        DefWithBodyId::FunctionId(it) => db.function_data(it).is_unsafe(),
+        DefWithBodyId::StaticId(_) | DefWithBodyId::ConstId(_) => false,
+    };
+    if is_unsafe {
+        return Vec::new();
     }
 
-    pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) {
-        let def = self.owner;
-        let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
-        let is_unsafe = match self.owner {
-            DefWithBodyId::FunctionId(it) => db.function_data(it).is_unsafe(),
-            DefWithBodyId::StaticId(_) | DefWithBodyId::ConstId(_) => false,
-        };
-        if is_unsafe
-            || unsafe_expressions
-                .iter()
-                .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block)
-                .count()
-                == 0
-        {
-            return;
-        }
-
-        let (_, body_source) = db.body_with_source_map(def);
-        for unsafe_expr in unsafe_expressions {
-            if !unsafe_expr.inside_unsafe_block {
-                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 })
-                }
-            }
-        }
-    }
+    unsafe_expressions(db, &infer, def)
+        .into_iter()
+        .filter(|it| !it.inside_unsafe_block)
+        .map(|it| it.expr)
+        .collect()
 }
 
 pub(crate) struct UnsafeExpr {
@@ -126,92 +97,3 @@ fn walk_unsafe(
         walk_unsafe(unsafe_exprs, db, infer, def, body, child, inside_unsafe_block);
     });
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::diagnostics::tests::check_diagnostics;
-
-    #[test]
-    fn missing_unsafe_diagnostic_with_raw_ptr() {
-        check_diagnostics(
-            r#"
-fn main() {
-    let x = &5 as *const usize;
-    unsafe { let y = *x; }
-    let z = *x;
-}         //^^ This operation is unsafe and requires an unsafe function or block
-"#,
-        )
-    }
-
-    #[test]
-    fn missing_unsafe_diagnostic_with_unsafe_call() {
-        check_diagnostics(
-            r#"
-struct HasUnsafe;
-
-impl HasUnsafe {
-    unsafe fn unsafe_fn(&self) {
-        let x = &5 as *const usize;
-        let y = *x;
-    }
-}
-
-unsafe fn unsafe_fn() {
-    let x = &5 as *const usize;
-    let y = *x;
-}
-
-fn main() {
-    unsafe_fn();
-  //^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
-    HasUnsafe.unsafe_fn();
-  //^^^^^^^^^^^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
-    unsafe {
-        unsafe_fn();
-        HasUnsafe.unsafe_fn();
-    }
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn missing_unsafe_diagnostic_with_static_mut() {
-        check_diagnostics(
-            r#"
-struct Ty {
-    a: u8,
-}
-
-static mut STATIC_MUT: Ty = Ty { a: 0 };
-
-fn main() {
-    let x = STATIC_MUT.a;
-          //^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
-    unsafe {
-        let x = STATIC_MUT.a;
-    }
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn no_missing_unsafe_diagnostic_with_safe_intrinsic() {
-        check_diagnostics(
-            r#"
-extern "rust-intrinsic" {
-    pub fn bitreverse(x: u32) -> u32; // Safe intrinsic
-    pub fn floorf32(x: f32) -> f32; // Unsafe intrinsic
-}
-
-fn main() {
-    let _ = bitreverse(12);
-    let _ = floorf32(12.0);
-          //^^^^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
-}
-"#,
-        );
-    }
-}

crates/hir_ty/src/infer.rs

@@ -36,8 +36,8 @@ use syntax::SmolStr;
 
 use super::{DomainGoal, InEnvironment, ProjectionTy, TraitEnvironment, TraitRef, Ty};
 use crate::{
-    db::HirDatabase, fold_tys, lower::ImplTraitLoweringMode,
-    to_assoc_type_id, AliasEq, AliasTy, Goal, Interner, Substitution, TyBuilder, TyExt, TyKind,
+    db::HirDatabase, fold_tys, lower::ImplTraitLoweringMode, to_assoc_type_id, AliasEq, AliasTy,
+    Goal, Interner, Substitution, TyBuilder, TyExt, TyKind,
 };
 
 // This lint has a false positive here. See the link below for details.

crates/ide/src/diagnostics.rs

@@ -855,6 +855,90 @@ impl Foo {
         Foo { bar: 0 }
     }
 }
+"#,
+        );
+    }
+
+    #[test]
+    fn missing_unsafe_diagnostic_with_raw_ptr() {
+        check_diagnostics(
+            r#"
+fn main() {
+    let x = &5 as *const usize;
+    unsafe { let y = *x; }
+    let z = *x;
+}         //^^ This operation is unsafe and requires an unsafe function or block
+"#,
+        )
+    }
+
+    #[test]
+    fn missing_unsafe_diagnostic_with_unsafe_call() {
+        check_diagnostics(
+            r#"
+struct HasUnsafe;
+
+impl HasUnsafe {
+    unsafe fn unsafe_fn(&self) {
+        let x = &5 as *const usize;
+        let y = *x;
+    }
+}
+
+unsafe fn unsafe_fn() {
+    let x = &5 as *const usize;
+    let y = *x;
+}
+
+fn main() {
+    unsafe_fn();
+  //^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+    HasUnsafe.unsafe_fn();
+  //^^^^^^^^^^^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+    unsafe {
+        unsafe_fn();
+        HasUnsafe.unsafe_fn();
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn missing_unsafe_diagnostic_with_static_mut() {
+        check_diagnostics(
+            r#"
+struct Ty {
+    a: u8,
+}
+
+static mut STATIC_MUT: Ty = Ty { a: 0 };
+
+fn main() {
+    let x = STATIC_MUT.a;
+          //^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+    unsafe {
+        let x = STATIC_MUT.a;
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn no_missing_unsafe_diagnostic_with_safe_intrinsic() {
+        check_diagnostics(
+            r#"
+extern "rust-intrinsic" {
+    pub fn bitreverse(x: u32) -> u32; // Safe intrinsic
+    pub fn floorf32(x: f32) -> f32; // Unsafe intrinsic
+}
+
+fn main() {
+    let _ = bitreverse(12);
+    let _ = floorf32(12.0);
+          //^^^^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block
+}
 "#,
         );
     }