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

This commit is contained in:
Paul Daniel Faria 2020-05-27 08:51:08 -04:00
parent b9569886a9
commit 9ce44be2ab
3 changed files with 19 additions and 25 deletions

View file

@ -3,10 +3,7 @@
use std::any::Any; use std::any::Any;
use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile}; use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile};
use ra_syntax::{ use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr};
ast::{self, NameOwner},
AstNode, AstPtr, SyntaxNodePtr,
};
use stdx::format_to; use stdx::format_to;
pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path}; pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path};
@ -176,15 +173,15 @@ impl AstDiagnostic for BreakOutsideOfLoop {
#[derive(Debug)] #[derive(Debug)]
pub struct MissingUnsafe { pub struct MissingUnsafe {
pub file: HirFileId, pub file: HirFileId,
pub fn_def: AstPtr<ast::FnDef>, pub expr: AstPtr<ast::Expr>,
} }
impl Diagnostic for MissingUnsafe { impl Diagnostic for MissingUnsafe {
fn message(&self) -> String { 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> { 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) { fn as_any(&self) -> &(dyn Any + Send + 'static) {
self self
@ -192,11 +189,11 @@ impl Diagnostic for MissingUnsafe {
} }
impl AstDiagnostic for MissingUnsafe { impl AstDiagnostic for MissingUnsafe {
type AST = ast::Name; type AST = ast::Expr;
fn ast(&self, db: &impl AstDatabase) -> Self::AST { fn ast(&self, db: &impl AstDatabase) -> Self::AST {
let root = db.parse_or_expand(self.source().file_id).unwrap(); let root = db.parse_or_expand(self.source().file_id).unwrap();
let node = self.source().value.to_node(&root); let node = self.source().value.to_node(&root);
ast::FnDef::cast(node).unwrap().name().unwrap() ast::Expr::cast(node).unwrap()
} }
} }

View file

@ -2,9 +2,7 @@
use std::sync::Arc; use std::sync::Arc;
use hir_def::{ use hir_def::{path::path, resolver::HasResolver, AdtId, DefWithBodyId, FunctionId};
path::path, resolver::HasResolver, src::HasSource, AdtId, DefWithBodyId, FunctionId, Lookup,
};
use hir_expand::diagnostics::DiagnosticSink; use hir_expand::diagnostics::DiagnosticSink;
use ra_syntax::{ast, AstPtr}; use ra_syntax::{ast, AstPtr};
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
@ -346,7 +344,7 @@ pub fn unsafe_expressions(
} }
} }
Expr::Call { callee, .. } => { Expr::Call { callee, .. } => {
let ty = &infer.type_of_expr[*callee]; let ty = &infer[*callee];
if let &Ty::Apply(ApplicationTy { if let &Ty::Apply(ApplicationTy {
ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)),
.. ..
@ -361,7 +359,7 @@ pub fn unsafe_expressions(
if infer if infer
.method_resolution(id) .method_resolution(id)
.map(|func| db.function_data(func).is_unsafe) .map(|func| db.function_data(func).is_unsafe)
.unwrap_or_else(|| false) .unwrap_or(false)
{ {
unsafe_exprs.push(UnsafeExpr::new(id)); unsafe_exprs.push(UnsafeExpr::new(id));
} }
@ -409,7 +407,7 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
let func_data = db.function_data(self.func); let func_data = db.function_data(self.func);
if func_data.is_unsafe if func_data.is_unsafe
|| unsafe_expressions || unsafe_expressions
.into_iter() .iter()
.filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block) .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block)
.count() .count()
== 0 == 0
@ -417,12 +415,11 @@ impl<'a, 'b> UnsafeValidator<'a, 'b> {
return; return;
} }
let loc = self.func.lookup(db.upcast()); let (_, body_source) = db.body_with_source_map(def);
let in_file = loc.source(db.upcast()); for unsafe_expr in unsafe_expressions {
if let Ok(in_file) = body_source.as_ref().expr_syntax(unsafe_expr.expr) {
let file = in_file.file_id; self.sink.push(MissingUnsafe { file: in_file.file_id, expr: in_file.value })
let fn_def = AstPtr::new(&in_file.value); }
}
self.sink.push(MissingUnsafe { file, fn_def })
} }
} }

View file

@ -552,7 +552,7 @@ fn missing_unsafe() {
.diagnostics() .diagnostics()
.0; .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] #[test]
@ -573,7 +573,7 @@ fn missing_unsafe() {
.diagnostics() .diagnostics()
.0; .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] #[test]
@ -599,7 +599,7 @@ fn missing_unsafe() {
.diagnostics() .diagnostics()
.0; .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] #[test]