aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--crates/ra_hir/src/code_model.rs5
-rw-r--r--crates/ra_hir/src/diagnostics.rs50
-rw-r--r--crates/ra_hir_ty/src/diagnostics.rs16
-rw-r--r--crates/ra_hir_ty/src/expr.rs70
-rw-r--r--crates/ra_hir_ty/src/test_db.rs10
-rw-r--r--crates/ra_hir_ty/src/tests.rs78
-rw-r--r--crates/ra_ide/src/syntax_highlighting/tests.rs2
7 files changed, 158 insertions, 73 deletions
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index 131180a63..13e763e52 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -25,7 +25,7 @@ use hir_expand::{
25use hir_ty::{ 25use hir_ty::{
26 autoderef, 26 autoderef,
27 display::{HirDisplayError, HirFormatter}, 27 display::{HirDisplayError, HirFormatter},
28 expr::ExprValidator, 28 expr::{ExprValidator, UnsafeValidator},
29 method_resolution, ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs, 29 method_resolution, ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs,
30 TraitEnvironment, Ty, TyDefId, TypeCtor, 30 TraitEnvironment, Ty, TyDefId, TypeCtor,
31}; 31};
@@ -36,7 +36,6 @@ use rustc_hash::FxHashSet;
36 36
37use crate::{ 37use crate::{
38 db::{DefDatabase, HirDatabase}, 38 db::{DefDatabase, HirDatabase},
39 diagnostics::UnsafeValidator,
40 has_source::HasSource, 39 has_source::HasSource,
41 CallableDef, HirDisplay, InFile, Name, 40 CallableDef, HirDisplay, InFile, Name,
42}; 41};
@@ -680,7 +679,7 @@ impl Function {
680 infer.add_diagnostics(db, self.id, sink); 679 infer.add_diagnostics(db, self.id, sink);
681 let mut validator = ExprValidator::new(self.id, infer.clone(), sink); 680 let mut validator = ExprValidator::new(self.id, infer.clone(), sink);
682 validator.validate_body(db); 681 validator.validate_body(db);
683 let mut validator = UnsafeValidator::new(&self, infer, sink); 682 let mut validator = UnsafeValidator::new(self.id, infer, sink);
684 validator.validate_body(db); 683 validator.validate_body(db);
685 } 684 }
686} 685}
diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs
index 562f3fe5c..c82883d0c 100644
--- a/crates/ra_hir/src/diagnostics.rs
+++ b/crates/ra_hir/src/diagnostics.rs
@@ -2,53 +2,3 @@
2pub use hir_def::diagnostics::UnresolvedModule; 2pub use hir_def::diagnostics::UnresolvedModule;
3pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; 3pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
4pub use hir_ty::diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField}; 4pub use hir_ty::diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField};
5
6use std::sync::Arc;
7
8use crate::code_model::Function;
9use crate::db::HirDatabase;
10use crate::has_source::HasSource;
11use hir_ty::{
12 diagnostics::{MissingUnsafe, UnnecessaryUnsafe},
13 expr::unsafe_expressions,
14 InferenceResult,
15};
16use ra_syntax::AstPtr;
17
18pub struct UnsafeValidator<'a, 'b: 'a> {
19 func: &'a Function,
20 infer: Arc<InferenceResult>,
21 sink: &'a mut DiagnosticSink<'b>,
22}
23
24impl<'a, 'b> UnsafeValidator<'a, 'b> {
25 pub fn new(
26 func: &'a Function,
27 infer: Arc<InferenceResult>,
28 sink: &'a mut DiagnosticSink<'b>,
29 ) -> UnsafeValidator<'a, 'b> {
30 UnsafeValidator { func, infer, sink }
31 }
32
33 pub fn validate_body(&mut self, db: &dyn HirDatabase) {
34 let def = self.func.id.into();
35 let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
36 let func_data = db.function_data(self.func.id);
37 let unnecessary = func_data.is_unsafe && unsafe_expressions.len() == 0;
38 let missing = !func_data.is_unsafe && unsafe_expressions.len() > 0;
39 if !(unnecessary || missing) {
40 return;
41 }
42
43 let in_file = self.func.source(db);
44 let file = in_file.file_id;
45 let fn_def = AstPtr::new(&in_file.value);
46 let fn_name = func_data.name.clone().into();
47
48 if unnecessary {
49 self.sink.push(UnnecessaryUnsafe { file, fn_def, fn_name })
50 } else {
51 self.sink.push(MissingUnsafe { file, fn_def, fn_name })
52 }
53 }
54}
diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index 3469cc680..c6ca322fa 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -3,7 +3,7 @@
3use std::any::Any; 3use std::any::Any;
4 4
5use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile}; 5use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile};
6use ra_syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; 6use ra_syntax::{ast::{self, NameOwner}, AstNode, AstPtr, SyntaxNodePtr};
7use stdx::format_to; 7use stdx::format_to;
8 8
9pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path}; 9pub use hir_def::{diagnostics::UnresolvedModule, expr::MatchArm, path::Path};
@@ -174,12 +174,11 @@ impl AstDiagnostic for BreakOutsideOfLoop {
174pub struct MissingUnsafe { 174pub struct MissingUnsafe {
175 pub file: HirFileId, 175 pub file: HirFileId,
176 pub fn_def: AstPtr<ast::FnDef>, 176 pub fn_def: AstPtr<ast::FnDef>,
177 pub fn_name: Name,
178} 177}
179 178
180impl Diagnostic for MissingUnsafe { 179impl Diagnostic for MissingUnsafe {
181 fn message(&self) -> String { 180 fn message(&self) -> String {
182 format!("Missing unsafe marker on fn `{}`", self.fn_name) 181 format!("Missing unsafe keyword on fn")
183 } 182 }
184 fn source(&self) -> InFile<SyntaxNodePtr> { 183 fn source(&self) -> InFile<SyntaxNodePtr> {
185 InFile { file_id: self.file, value: self.fn_def.clone().into() } 184 InFile { file_id: self.file, value: self.fn_def.clone().into() }
@@ -190,12 +189,12 @@ impl Diagnostic for MissingUnsafe {
190} 189}
191 190
192impl AstDiagnostic for MissingUnsafe { 191impl AstDiagnostic for MissingUnsafe {
193 type AST = ast::FnDef; 192 type AST = ast::Name;
194 193
195 fn ast(&self, db: &impl AstDatabase) -> Self::AST { 194 fn ast(&self, db: &impl AstDatabase) -> Self::AST {
196 let root = db.parse_or_expand(self.source().file_id).unwrap(); 195 let root = db.parse_or_expand(self.source().file_id).unwrap();
197 let node = self.source().value.to_node(&root); 196 let node = self.source().value.to_node(&root);
198 ast::FnDef::cast(node).unwrap() 197 ast::FnDef::cast(node).unwrap().name().unwrap()
199 } 198 }
200} 199}
201 200
@@ -203,12 +202,11 @@ impl AstDiagnostic for MissingUnsafe {
203pub struct UnnecessaryUnsafe { 202pub struct UnnecessaryUnsafe {
204 pub file: HirFileId, 203 pub file: HirFileId,
205 pub fn_def: AstPtr<ast::FnDef>, 204 pub fn_def: AstPtr<ast::FnDef>,
206 pub fn_name: Name,
207} 205}
208 206
209impl Diagnostic for UnnecessaryUnsafe { 207impl Diagnostic for UnnecessaryUnsafe {
210 fn message(&self) -> String { 208 fn message(&self) -> String {
211 format!("Unnecessary unsafe marker on fn `{}`", self.fn_name) 209 format!("Unnecessary unsafe keyword on fn")
212 } 210 }
213 fn source(&self) -> InFile<SyntaxNodePtr> { 211 fn source(&self) -> InFile<SyntaxNodePtr> {
214 InFile { file_id: self.file, value: self.fn_def.clone().into() } 212 InFile { file_id: self.file, value: self.fn_def.clone().into() }
@@ -219,11 +217,11 @@ impl Diagnostic for UnnecessaryUnsafe {
219} 217}
220 218
221impl AstDiagnostic for UnnecessaryUnsafe { 219impl AstDiagnostic for UnnecessaryUnsafe {
222 type AST = ast::FnDef; 220 type AST = ast::Name;
223 221
224 fn ast(&self, db: &impl AstDatabase) -> Self::AST { 222 fn ast(&self, db: &impl AstDatabase) -> Self::AST {
225 let root = db.parse_or_expand(self.source().file_id).unwrap(); 223 let root = db.parse_or_expand(self.source().file_id).unwrap();
226 let node = self.source().value.to_node(&root); 224 let node = self.source().value.to_node(&root);
227 ast::FnDef::cast(node).unwrap() 225 ast::FnDef::cast(node).unwrap().name().unwrap()
228 } 226 }
229} 227}
diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs
index 795f1762c..7532e2dc7 100644
--- a/crates/ra_hir_ty/src/expr.rs
+++ b/crates/ra_hir_ty/src/expr.rs
@@ -2,14 +2,19 @@
2 2
3use std::sync::Arc; 3use std::sync::Arc;
4 4
5use hir_def::{path::path, resolver::HasResolver, AdtId, DefWithBodyId, FunctionId}; 5use hir_def::{
6 path::path, resolver::HasResolver, src::HasSource, AdtId, DefWithBodyId, FunctionId, Lookup,
7};
6use hir_expand::diagnostics::DiagnosticSink; 8use hir_expand::diagnostics::DiagnosticSink;
7use ra_syntax::{ast, AstPtr}; 9use ra_syntax::{ast, AstPtr};
8use rustc_hash::FxHashSet; 10use rustc_hash::FxHashSet;
9 11
10use crate::{ 12use crate::{
11 db::HirDatabase, 13 db::HirDatabase,
12 diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields}, 14 diagnostics::{
15 MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, MissingUnsafe,
16 UnnecessaryUnsafe,
17 },
13 utils::variant_data, 18 utils::variant_data,
14 ApplicationTy, InferenceResult, Ty, TypeCtor, 19 ApplicationTy, InferenceResult, Ty, TypeCtor,
15 _match::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, 20 _match::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
@@ -321,16 +326,63 @@ pub fn unsafe_expressions(
321 let mut unsafe_expr_ids = vec![]; 326 let mut unsafe_expr_ids = vec![];
322 let body = db.body(def); 327 let body = db.body(def);
323 for (id, expr) in body.exprs.iter() { 328 for (id, expr) in body.exprs.iter() {
324 if let Expr::Call { callee, .. } = expr { 329 match expr {
325 if infer 330 Expr::Call { callee, .. } => {
326 .method_resolution(*callee) 331 if infer
327 .map(|func| db.function_data(func).is_unsafe) 332 .method_resolution(*callee)
328 .unwrap_or(false) 333 .map(|func| db.function_data(func).is_unsafe)
329 { 334 .unwrap_or(false)
330 unsafe_expr_ids.push(id); 335 {
336 unsafe_expr_ids.push(id);
337 }
338 }
339 Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
340 if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] {
341 unsafe_expr_ids.push(id);
342 }
331 } 343 }
344 _ => {}
332 } 345 }
333 } 346 }
334 347
335 unsafe_expr_ids 348 unsafe_expr_ids
336} 349}
350
351pub struct UnsafeValidator<'a, 'b: 'a> {
352 func: FunctionId,
353 infer: Arc<InferenceResult>,
354 sink: &'a mut DiagnosticSink<'b>,
355}
356
357impl<'a, 'b> UnsafeValidator<'a, 'b> {
358 pub fn new(
359 func: FunctionId,
360 infer: Arc<InferenceResult>,
361 sink: &'a mut DiagnosticSink<'b>,
362 ) -> UnsafeValidator<'a, 'b> {
363 UnsafeValidator { func, infer, sink }
364 }
365
366 pub fn validate_body(&mut self, db: &dyn HirDatabase) {
367 let def = self.func.into();
368 let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
369 let func_data = db.function_data(self.func);
370 let unnecessary = func_data.is_unsafe && unsafe_expressions.len() == 0;
371 let missing = !func_data.is_unsafe && unsafe_expressions.len() > 0;
372 if !(unnecessary || missing) {
373 return;
374 }
375
376 let loc = self.func.lookup(db.upcast());
377 let in_file = loc.source(db.upcast());
378
379 let file = in_file.file_id;
380 let fn_def = AstPtr::new(&in_file.value);
381
382 if unnecessary {
383 self.sink.push(UnnecessaryUnsafe { file, fn_def })
384 } else {
385 self.sink.push(MissingUnsafe { file, fn_def })
386 }
387 }
388}
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index ad04e3e0f..9ccf2aa37 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -11,7 +11,11 @@ use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, SourceDataba
11use rustc_hash::FxHashSet; 11use rustc_hash::FxHashSet;
12use stdx::format_to; 12use stdx::format_to;
13 13
14use crate::{db::HirDatabase, diagnostics::Diagnostic, expr::ExprValidator}; 14use crate::{
15 db::HirDatabase,
16 diagnostics::Diagnostic,
17 expr::{ExprValidator, UnsafeValidator},
18};
15 19
16#[salsa::database( 20#[salsa::database(
17 ra_db::SourceDatabaseExtStorage, 21 ra_db::SourceDatabaseExtStorage,
@@ -119,7 +123,9 @@ impl TestDB {
119 let infer = self.infer(f.into()); 123 let infer = self.infer(f.into());
120 let mut sink = DiagnosticSink::new(&mut cb); 124 let mut sink = DiagnosticSink::new(&mut cb);
121 infer.add_diagnostics(self, f, &mut sink); 125 infer.add_diagnostics(self, f, &mut sink);
122 let mut validator = ExprValidator::new(f, infer, &mut sink); 126 let mut validator = ExprValidator::new(f, infer.clone(), &mut sink);
127 validator.validate_body(self);
128 let mut validator = UnsafeValidator::new(f, infer, &mut sink);
123 validator.validate_body(self); 129 validator.validate_body(self);
124 } 130 }
125 } 131 }
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 85ff26a36..4ff2b2d4a 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -539,6 +539,84 @@ fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() {
539} 539}
540 540
541#[test] 541#[test]
542fn missing_unsafe_diagnostic_with_raw_ptr() {
543 let diagnostics = TestDB::with_files(
544 r"
545//- /lib.rs
546fn missing_unsafe() {
547 let x = &5 as *usize;
548 let y = *x;
549}
550",
551 )
552 .diagnostics()
553 .0;
554
555 assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n let x = &5 as *usize;\n let y = *x;\n}": Missing unsafe keyword on fn"#);
556}
557
558#[test]
559fn missing_unsafe_diagnostic_with_unsafe_call() {
560 let diagnostics = TestDB::with_files(
561 r"
562//- /lib.rs
563unsafe fn unsafe_fn() {
564 let x = &5 as *usize;
565 let y = *x;
566}
567
568fn missing_unsafe() {
569 unsafe_fn();
570}
571",
572 )
573 .diagnostics()
574 .0;
575
576 assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n unsafe_fn();\n}": Missing unsafe keyword on fn"#);
577}
578
579#[test]
580fn missing_unsafe_diagnostic_with_unsafe_method_call() {
581 let diagnostics = TestDB::with_files(
582 r"
583//- /lib.rs
584struct HasUnsafe;
585
586impl HasUnsafe {
587 unsafe fn unsafe_fn() {
588 let x = &5 as *usize;
589 let y = *x;
590 }
591}
592
593fn missing_unsafe() {
594 HasUnsafe.unsafe_fn();
595}
596
597",
598 )
599 .diagnostics()
600 .0;
601
602 assert_snapshot!(diagnostics, @r#""fn missing_unsafe() {\n HasUnsafe.unsafe_fn();\n}": Missing unsafe keyword on fn"#);
603}
604
605#[test]
606fn unnecessary_unsafe_diagnostic() {
607 let diagnostics = TestDB::with_files(
608 r"
609//- /lib.rs
610unsafe fn actually_safe_fn() {}
611",
612 )
613 .diagnostics()
614 .0;
615
616 assert_snapshot!(diagnostics, @r#""unsafe fn actually_safe_fn() {}": Unnecessary unsafe keyword on fn"#);
617}
618
619#[test]
542fn break_outside_of_loop() { 620fn break_outside_of_loop() {
543 let diagnostics = TestDB::with_files( 621 let diagnostics = TestDB::with_files(
544 r" 622 r"
diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs
index 39cd74ac3..43f554a29 100644
--- a/crates/ra_ide/src/syntax_highlighting/tests.rs
+++ b/crates/ra_ide/src/syntax_highlighting/tests.rs
@@ -384,9 +384,11 @@ impl HasUnsafeFn {
384} 384}
385 385
386fn main() { 386fn main() {
387 let x = &5 as *usize;
387 unsafe { 388 unsafe {
388 unsafe_fn(); 389 unsafe_fn();
389 HasUnsafeFn.unsafe_method(); 390 HasUnsafeFn.unsafe_method();
391 let y = *x;
390 } 392 }
391} 393}
392"# 394"#