diff --git a/.gitignore b/.gitignore index aef0fac339..472fe1a137 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,5 @@ crates/*/target *.log *.iml .vscode/settings.json -*.html generated_assists.adoc generated_features.adoc diff --git a/Cargo.lock b/Cargo.lock index a70c22a949..dc758a1f09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -30,7 +30,7 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" dependencies = [ - "winapi 0.3.8", + "winapi 0.3.9", ] [[package]] @@ -1791,7 +1791,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ca8a50ef2360fbd1eeb0ecd46795a87a19024eb4b53c5dc916ca1fd95fe62438" dependencies = [ "libc", - "winapi 0.3.8", + "winapi 0.3.9", ] [[package]] diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index a379b9f49b..27e94b7fe7 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -26,8 +26,10 @@ use hir_ty::{ autoderef, display::{HirDisplayError, HirFormatter}, expr::ExprValidator, - method_resolution, ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs, - TraitEnvironment, Ty, TyDefId, TypeCtor, + method_resolution, + unsafe_validation::UnsafeValidator, + ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs, TraitEnvironment, Ty, + TyDefId, TypeCtor, }; use ra_db::{CrateId, CrateName, Edition, FileId}; use ra_prof::profile; @@ -677,7 +679,9 @@ impl Function { let _p = profile("Function::diagnostics"); let infer = db.infer(self.id.into()); infer.add_diagnostics(db, self.id, sink); - let mut validator = ExprValidator::new(self.id, infer, sink); + let mut validator = ExprValidator::new(self.id, infer.clone(), sink); + validator.validate_body(db); + let mut validator = UnsafeValidator::new(self.id, infer, sink); validator.validate_body(db); } } diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index a7e2e09822..c6bc85e2f1 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<'_> { if !self.expander.is_cfg_enabled(&expr) { return self.missing_expr(); } + match expr { ast::Expr::IfExpr(e) => { let then_branch = self.collect_block_opt(e.then_branch()); @@ -218,8 +219,12 @@ impl ExprCollector<'_> { let body = self.collect_block_opt(e.block_expr()); self.alloc_expr(Expr::TryBlock { body }, syntax_ptr) } + ast::Effect::Unsafe(_) => { + let body = self.collect_block_opt(e.block_expr()); + self.alloc_expr(Expr::Unsafe { body }, syntax_ptr) + } // FIXME: we need to record these effects somewhere... - ast::Effect::Async(_) | ast::Effect::Label(_) | ast::Effect::Unsafe(_) => { + ast::Effect::Async(_) | ast::Effect::Label(_) => { self.collect_block_opt(e.block_expr()) } }, @@ -445,7 +450,6 @@ impl ExprCollector<'_> { Mutability::from_mutable(e.mut_token().is_some()) }; let rawness = Rawness::from_raw(raw_tok); - self.alloc_expr(Expr::Ref { expr, rawness, mutability }, syntax_ptr) } ast::Expr::PrefixExpr(e) => { diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index ca49b26d15..e41cfc16b9 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -150,6 +150,9 @@ pub enum Expr { Tuple { exprs: Vec, }, + Unsafe { + body: ExprId, + }, Array(Array), Literal(Literal), } @@ -247,7 +250,7 @@ impl Expr { f(*expr); } } - Expr::TryBlock { body } => f(*body), + Expr::TryBlock { body } | Expr::Unsafe { body } => f(*body), Expr::Loop { body, .. } => f(*body), Expr::While { condition, body, .. } => { f(*condition); diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index ebd9cb08f9..a59efb3476 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 { ast::Expr::cast(node).unwrap() } } + +#[derive(Debug)] +pub struct MissingUnsafe { + pub file: HirFileId, + pub expr: AstPtr, +} + +impl Diagnostic for MissingUnsafe { + fn message(&self) -> String { + format!("This operation is unsafe and requires an unsafe function or block") + } + fn source(&self) -> InFile { + InFile { file_id: self.file, value: self.expr.clone().into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +impl AstDiagnostic for MissingUnsafe { + type AST = ast::Expr; + + 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::Expr::cast(node).unwrap() + } +} diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index a9565a58d1..61af5f0645 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> { // FIXME: Breakable block inference self.infer_block(statements, *tail, expected) } + Expr::Unsafe { body } => self.infer_expr(*body, expected), Expr::TryBlock { body } => { let _inner = self.infer_expr(*body, expected); // 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 f222323249..4141581393 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; pub mod db; pub mod diagnostics; pub mod expr; +pub mod unsafe_validation; #[cfg(test)] mod tests; diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs index ad04e3e0f9..9c2c6959dd 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 use rustc_hash::FxHashSet; use stdx::format_to; -use crate::{db::HirDatabase, diagnostics::Diagnostic, expr::ExprValidator}; +use crate::{ + db::HirDatabase, diagnostics::Diagnostic, expr::ExprValidator, + unsafe_validation::UnsafeValidator, +}; #[salsa::database( ra_db::SourceDatabaseExtStorage, @@ -119,7 +122,9 @@ impl TestDB { let infer = self.infer(f.into()); let mut sink = DiagnosticSink::new(&mut cb); infer.add_diagnostics(self, f, &mut sink); - let mut validator = ExprValidator::new(f, infer, &mut sink); + let mut validator = ExprValidator::new(f, infer.clone(), &mut sink); + validator.validate_body(self); + let mut validator = UnsafeValidator::new(f, infer, &mut sink); validator.validate_body(self); } } diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 85ff26a368..2a85ce85d6 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -538,6 +538,155 @@ fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { assert_snapshot!(diagnostics, @""); } +#[test] +fn missing_unsafe_diagnostic_with_raw_ptr() { + let diagnostics = TestDB::with_files( + r" +//- /lib.rs +fn missing_unsafe() { + let x = &5 as *const usize; + let y = *x; +} +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#); +} + +#[test] +fn missing_unsafe_diagnostic_with_unsafe_call() { + let diagnostics = TestDB::with_files( + r" +//- /lib.rs +unsafe fn unsafe_fn() { + let x = &5 as *const usize; + let y = *x; +} + +fn missing_unsafe() { + unsafe_fn(); +} +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @r#""unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#); +} + +#[test] +fn missing_unsafe_diagnostic_with_unsafe_method_call() { + let diagnostics = TestDB::with_files( + r" +struct HasUnsafe; + +impl HasUnsafe { + unsafe fn unsafe_fn(&self) { + let x = &5 as *const usize; + let y = *x; + } +} + +fn missing_unsafe() { + HasUnsafe.unsafe_fn(); +} + +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @r#""HasUnsafe.unsafe_fn()": This operation is unsafe and requires an unsafe function or block"#); +} + +#[test] +fn no_missing_unsafe_diagnostic_with_raw_ptr_in_unsafe_block() { + let diagnostics = TestDB::with_files( + r" +fn nothing_to_see_move_along() { + let x = &5 as *const usize; + unsafe { + let y = *x; + } +} +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @""); +} + +#[test] +fn missing_unsafe_diagnostic_with_raw_ptr_outside_unsafe_block() { + let diagnostics = TestDB::with_files( + r" +fn nothing_to_see_move_along() { + let x = &5 as *const usize; + unsafe { + let y = *x; + } + let z = *x; +} +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @r#""*x": This operation is unsafe and requires an unsafe function or block"#); +} + +#[test] +fn no_missing_unsafe_diagnostic_with_unsafe_call_in_unsafe_block() { + let diagnostics = TestDB::with_files( + r" +unsafe fn unsafe_fn() { + let x = &5 as *const usize; + let y = *x; +} + +fn nothing_to_see_move_along() { + unsafe { + unsafe_fn(); + } +} +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @""); +} + +#[test] +fn no_missing_unsafe_diagnostic_with_unsafe_method_call_in_unsafe_block() { + let diagnostics = TestDB::with_files( + r" +struct HasUnsafe; + +impl HasUnsafe { + unsafe fn unsafe_fn() { + let x = &5 as *const usize; + let y = *x; + } +} + +fn nothing_to_see_move_along() { + unsafe { + HasUnsafe.unsafe_fn(); + } +} + +", + ) + .diagnostics() + .0; + + assert_snapshot!(diagnostics, @""); +} + #[test] fn break_outside_of_loop() { let diagnostics = TestDB::with_files( diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index 7d8197f8b0..cd919466f4 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() { @r###" 10..130 '{ ...2 }; }': () 20..21 'x': i32 + 24..37 'unsafe { 92 }': i32 31..37 '{ 92 }': i32 33..35 '92': i32 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 0000000000..c512c4f8e9 --- /dev/null +++ b/crates/ra_hir_ty/src/unsafe_validation.rs @@ -0,0 +1,120 @@ +//! 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}, + DefWithBodyId, FunctionId, +}; +use hir_expand::diagnostics::DiagnosticSink; + +use crate::{ + db::HirDatabase, diagnostics::MissingUnsafe, lower::CallableDef, ApplicationTy, + InferenceResult, Ty, TypeCtor, +}; + +pub struct UnsafeValidator<'a, 'b: 'a> { + func: FunctionId, + infer: Arc, + sink: &'a mut DiagnosticSink<'b>, +} + +impl<'a, 'b> UnsafeValidator<'a, 'b> { + pub fn new( + func: FunctionId, + infer: Arc, + sink: &'a mut DiagnosticSink<'b>, + ) -> UnsafeValidator<'a, 'b> { + UnsafeValidator { func, infer, sink } + } + + pub fn validate_body(&mut self, db: &dyn HirDatabase) { + let def = self.func.into(); + let unsafe_expressions = unsafe_expressions(db, self.infer.as_ref(), def); + let func_data = db.function_data(self.func); + if func_data.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 }) + } + } + } + } +} + +pub struct UnsafeExpr { + pub expr: ExprId, + pub inside_unsafe_block: bool, +} + +pub fn unsafe_expressions( + db: &dyn HirDatabase, + infer: &InferenceResult, + def: DefWithBodyId, +) -> Vec { + let mut unsafe_exprs = vec![]; + let body = db.body(def); + walk_unsafe(&mut unsafe_exprs, db, infer, &body, body.body_expr, false); + + unsafe_exprs +} + +fn walk_unsafe( + unsafe_exprs: &mut Vec, + db: &dyn HirDatabase, + infer: &InferenceResult, + body: &Body, + current: ExprId, + inside_unsafe_block: bool, +) { + let expr = &body.exprs[current]; + match expr { + Expr::Call { callee, .. } => { + let ty = &infer[*callee]; + if let &Ty::Apply(ApplicationTy { + ctor: TypeCtor::FnDef(CallableDef::FunctionId(func)), + .. + }) = ty + { + if db.function_data(func).is_unsafe { + unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); + } + } + } + Expr::MethodCall { .. } => { + if infer + .method_resolution(current) + .map(|func| db.function_data(func).is_unsafe) + .unwrap_or(false) + { + unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); + } + } + Expr::UnaryOp { expr, op: UnaryOp::Deref } => { + if let Ty::Apply(ApplicationTy { ctor: TypeCtor::RawPtr(..), .. }) = &infer[*expr] { + unsafe_exprs.push(UnsafeExpr { expr: current, inside_unsafe_block }); + } + } + Expr::Unsafe { body: child } => { + return walk_unsafe(unsafe_exprs, db, infer, body, *child, true); + } + _ => {} + } + + expr.walk_child_exprs(|child| { + walk_unsafe(unsafe_exprs, db, infer, body, child, inside_unsafe_block); + }); +} diff --git a/crates/ra_ide/src/syntax_highlighting/tags.rs b/crates/ra_ide/src/syntax_highlighting/tags.rs index e8e251cfc0..13d9dd195a 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 { EnumVariant, EscapeSequence, Field, - FormatSpecifier, Function, Generic, Keyword, @@ -33,7 +32,6 @@ pub enum HighlightTag { Macro, Module, NumericLiteral, - Operator, SelfKeyword, SelfType, Static, @@ -45,6 +43,8 @@ pub enum HighlightTag { Union, Local, UnresolvedReference, + FormatSpecifier, + Operator, } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]