From 499d4c454d1a66d10c3cf4c9bacbdb15f295af39 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 24 May 2020 13:18:31 -0400 Subject: Remove UnnecessaryUnsafe diagnostic, Fix Expr::Call unsafe analysis --- crates/ra_hir_ty/src/diagnostics.rs | 33 ++++----------------------------- crates/ra_hir_ty/src/expr.rs | 29 ++++++++++------------------- crates/ra_hir_ty/src/tests.rs | 28 +++++++--------------------- 3 files changed, 21 insertions(+), 69 deletions(-) (limited to 'crates/ra_hir_ty') diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index c6ca322fa..0b7bcdc9d 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/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, -} - -impl Diagnostic for UnnecessaryUnsafe { - fn message(&self) -> String { - format!("Unnecessary unsafe keyword on fn") - } - fn source(&self) -> InFile { - 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() - } -} diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 04668f486..1a0710e0b 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/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 - .method_resolution(/* id */ *callee) - .map(|func| db.function_data(func).is_unsafe) - .unwrap_or(false) - { - unsafe_expr_ids.push(id); - } + let ty = &infer.type_of_expr[*callee]; + if let &Ty::Apply(ApplicationTy {ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), .. }) = ty { + if db.function_data(func).is_unsafe { + unsafe_expr_ids.push(id); + } + } } - Expr::MethodCall {/*_receiver, method_name,*/ .. } => { - // let receiver_ty = &infer.type_of_expr[*receiver]; - // receiver_ty + Expr::MethodCall { .. } => { 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; - let missing = !func_data.is_unsafe && unsafe_expressions.len() > 0; - if !(unnecessary || missing) { + if func_data.is_unsafe || unsafe_expressions.len() == 0 { 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 }) - } + self.sink.push(MissingUnsafe { file, fn_def }) } } diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index c1f6fbab8..28faccf38 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/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( -- cgit v1.2.3