aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-06-27 17:03:29 +0100
committerGitHub <[email protected]>2020-06-27 17:03:29 +0100
commit45fc8d5c84d40da7a5dbf9b1a23ec389f54d1a81 (patch)
treeebe09304a24180b010b21a6aea3a727433403565 /crates
parent9d1e2c4d9dc6c7f5fbaee5d9907d135f618d7ac6 (diff)
parent9777d2cb2dcea7b5a3b289708fea21b4bf787f0f (diff)
Merge #4587
4587: Add "missing unsafe" diagnostics r=Nashenas88 a=Nashenas88 Addresses #190 Co-authored-by: Paul Daniel Faria <[email protected]>
Diffstat (limited to 'crates')
-rw-r--r--crates/ra_hir/src/code_model.rs10
-rw-r--r--crates/ra_hir_def/src/body/lower.rs8
-rw-r--r--crates/ra_hir_def/src/expr.rs5
-rw-r--r--crates/ra_hir_ty/src/diagnostics.rs28
-rw-r--r--crates/ra_hir_ty/src/infer/expr.rs1
-rw-r--r--crates/ra_hir_ty/src/lib.rs1
-rw-r--r--crates/ra_hir_ty/src/test_db.rs9
-rw-r--r--crates/ra_hir_ty/src/tests.rs149
-rw-r--r--crates/ra_hir_ty/src/tests/simple.rs1
-rw-r--r--crates/ra_hir_ty/src/unsafe_validation.rs120
-rw-r--r--crates/ra_ide/src/syntax_highlighting/tags.rs4
11 files changed, 326 insertions, 10 deletions
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index a379b9f49..27e94b7fe 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -26,8 +26,10 @@ use hir_ty::{
26 autoderef, 26 autoderef,
27 display::{HirDisplayError, HirFormatter}, 27 display::{HirDisplayError, HirFormatter},
28 expr::ExprValidator, 28 expr::ExprValidator,
29 method_resolution, ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs, 29 method_resolution,
30 TraitEnvironment, Ty, TyDefId, TypeCtor, 30 unsafe_validation::UnsafeValidator,
31 ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs, TraitEnvironment, Ty,
32 TyDefId, TypeCtor,
31}; 33};
32use ra_db::{CrateId, CrateName, Edition, FileId}; 34use ra_db::{CrateId, CrateName, Edition, FileId};
33use ra_prof::profile; 35use ra_prof::profile;
@@ -677,7 +679,9 @@ impl Function {
677 let _p = profile("Function::diagnostics"); 679 let _p = profile("Function::diagnostics");
678 let infer = db.infer(self.id.into()); 680 let infer = db.infer(self.id.into());
679 infer.add_diagnostics(db, self.id, sink); 681 infer.add_diagnostics(db, self.id, sink);
680 let mut validator = ExprValidator::new(self.id, infer, sink); 682 let mut validator = ExprValidator::new(self.id, infer.clone(), sink);
683 validator.validate_body(db);
684 let mut validator = UnsafeValidator::new(self.id, infer, sink);
681 validator.validate_body(db); 685 validator.validate_body(db);
682 } 686 }
683} 687}
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index a7e2e0982..c6bc85e2f 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -176,6 +176,7 @@ impl ExprCollector<'_> {
176 if !self.expander.is_cfg_enabled(&expr) { 176 if !self.expander.is_cfg_enabled(&expr) {
177 return self.missing_expr(); 177 return self.missing_expr();
178 } 178 }
179
179 match expr { 180 match expr {
180 ast::Expr::IfExpr(e) => { 181 ast::Expr::IfExpr(e) => {
181 let then_branch = self.collect_block_opt(e.then_branch()); 182 let then_branch = self.collect_block_opt(e.then_branch());
@@ -218,8 +219,12 @@ impl ExprCollector<'_> {
218 let body = self.collect_block_opt(e.block_expr()); 219 let body = self.collect_block_opt(e.block_expr());
219 self.alloc_expr(Expr::TryBlock { body }, syntax_ptr) 220 self.alloc_expr(Expr::TryBlock { body }, syntax_ptr)
220 } 221 }
222 ast::Effect::Unsafe(_) => {
223 let body = self.collect_block_opt(e.block_expr());
224 self.alloc_expr(Expr::Unsafe { body }, syntax_ptr)
225 }
221 // FIXME: we need to record these effects somewhere... 226 // FIXME: we need to record these effects somewhere...
222 ast::Effect::Async(_) | ast::Effect::Label(_) | ast::Effect::Unsafe(_) => { 227 ast::Effect::Async(_) | ast::Effect::Label(_) => {
223 self.collect_block_opt(e.block_expr()) 228 self.collect_block_opt(e.block_expr())
224 } 229 }
225 }, 230 },
@@ -445,7 +450,6 @@ impl ExprCollector<'_> {
445 Mutability::from_mutable(e.mut_token().is_some()) 450 Mutability::from_mutable(e.mut_token().is_some())
446 }; 451 };
447 let rawness = Rawness::from_raw(raw_tok); 452 let rawness = Rawness::from_raw(raw_tok);
448
449 self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr) 453 self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr)
450 } 454 }
451 ast::Expr::PrefixExpr(e) => { 455 ast::Expr::PrefixExpr(e) => {
diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs
index ca49b26d1..e41cfc16b 100644
--- a/crates/ra_hir_def/src/expr.rs
+++ b/crates/ra_hir_def/src/expr.rs
@@ -150,6 +150,9 @@ pub enum Expr {
150 Tuple { 150 Tuple {
151 exprs: Vec<ExprId>, 151 exprs: Vec<ExprId>,
152 }, 152 },
153 Unsafe {
154 body: ExprId,
155 },
153 Array(Array), 156 Array(Array),
154 Literal(Literal), 157 Literal(Literal),
155} 158}
@@ -247,7 +250,7 @@ impl Expr {
247 f(*expr); 250 f(*expr);
248 } 251 }
249 } 252 }
250 Expr::TryBlock { body } => f(*body), 253 Expr::TryBlock { body } | Expr::Unsafe { body } => f(*body),
251 Expr::Loop { body, .. } => f(*body), 254 Expr::Loop { body, .. } => f(*body),
252 Expr::While { condition, body, .. } => { 255 Expr::While { condition, body, .. } => {
253 f(*condition); 256 f(*condition);
diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index ebd9cb08f..a59efb347 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -169,3 +169,31 @@ impl AstDiagnostic for BreakOutsideOfLoop {
169 ast::Expr::cast(node).unwrap() 169 ast::Expr::cast(node).unwrap()
170 } 170 }
171} 171}
172
173#[derive(Debug)]
174pub struct MissingUnsafe {
175 pub file: HirFileId,
176 pub expr: AstPtr<ast::Expr>,
177}
178
179impl Diagnostic for MissingUnsafe {
180 fn message(&self) -> String {
181 format!("This operation is unsafe and requires an unsafe function or block")
182 }
183 fn source(&self) -> InFile<SyntaxNodePtr> {
184 InFile { file_id: self.file, value: self.expr.clone().into() }
185 }
186 fn as_any(&self) -> &(dyn Any + Send + 'static) {
187 self
188 }
189}
190
191impl AstDiagnostic for MissingUnsafe {
192 type AST = ast::Expr;
193
194 fn ast(&self, db: &impl AstDatabase) -> Self::AST {
195 let root = db.parse_or_expand(self.source().file_id).unwrap();
196 let node = self.source().value.to_node(&root);
197 ast::Expr::cast(node).unwrap()
198 }
199}
diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs
index a9565a58d..61af5f064 100644
--- a/crates/ra_hir_ty/src/infer/expr.rs
+++ b/crates/ra_hir_ty/src/infer/expr.rs
@@ -142,6 +142,7 @@ impl<'a> InferenceContext<'a> {
142 // FIXME: Breakable block inference 142 // FIXME: Breakable block inference
143 self.infer_block(statements, *tail, expected) 143 self.infer_block(statements, *tail, expected)
144 } 144 }
145 Expr::Unsafe { body } => self.infer_expr(*body, expected),
145 Expr::TryBlock { body } => { 146 Expr::TryBlock { body } => {
146 let _inner = self.infer_expr(*body, expected); 147 let _inner = self.infer_expr(*body, expected);
147 // FIXME should be std::result::Result<{inner}, _> 148 // FIXME should be std::result::Result<{inner}, _>
diff --git a/crates/ra_hir_ty/src/lib.rs b/crates/ra_hir_ty/src/lib.rs
index f22232324..414158139 100644
--- a/crates/ra_hir_ty/src/lib.rs
+++ b/crates/ra_hir_ty/src/lib.rs
@@ -37,6 +37,7 @@ pub(crate) mod utils;
37pub mod db; 37pub mod db;
38pub mod diagnostics; 38pub mod diagnostics;
39pub mod expr; 39pub mod expr;
40pub mod unsafe_validation;
40 41
41#[cfg(test)] 42#[cfg(test)]
42mod tests; 43mod tests;
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index ad04e3e0f..9c2c6959d 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -11,7 +11,10 @@ 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, diagnostics::Diagnostic, expr::ExprValidator,
16 unsafe_validation::UnsafeValidator,
17};
15 18
16#[salsa::database( 19#[salsa::database(
17 ra_db::SourceDatabaseExtStorage, 20 ra_db::SourceDatabaseExtStorage,
@@ -119,7 +122,9 @@ impl TestDB {
119 let infer = self.infer(f.into()); 122 let infer = self.infer(f.into());
120 let mut sink = DiagnosticSink::new(&mut cb); 123 let mut sink = DiagnosticSink::new(&mut cb);
121 infer.add_diagnostics(self, f, &mut sink); 124 infer.add_diagnostics(self, f, &mut sink);
122 let mut validator = ExprValidator::new(f, infer, &mut sink); 125 let mut validator = ExprValidator::new(f, infer.clone(), &mut sink);
126 validator.validate_body(self);
127 let mut validator = UnsafeValidator::new(f, infer, &mut sink);
123 validator.validate_body(self); 128 validator.validate_body(self);
124 } 129 }
125 } 130 }
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 85ff26a36..2a85ce85d 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -539,6 +539,155 @@ 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 *const usize;
548 let y = *x;
549}
550",
551 )
552 .diagnostics()
553 .0;
554
555 assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#);
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 *const 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#""unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#);
577}
578
579#[test]
580fn missing_unsafe_diagnostic_with_unsafe_method_call() {
581 let diagnostics = TestDB::with_files(
582 r"
583struct HasUnsafe;
584
585impl HasUnsafe {
586 unsafe fn unsafe_fn(&self) {
587 let x = &5 as *const usize;
588 let y = *x;
589 }
590}
591
592fn missing_unsafe() {
593 HasUnsafe.unsafe_fn();
594}
595
596",
597 )
598 .diagnostics()
599 .0;
600
601 assert_snapshot!(diagnostics, @r#""HasUnsafe.unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#);
602}
603
604#[test]
605fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() {
606 let diagnostics = TestDB::with_files(
607 r"
608fn nothing_to_see_move_along() {
609 let x = &5 as *const usize;
610 unsafe {
611 let y = *x;
612 }
613}
614",
615 )
616 .diagnostics()
617 .0;
618
619 assert_snapshot!(diagnostics, @"");
620}
621
622#[test]
623fn missing_unsafe_diagnostic_with_raw_ptr_outside_unsafe_block() {
624 let diagnostics = TestDB::with_files(
625 r"
626fn nothing_to_see_move_along() {
627 let x = &5 as *const usize;
628 unsafe {
629 let y = *x;
630 }
631 let z = *x;
632}
633",
634 )
635 .diagnostics()
636 .0;
637
638 assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#);
639}
640
641#[test]
642fn no_missing_unsafe_diagnostic_with_unsafe_call_in_unsafe_block() {
643 let diagnostics = TestDB::with_files(
644 r"
645unsafe fn unsafe_fn() {
646 let x = &5 as *const usize;
647 let y = *x;
648}
649
650fn nothing_to_see_move_along() {
651 unsafe {
652 unsafe_fn();
653 }
654}
655",
656 )
657 .diagnostics()
658 .0;
659
660 assert_snapshot!(diagnostics, @"");
661}
662
663#[test]
664fn no_missing_unsafe_diagnostic_with_unsafe_method_call_in_unsafe_block() {
665 let diagnostics = TestDB::with_files(
666 r"
667struct HasUnsafe;
668
669impl HasUnsafe {
670 unsafe fn unsafe_fn() {
671 let x = &5 as *const usize;
672 let y = *x;
673 }
674}
675
676fn nothing_to_see_move_along() {
677 unsafe {
678 HasUnsafe.unsafe_fn();
679 }
680}
681
682",
683 )
684 .diagnostics()
685 .0;
686
687 assert_snapshot!(diagnostics, @"");
688}
689
690#[test]
542fn break_outside_of_loop() { 691fn break_outside_of_loop() {
543 let diagnostics = TestDB::with_files( 692 let diagnostics = TestDB::with_files(
544 r" 693 r"
diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs
index 7d8197f8b..cd919466f 100644
--- a/crates/ra_hir_ty/src/tests/simple.rs
+++ b/crates/ra_hir_ty/src/tests/simple.rs
@@ -1880,6 +1880,7 @@ fn main() {
1880 @r###" 1880 @r###"
1881 10..130 '{ ...2 }; }': () 1881 10..130 '{ ...2 }; }': ()
1882 20..21 'x': i32 1882 20..21 'x': i32
1883 24..37 'unsafe { 92 }': i32
1883 31..37 '{ 92 }': i32 1884 31..37 '{ 92 }': i32
1884 33..35 '92': i32 1885 33..35 '92': i32
1885 47..48 'y': {unknown} 1886 47..48 'y': {unknown}
diff --git a/crates/ra_hir_ty/src/unsafe_validation.rs b/crates/ra_hir_ty/src/unsafe_validation.rs
new file mode 100644
index 000000000..c512c4f8e
--- /dev/null
+++ b/crates/ra_hir_ty/src/unsafe_validation.rs
@@ -0,0 +1,120 @@
1//! Provides validations for unsafe code. Currently checks if unsafe functions are missing
2//! unsafe blocks.
3
4use std::sync::Arc;
5
6use hir_def::{
7 body::Body,
8 expr::{Expr, ExprId, UnaryOp},
9 DefWithBodyId, FunctionId,
10};
11use hir_expand::diagnostics::DiagnosticSink;
12
13use crate::{
14 db::HirDatabase, diagnostics::MissingUnsafe, lower::CallableDef, ApplicationTy,
15 InferenceResult, Ty, TypeCtor,
16};
17
18pub struct UnsafeValidator<'a, 'b: 'a> {
19 func: FunctionId,
20 infer: Arc<InferenceResult>,
21 sink: &'a mut DiagnosticSink<'b>,
22}
23
24impl<'a, 'b> UnsafeValidator<'a, 'b> {
25 pub fn new(
26 func: FunctionId,
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.into();
35 let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def);
36 let func_data = db.function_data(self.func);
37 if func_data.is_unsafe
38 || unsafe_expressions
39 .iter()
40 .filter(|unsafe_expr| !unsafe_expr.inside_unsafe_block)
41 .count()
42 == 0
43 {
44 return;
45 }
46
47 let (_, body_source) = db.body_with_source_map(def);
48 for unsafe_expr in unsafe_expressions {
49 if !unsafe_expr.inside_unsafe_block {
50 if let Ok(in_file) = body_source.as_ref().expr_syntax(unsafe_expr.expr) {
51 self.sink.push(MissingUnsafe { file: in_file.file_id, expr: in_file.value })
52 }
53 }
54 }
55 }
56}
57
58pub struct UnsafeExpr {
59 pub expr: ExprId,
60 pub inside_unsafe_block: bool,
61}
62
63pub fn unsafe_expressions(
64 db: &dyn HirDatabase,
65 infer: &InferenceResult,
66 def: DefWithBodyId,
67) -> Vec<UnsafeExpr> {
68 let mut unsafe_exprs = vec![];
69 let body = db.body(def);
70 walk_unsafe(&mut unsafe_exprs, db, infer, &body, body.body_expr, false);
71
72 unsafe_exprs
73}
74
75fn walk_unsafe(
76 unsafe_exprs: &mut Vec<UnsafeExpr>,
77 db: &dyn HirDatabase,
78 infer: &InferenceResult,
79 body: &Body,
80 current: ExprId,
81 inside_unsafe_block: bool,
82) {
83 let expr = &body.exprs[current];
84 match expr {
85 Expr::Call { callee, .. } => {
86 let ty = &infer[*callee];
87 if let &Ty::Apply(ApplicationTy {
88 ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)),
89 ..
90 }) = ty
91 {
92 if db.function_data(func).is_unsafe {
93 unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block });
94 }
95 }
96 }
97 Expr::MethodCall { .. } => {
98 if infer
99 .method_resolution(current)
100 .map(|func| db.function_data(func).is_unsafe)
101 .unwrap_or(false)
102 {
103 unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block });
104 }
105 }
106 Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
107 if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] {
108 unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block });
109 }
110 }
111 Expr::Unsafe { body: child } => {
112 return walk_unsafe(unsafe_exprs, db, infer, body, *child, true);
113 }
114 _ => {}
115 }
116
117 expr.walk_child_exprs(|child| {
118 walk_unsafe(unsafe_exprs, db, infer, body, child, inside_unsafe_block);
119 });
120}
diff --git a/crates/ra_ide/src/syntax_highlighting/tags.rs b/crates/ra_ide/src/syntax_highlighting/tags.rs
index e8e251cfc..13d9dd195 100644
--- a/crates/ra_ide/src/syntax_highlighting/tags.rs
+++ b/crates/ra_ide/src/syntax_highlighting/tags.rs
@@ -25,7 +25,6 @@ pub enum HighlightTag {
25 EnumVariant, 25 EnumVariant,
26 EscapeSequence, 26 EscapeSequence,
27 Field, 27 Field,
28 FormatSpecifier,
29 Function, 28 Function,
30 Generic, 29 Generic,
31 Keyword, 30 Keyword,
@@ -33,7 +32,6 @@ pub enum HighlightTag {
33 Macro, 32 Macro,
34 Module, 33 Module,
35 NumericLiteral, 34 NumericLiteral,
36 Operator,
37 SelfKeyword, 35 SelfKeyword,
38 SelfType, 36 SelfType,
39 Static, 37 Static,
@@ -45,6 +43,8 @@ pub enum HighlightTag {
45 Union, 43 Union,
46 Local, 44 Local,
47 UnresolvedReference, 45 UnresolvedReference,
46 FormatSpecifier,
47 Operator,
48} 48}
49 49
50#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] 50#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]