Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0b95bed
Add unsafe diagnostics and unsafe highlighting
Nashenas88 May 23, 2020
daf1cac
Move diagnostics back into expr, add tests for diagnostics, fix logic…
Nashenas88 May 24, 2020
b358fbf
Add tests covering unsafe blocks, more attempts to get call expr test…
Nashenas88 May 24, 2020
499d4c4
Remove UnnecessaryUnsafe diagnostic, Fix Expr::Call unsafe analysis
Nashenas88 May 24, 2020
c622551
Fix typo in test
Nashenas88 May 24, 2020
3df0f9c
Add missing self param to test
Nashenas88 May 24, 2020
278cbf1
Track unsafe blocks, don't trigger missing unsafe diagnostic when uns…
Nashenas88 May 24, 2020
b956988
Rename Expr::UnsafeBlock to Expr::Unsafe
Nashenas88 May 24, 2020
9ce44be
Address review comments, have MissingUnsafe diagnostic point to each …
Nashenas88 May 27, 2020
7f2219d
Track expr parents during lowering, use parent map when checking if u…
Nashenas88 May 28, 2020
6c16823
Account for deref token in syntax highlighting of unsafe, add test fo…
Nashenas88 May 28, 2020
f678e0d
Add HighlightTag::Operator, use it for unsafe deref. Move unsafe vali…
Nashenas88 May 28, 2020
2608a6f
unsafe: Clean up, improve tracking, add debug_assert
Nashenas88 May 29, 2020
f78df42
Fix issues caused during rebase
Nashenas88 Jun 2, 2020
2fc92fa
Remove track_parent and parent_map, replace with simple walk in missi…
Nashenas88 Jun 2, 2020
2ca52bb
Revert ide highlighting changes (addressing on another branch)
Nashenas88 Jun 2, 2020
28bb8ed
Cleanup changes leftover from previous tracking attempt
Nashenas88 Jun 2, 2020
b1992b4
Remove unneeded code, filename from tests, fix rebasing issues
Nashenas88 Jun 27, 2020
b7e25ba
Improve perf of finding unsafe exprs
Nashenas88 Jun 27, 2020
68a649d
Simplify unsafe expr collection match
Nashenas88 Jun 27, 2020
9777d2c
Remove html from gitignore so highlight snapshots are not ignored
Nashenas88 Jun 27, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@ crates/*/target
*.log
*.iml
.vscode/settings.json
*.html
generated_assists.adoc
generated_features.adoc
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions crates/ra_hir/src/code_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
8 changes: 6 additions & 2 deletions crates/ra_hir_def/src/body/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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())
}
},
Expand Down Expand Up @@ -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) => {
Expand Down
5 changes: 4 additions & 1 deletion crates/ra_hir_def/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ pub enum Expr {
Tuple {
exprs: Vec<ExprId>,
},
Unsafe {
body: ExprId,
},
Array(Array),
Literal(Literal),
}
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions crates/ra_hir_ty/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,31 @@ impl AstDiagnostic for BreakOutsideOfLoop {
ast::Expr::cast(node).unwrap()
}
}

#[derive(Debug)]
pub struct MissingUnsafe {
pub file: HirFileId,
pub expr: AstPtr<ast::Expr>,
}

impl Diagnostic for MissingUnsafe {
fn message(&self) -> String {
format!("This operation is unsafe and requires an unsafe function or block")
}
fn source(&self) -> InFile<SyntaxNodePtr> {
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()
}
}
1 change: 1 addition & 0 deletions crates/ra_hir_ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}, _>
Expand Down
1 change: 1 addition & 0 deletions crates/ra_hir_ty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 7 additions & 2 deletions crates/ra_hir_ty/src/test_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand Down
149 changes: 149 additions & 0 deletions crates/ra_hir_ty/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions crates/ra_hir_ty/src/tests/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading