mirror of
https://github.com/rust-lang/rust-analyzer
synced 2025-01-13 13:48:50 +00:00
Merge pull request #18609 from ChayimFriedman2/unsafe-coverage
feat: Extend reported unsafe operations
This commit is contained in:
Lukas Wirth
GitHub
commit
edb432639b
7 changed files with 511 additions and 125 deletions
|
|
@ -408,7 +408,8 @@ impl Body {
|
|||
f(else_branch);
|
||||
}
|
||||
}
|
||||
Expr::Let { expr, .. } => {
|
||||
Expr::Let { expr, pat } => {
|
||||
self.walk_exprs_in_pat(*pat, &mut f);
|
||||
f(*expr);
|
||||
}
|
||||
Expr::Block { statements, tail, .. }
|
||||
|
|
@ -442,6 +443,137 @@ impl Body {
|
|||
f(*receiver);
|
||||
args.iter().copied().for_each(f);
|
||||
}
|
||||
Expr::Match { expr, arms } => {
|
||||
f(*expr);
|
||||
arms.iter().for_each(|arm| {
|
||||
f(arm.expr);
|
||||
self.walk_exprs_in_pat(arm.pat, &mut f);
|
||||
});
|
||||
}
|
||||
Expr::Break { expr, .. }
|
||||
| Expr::Return { expr }
|
||||
| Expr::Yield { expr }
|
||||
| Expr::Yeet { expr } => {
|
||||
if let &Some(expr) = expr {
|
||||
f(expr);
|
||||
}
|
||||
}
|
||||
Expr::Become { expr } => f(*expr),
|
||||
Expr::RecordLit { fields, spread, .. } => {
|
||||
for field in fields.iter() {
|
||||
f(field.expr);
|
||||
}
|
||||
if let &Some(expr) = spread {
|
||||
f(expr);
|
||||
}
|
||||
}
|
||||
Expr::Closure { body, .. } => {
|
||||
f(*body);
|
||||
}
|
||||
Expr::BinaryOp { lhs, rhs, .. } => {
|
||||
f(*lhs);
|
||||
f(*rhs);
|
||||
}
|
||||
Expr::Range { lhs, rhs, .. } => {
|
||||
if let &Some(lhs) = rhs {
|
||||
f(lhs);
|
||||
}
|
||||
if let &Some(rhs) = lhs {
|
||||
f(rhs);
|
||||
}
|
||||
}
|
||||
Expr::Index { base, index, .. } => {
|
||||
f(*base);
|
||||
f(*index);
|
||||
}
|
||||
Expr::Field { expr, .. }
|
||||
| Expr::Await { expr }
|
||||
| Expr::Cast { expr, .. }
|
||||
| Expr::Ref { expr, .. }
|
||||
| Expr::UnaryOp { expr, .. }
|
||||
| Expr::Box { expr } => {
|
||||
f(*expr);
|
||||
}
|
||||
Expr::Tuple { exprs, .. } => exprs.iter().copied().for_each(f),
|
||||
Expr::Array(a) => match a {
|
||||
Array::ElementList { elements, .. } => elements.iter().copied().for_each(f),
|
||||
Array::Repeat { initializer, repeat } => {
|
||||
f(*initializer);
|
||||
f(*repeat)
|
||||
}
|
||||
},
|
||||
&Expr::Assignment { target, value } => {
|
||||
self.walk_exprs_in_pat(target, &mut f);
|
||||
f(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn walk_child_exprs_without_pats(&self, expr_id: ExprId, mut f: impl FnMut(ExprId)) {
|
||||
let expr = &self[expr_id];
|
||||
match expr {
|
||||
Expr::Continue { .. }
|
||||
| Expr::Const(_)
|
||||
| Expr::Missing
|
||||
| Expr::Path(_)
|
||||
| Expr::OffsetOf(_)
|
||||
| Expr::Literal(_)
|
||||
| Expr::Underscore => {}
|
||||
Expr::InlineAsm(it) => it.operands.iter().for_each(|(_, op)| match op {
|
||||
AsmOperand::In { expr, .. }
|
||||
| AsmOperand::Out { expr: Some(expr), .. }
|
||||
| AsmOperand::InOut { expr, .. } => f(*expr),
|
||||
AsmOperand::SplitInOut { in_expr, out_expr, .. } => {
|
||||
f(*in_expr);
|
||||
if let Some(out_expr) = out_expr {
|
||||
f(*out_expr);
|
||||
}
|
||||
}
|
||||
AsmOperand::Out { expr: None, .. }
|
||||
| AsmOperand::Const(_)
|
||||
| AsmOperand::Label(_)
|
||||
| AsmOperand::Sym(_) => (),
|
||||
}),
|
||||
Expr::If { condition, then_branch, else_branch } => {
|
||||
f(*condition);
|
||||
f(*then_branch);
|
||||
if let &Some(else_branch) = else_branch {
|
||||
f(else_branch);
|
||||
}
|
||||
}
|
||||
Expr::Let { expr, .. } => {
|
||||
f(*expr);
|
||||
}
|
||||
Expr::Block { statements, tail, .. }
|
||||
| Expr::Unsafe { statements, tail, .. }
|
||||
| Expr::Async { statements, tail, .. } => {
|
||||
for stmt in statements.iter() {
|
||||
match stmt {
|
||||
Statement::Let { initializer, else_branch, .. } => {
|
||||
if let &Some(expr) = initializer {
|
||||
f(expr);
|
||||
}
|
||||
if let &Some(expr) = else_branch {
|
||||
f(expr);
|
||||
}
|
||||
}
|
||||
Statement::Expr { expr: expression, .. } => f(*expression),
|
||||
Statement::Item(_) => (),
|
||||
}
|
||||
}
|
||||
if let &Some(expr) = tail {
|
||||
f(expr);
|
||||
}
|
||||
}
|
||||
Expr::Loop { body, .. } => f(*body),
|
||||
Expr::Call { callee, args, .. } => {
|
||||
f(*callee);
|
||||
args.iter().copied().for_each(f);
|
||||
}
|
||||
Expr::MethodCall { receiver, args, .. } => {
|
||||
f(*receiver);
|
||||
args.iter().copied().for_each(f);
|
||||
}
|
||||
Expr::Match { expr, arms } => {
|
||||
f(*expr);
|
||||
arms.iter().map(|arm| arm.expr).for_each(f);
|
||||
|
|
@ -498,10 +630,7 @@ impl Body {
|
|||
f(*repeat)
|
||||
}
|
||||
},
|
||||
&Expr::Assignment { target, value } => {
|
||||
self.walk_exprs_in_pat(target, &mut f);
|
||||
f(value);
|
||||
}
|
||||
&Expr::Assignment { target: _, value } => f(value),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -9,5 +9,5 @@ pub use crate::diagnostics::{
|
|||
expr::{
|
||||
record_literal_missing_fields, record_pattern_missing_fields, BodyValidationDiagnostic,
|
||||
},
|
||||
unsafe_check::{missing_unsafe, unsafe_expressions, UnsafeExpr},
|
||||
unsafe_check::{missing_unsafe, unsafe_expressions, InsideUnsafeBlock, UnsafetyReason},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,12 +1,16 @@
|
|||
//! Provides validations for unsafe code. Currently checks if unsafe functions are missing
|
||||
//! unsafe blocks.
|
||||
|
||||
use std::mem;
|
||||
|
||||
use either::Either;
|
||||
use hir_def::{
|
||||
body::Body,
|
||||
hir::{Expr, ExprId, ExprOrPatId, Pat, UnaryOp},
|
||||
resolver::{resolver_for_expr, ResolveValueResult, Resolver, ValueNs},
|
||||
hir::{Expr, ExprId, ExprOrPatId, Pat, PatId, Statement, UnaryOp},
|
||||
path::Path,
|
||||
resolver::{HasResolver, ResolveValueResult, Resolver, ValueNs},
|
||||
type_ref::Rawness,
|
||||
DefWithBodyId,
|
||||
AdtId, DefWithBodyId, FieldId, VariantId,
|
||||
};
|
||||
|
||||
use crate::{
|
||||
|
|
@ -16,7 +20,10 @@ use crate::{
|
|||
/// Returns `(unsafe_exprs, fn_is_unsafe)`.
|
||||
///
|
||||
/// If `fn_is_unsafe` is false, `unsafe_exprs` are hard errors. If true, they're `unsafe_op_in_unsafe_fn`.
|
||||
pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec<ExprOrPatId>, bool) {
|
||||
pub fn missing_unsafe(
|
||||
db: &dyn HirDatabase,
|
||||
def: DefWithBodyId,
|
||||
) -> (Vec<(ExprOrPatId, UnsafetyReason)>, bool) {
|
||||
let _p = tracing::info_span!("missing_unsafe").entered();
|
||||
|
||||
let mut res = Vec::new();
|
||||
|
|
@ -30,111 +37,243 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec<ExprOrPa
|
|||
|
||||
let body = db.body(def);
|
||||
let infer = db.infer(def);
|
||||
unsafe_expressions(db, &infer, def, &body, body.body_expr, &mut |expr| {
|
||||
if !expr.inside_unsafe_block {
|
||||
res.push(expr.node);
|
||||
let mut callback = |node, inside_unsafe_block, reason| {
|
||||
if inside_unsafe_block == InsideUnsafeBlock::No {
|
||||
res.push((node, reason));
|
||||
}
|
||||
});
|
||||
};
|
||||
let mut visitor = UnsafeVisitor::new(db, &infer, &body, def, &mut callback);
|
||||
visitor.walk_expr(body.body_expr);
|
||||
|
||||
if !is_unsafe {
|
||||
// Unsafety in function parameter patterns (that can only be union destructuring)
|
||||
// cannot be inserted into an unsafe block, so even with `unsafe_op_in_unsafe_fn`
|
||||
// it is turned off for unsafe functions.
|
||||
for ¶m in &body.params {
|
||||
visitor.walk_pat(param);
|
||||
}
|
||||
}
|
||||
|
||||
(res, is_unsafe)
|
||||
}
|
||||
|
||||
pub struct UnsafeExpr {
|
||||
pub node: ExprOrPatId,
|
||||
pub inside_unsafe_block: bool,
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub enum UnsafetyReason {
|
||||
UnionField,
|
||||
UnsafeFnCall,
|
||||
InlineAsm,
|
||||
RawPtrDeref,
|
||||
MutableStatic,
|
||||
ExternStatic,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum InsideUnsafeBlock {
|
||||
No,
|
||||
Yes,
|
||||
}
|
||||
|
||||
// FIXME: Move this out, its not a diagnostic only thing anymore, and handle unsafe pattern accesses as well
|
||||
pub fn unsafe_expressions(
|
||||
db: &dyn HirDatabase,
|
||||
infer: &InferenceResult,
|
||||
def: DefWithBodyId,
|
||||
body: &Body,
|
||||
current: ExprId,
|
||||
unsafe_expr_cb: &mut dyn FnMut(UnsafeExpr),
|
||||
unsafe_expr_cb: &mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
|
||||
) {
|
||||
walk_unsafe(
|
||||
db,
|
||||
infer,
|
||||
body,
|
||||
&mut resolver_for_expr(db.upcast(), def, current),
|
||||
def,
|
||||
current,
|
||||
false,
|
||||
unsafe_expr_cb,
|
||||
)
|
||||
let mut visitor = UnsafeVisitor::new(db, infer, body, def, unsafe_expr_cb);
|
||||
_ = visitor.resolver.update_to_inner_scope(db.upcast(), def, current);
|
||||
visitor.walk_expr(current);
|
||||
}
|
||||
|
||||
fn walk_unsafe(
|
||||
db: &dyn HirDatabase,
|
||||
infer: &InferenceResult,
|
||||
body: &Body,
|
||||
resolver: &mut Resolver,
|
||||
struct UnsafeVisitor<'a> {
|
||||
db: &'a dyn HirDatabase,
|
||||
infer: &'a InferenceResult,
|
||||
body: &'a Body,
|
||||
resolver: Resolver,
|
||||
def: DefWithBodyId,
|
||||
current: ExprId,
|
||||
inside_unsafe_block: bool,
|
||||
unsafe_expr_cb: &mut dyn FnMut(UnsafeExpr),
|
||||
) {
|
||||
let mut mark_unsafe_path = |path, node| {
|
||||
let g = resolver.update_to_inner_scope(db.upcast(), def, current);
|
||||
let hygiene = body.expr_or_pat_path_hygiene(node);
|
||||
let value_or_partial = resolver.resolve_path_in_value_ns(db.upcast(), path, hygiene);
|
||||
if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial {
|
||||
let static_data = db.static_data(id);
|
||||
if static_data.mutable || (static_data.is_extern && !static_data.has_safe_kw) {
|
||||
unsafe_expr_cb(UnsafeExpr { node, inside_unsafe_block });
|
||||
}
|
||||
}
|
||||
resolver.reset_to_guard(g);
|
||||
};
|
||||
inside_unsafe_block: InsideUnsafeBlock,
|
||||
inside_assignment: bool,
|
||||
inside_union_destructure: bool,
|
||||
unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
|
||||
}
|
||||
|
||||
let expr = &body.exprs[current];
|
||||
match expr {
|
||||
&Expr::Call { callee, .. } => {
|
||||
if let Some(func) = infer[callee].as_fn_def(db) {
|
||||
if is_fn_unsafe_to_call(db, func) {
|
||||
unsafe_expr_cb(UnsafeExpr { node: current.into(), inside_unsafe_block });
|
||||
}
|
||||
}
|
||||
impl<'a> UnsafeVisitor<'a> {
|
||||
fn new(
|
||||
db: &'a dyn HirDatabase,
|
||||
infer: &'a InferenceResult,
|
||||
body: &'a Body,
|
||||
def: DefWithBodyId,
|
||||
unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason),
|
||||
) -> Self {
|
||||
let resolver = def.resolver(db.upcast());
|
||||
Self {
|
||||
db,
|
||||
infer,
|
||||
body,
|
||||
resolver,
|
||||
def,
|
||||
inside_unsafe_block: InsideUnsafeBlock::No,
|
||||
inside_assignment: false,
|
||||
inside_union_destructure: false,
|
||||
unsafe_expr_cb,
|
||||
}
|
||||
Expr::Path(path) => mark_unsafe_path(path, current.into()),
|
||||
Expr::Ref { expr, rawness: Rawness::RawPtr, mutability: _ } => {
|
||||
if let Expr::Path(_) = body.exprs[*expr] {
|
||||
// Do not report unsafe for `addr_of[_mut]!(EXTERN_OR_MUT_STATIC)`,
|
||||
// see https://github.com/rust-lang/rust/pull/125834.
|
||||
return;
|
||||
}
|
||||
}
|
||||
Expr::MethodCall { .. } => {
|
||||
if infer
|
||||
.method_resolution(current)
|
||||
.map(|(func, _)| is_fn_unsafe_to_call(db, func))
|
||||
.unwrap_or(false)
|
||||
{
|
||||
unsafe_expr_cb(UnsafeExpr { node: current.into(), inside_unsafe_block });
|
||||
}
|
||||
}
|
||||
Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
|
||||
if let TyKind::Raw(..) = &infer[*expr].kind(Interner) {
|
||||
unsafe_expr_cb(UnsafeExpr { node: current.into(), inside_unsafe_block });
|
||||
}
|
||||
}
|
||||
Expr::Unsafe { .. } => {
|
||||
return body.walk_child_exprs(current, |child| {
|
||||
walk_unsafe(db, infer, body, resolver, def, child, true, unsafe_expr_cb);
|
||||
});
|
||||
}
|
||||
&Expr::Assignment { target, value: _ } => {
|
||||
body.walk_pats(target, &mut |pat| {
|
||||
if let Pat::Path(path) = &body[pat] {
|
||||
mark_unsafe_path(path, pat.into());
|
||||
}
|
||||
});
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
body.walk_child_exprs(current, |child| {
|
||||
walk_unsafe(db, infer, body, resolver, def, child, inside_unsafe_block, unsafe_expr_cb);
|
||||
});
|
||||
fn call_cb(&mut self, node: ExprOrPatId, reason: UnsafetyReason) {
|
||||
(self.unsafe_expr_cb)(node, self.inside_unsafe_block, reason);
|
||||
}
|
||||
|
||||
fn walk_pats_top(&mut self, pats: impl Iterator<Item = PatId>, parent_expr: ExprId) {
|
||||
let guard = self.resolver.update_to_inner_scope(self.db.upcast(), self.def, parent_expr);
|
||||
pats.for_each(|pat| self.walk_pat(pat));
|
||||
self.resolver.reset_to_guard(guard);
|
||||
}
|
||||
|
||||
fn walk_pat(&mut self, current: PatId) {
|
||||
let pat = &self.body.pats[current];
|
||||
|
||||
if self.inside_union_destructure {
|
||||
match pat {
|
||||
Pat::Tuple { .. }
|
||||
| Pat::Record { .. }
|
||||
| Pat::Range { .. }
|
||||
| Pat::Slice { .. }
|
||||
| Pat::Path(..)
|
||||
| Pat::Lit(..)
|
||||
| Pat::Bind { .. }
|
||||
| Pat::TupleStruct { .. }
|
||||
| Pat::Ref { .. }
|
||||
| Pat::Box { .. }
|
||||
| Pat::Expr(..)
|
||||
| Pat::ConstBlock(..) => self.call_cb(current.into(), UnsafetyReason::UnionField),
|
||||
// `Or` only wraps other patterns, and `Missing`/`Wild` do not constitute a read.
|
||||
Pat::Missing | Pat::Wild | Pat::Or(_) => {}
|
||||
}
|
||||
}
|
||||
|
||||
match pat {
|
||||
Pat::Record { .. } => {
|
||||
if let Some((AdtId::UnionId(_), _)) = self.infer[current].as_adt() {
|
||||
let old_inside_union_destructure =
|
||||
mem::replace(&mut self.inside_union_destructure, true);
|
||||
self.body.walk_pats_shallow(current, |pat| self.walk_pat(pat));
|
||||
self.inside_union_destructure = old_inside_union_destructure;
|
||||
return;
|
||||
}
|
||||
}
|
||||
Pat::Path(path) => self.mark_unsafe_path(current.into(), path),
|
||||
&Pat::ConstBlock(expr) => {
|
||||
let old_inside_assignment = mem::replace(&mut self.inside_assignment, false);
|
||||
self.walk_expr(expr);
|
||||
self.inside_assignment = old_inside_assignment;
|
||||
}
|
||||
&Pat::Expr(expr) => self.walk_expr(expr),
|
||||
_ => {}
|
||||
}
|
||||
|
||||
self.body.walk_pats_shallow(current, |pat| self.walk_pat(pat));
|
||||
}
|
||||
|
||||
fn walk_expr(&mut self, current: ExprId) {
|
||||
let expr = &self.body.exprs[current];
|
||||
let inside_assignment = mem::replace(&mut self.inside_assignment, false);
|
||||
match expr {
|
||||
&Expr::Call { callee, .. } => {
|
||||
if let Some(func) = self.infer[callee].as_fn_def(self.db) {
|
||||
if is_fn_unsafe_to_call(self.db, func) {
|
||||
self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall);
|
||||
}
|
||||
}
|
||||
}
|
||||
Expr::Path(path) => {
|
||||
let guard =
|
||||
self.resolver.update_to_inner_scope(self.db.upcast(), self.def, current);
|
||||
self.mark_unsafe_path(current.into(), path);
|
||||
self.resolver.reset_to_guard(guard);
|
||||
}
|
||||
Expr::Ref { expr, rawness: Rawness::RawPtr, mutability: _ } => {
|
||||
if let Expr::Path(_) = self.body.exprs[*expr] {
|
||||
// Do not report unsafe for `addr_of[_mut]!(EXTERN_OR_MUT_STATIC)`,
|
||||
// see https://github.com/rust-lang/rust/pull/125834.
|
||||
return;
|
||||
}
|
||||
}
|
||||
Expr::MethodCall { .. } => {
|
||||
if self
|
||||
.infer
|
||||
.method_resolution(current)
|
||||
.map(|(func, _)| is_fn_unsafe_to_call(self.db, func))
|
||||
.unwrap_or(false)
|
||||
{
|
||||
self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall);
|
||||
}
|
||||
}
|
||||
Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
|
||||
if let TyKind::Raw(..) = &self.infer[*expr].kind(Interner) {
|
||||
self.call_cb(current.into(), UnsafetyReason::RawPtrDeref);
|
||||
}
|
||||
}
|
||||
Expr::Unsafe { .. } => {
|
||||
let old_inside_unsafe_block =
|
||||
mem::replace(&mut self.inside_unsafe_block, InsideUnsafeBlock::Yes);
|
||||
self.body.walk_child_exprs_without_pats(current, |child| self.walk_expr(child));
|
||||
self.inside_unsafe_block = old_inside_unsafe_block;
|
||||
return;
|
||||
}
|
||||
&Expr::Assignment { target, value: _ } => {
|
||||
let old_inside_assignment = mem::replace(&mut self.inside_assignment, true);
|
||||
self.walk_pats_top(std::iter::once(target), current);
|
||||
self.inside_assignment = old_inside_assignment;
|
||||
}
|
||||
Expr::InlineAsm(_) => self.call_cb(current.into(), UnsafetyReason::InlineAsm),
|
||||
// rustc allows union assignment to propagate through field accesses and casts.
|
||||
Expr::Cast { .. } => self.inside_assignment = inside_assignment,
|
||||
Expr::Field { .. } => {
|
||||
self.inside_assignment = inside_assignment;
|
||||
if !inside_assignment {
|
||||
if let Some(Either::Left(FieldId { parent: VariantId::UnionId(_), .. })) =
|
||||
self.infer.field_resolution(current)
|
||||
{
|
||||
self.call_cb(current.into(), UnsafetyReason::UnionField);
|
||||
}
|
||||
}
|
||||
}
|
||||
Expr::Block { statements, .. } | Expr::Async { statements, .. } => {
|
||||
self.walk_pats_top(
|
||||
statements.iter().filter_map(|statement| match statement {
|
||||
&Statement::Let { pat, .. } => Some(pat),
|
||||
_ => None,
|
||||
}),
|
||||
current,
|
||||
);
|
||||
}
|
||||
Expr::Match { arms, .. } => {
|
||||
self.walk_pats_top(arms.iter().map(|arm| arm.pat), current);
|
||||
}
|
||||
&Expr::Let { pat, .. } => {
|
||||
self.walk_pats_top(std::iter::once(pat), current);
|
||||
}
|
||||
Expr::Closure { args, .. } => {
|
||||
self.walk_pats_top(args.iter().copied(), current);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
self.body.walk_child_exprs_without_pats(current, |child| self.walk_expr(child));
|
||||
}
|
||||
|
||||
fn mark_unsafe_path(&mut self, node: ExprOrPatId, path: &Path) {
|
||||
let hygiene = self.body.expr_or_pat_path_hygiene(node);
|
||||
let value_or_partial =
|
||||
self.resolver.resolve_path_in_value_ns(self.db.upcast(), path, hygiene);
|
||||
if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial {
|
||||
let static_data = self.db.static_data(id);
|
||||
if static_data.mutable {
|
||||
self.call_cb(node, UnsafetyReason::MutableStatic);
|
||||
} else if static_data.is_extern && !static_data.has_safe_kw {
|
||||
self.call_cb(node, UnsafetyReason::ExternStatic);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,9 @@
|
|||
//! be expressed in terms of hir types themselves.
|
||||
pub use hir_ty::diagnostics::{CaseType, IncorrectCase};
|
||||
use hir_ty::{
|
||||
db::HirDatabase, diagnostics::BodyValidationDiagnostic, CastError, InferenceDiagnostic,
|
||||
db::HirDatabase,
|
||||
diagnostics::{BodyValidationDiagnostic, UnsafetyReason},
|
||||
CastError, InferenceDiagnostic,
|
||||
};
|
||||
|
||||
use cfg::{CfgExpr, CfgOptions};
|
||||
|
|
@ -258,9 +260,10 @@ pub struct PrivateField {
|
|||
|
||||
#[derive(Debug)]
|
||||
pub struct MissingUnsafe {
|
||||
pub expr: InFile<AstPtr<Either<ast::Expr, ast::Pat>>>,
|
||||
pub node: InFile<AstPtr<Either<ast::Expr, ast::Pat>>>,
|
||||
/// If true, the diagnostics is an `unsafe_op_in_unsafe_fn` lint instead of a hard error.
|
||||
pub only_lint: bool,
|
||||
pub reason: UnsafetyReason,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
|
|
|||
|
|
@ -147,6 +147,7 @@ pub use {
|
|||
},
|
||||
hir_ty::{
|
||||
consteval::ConstEvalError,
|
||||
diagnostics::UnsafetyReason,
|
||||
display::{ClosureStyle, HirDisplay, HirDisplayError, HirWrite},
|
||||
dyn_compatibility::{DynCompatibilityViolation, MethodViolationCode},
|
||||
layout::LayoutError,
|
||||
|
|
@ -1890,10 +1891,10 @@ impl DefWithBody {
|
|||
);
|
||||
}
|
||||
|
||||
let (unafe_exprs, only_lint) = hir_ty::diagnostics::missing_unsafe(db, self.into());
|
||||
for expr in unafe_exprs {
|
||||
match source_map.expr_or_pat_syntax(expr) {
|
||||
Ok(expr) => acc.push(MissingUnsafe { expr, only_lint }.into()),
|
||||
let (unsafe_exprs, only_lint) = hir_ty::diagnostics::missing_unsafe(db, self.into());
|
||||
for (node, reason) in unsafe_exprs {
|
||||
match source_map.expr_or_pat_syntax(node) {
|
||||
Ok(node) => acc.push(MissingUnsafe { node, only_lint, reason }.into()),
|
||||
Err(SyntheticSyntax) => {
|
||||
// FIXME: Here and elsewhere in this file, the `expr` was
|
||||
// desugared, report or assert that this doesn't happen.
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ use hir_expand::{
|
|||
use hir_ty::{
|
||||
diagnostics::{
|
||||
record_literal_missing_fields, record_pattern_missing_fields, unsafe_expressions,
|
||||
UnsafeExpr,
|
||||
InsideUnsafeBlock,
|
||||
},
|
||||
lang_items::lang_items_for_bin_op,
|
||||
method_resolution, Adjustment, InferenceResult, Interner, Substitution, Ty, TyExt, TyKind,
|
||||
|
|
@ -939,8 +939,8 @@ impl SourceAnalyzer {
|
|||
*def,
|
||||
body,
|
||||
expr_id,
|
||||
&mut |UnsafeExpr { inside_unsafe_block, .. }| {
|
||||
is_unsafe |= !inside_unsafe_block
|
||||
&mut |_, inside_unsafe_block, _| {
|
||||
is_unsafe |= inside_unsafe_block == InsideUnsafeBlock::No
|
||||
},
|
||||
)
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
use hir::db::ExpandDatabase;
|
||||
use hir::HirFileIdExt;
|
||||
use hir::{HirFileIdExt, UnsafetyReason};
|
||||
use ide_db::text_edit::TextEdit;
|
||||
use ide_db::{assists::Assist, source_change::SourceChange};
|
||||
use syntax::{ast, SyntaxNode};
|
||||
|
|
@ -16,23 +16,35 @@ pub(crate) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsaf
|
|||
} else {
|
||||
DiagnosticCode::RustcHardError("E0133")
|
||||
};
|
||||
let operation = display_unsafety_reason(d.reason);
|
||||
Diagnostic::new_with_syntax_node_ptr(
|
||||
ctx,
|
||||
code,
|
||||
"this operation is unsafe and requires an unsafe function or block",
|
||||
d.expr.map(|it| it.into()),
|
||||
format!("{operation} is unsafe and requires an unsafe function or block"),
|
||||
d.node.map(|it| it.into()),
|
||||
)
|
||||
.with_fixes(fixes(ctx, d))
|
||||
}
|
||||
|
||||
fn display_unsafety_reason(reason: UnsafetyReason) -> &'static str {
|
||||
match reason {
|
||||
UnsafetyReason::UnionField => "access to union field",
|
||||
UnsafetyReason::UnsafeFnCall => "call to unsafe function",
|
||||
UnsafetyReason::InlineAsm => "use of inline assembly",
|
||||
UnsafetyReason::RawPtrDeref => "dereference of raw pointer",
|
||||
UnsafetyReason::MutableStatic => "use of mutable static",
|
||||
UnsafetyReason::ExternStatic => "use of extern static",
|
||||
}
|
||||
}
|
||||
|
||||
fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Option<Vec<Assist>> {
|
||||
// The fixit will not work correctly for macro expansions, so we don't offer it in that case.
|
||||
if d.expr.file_id.is_macro() {
|
||||
if d.node.file_id.is_macro() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let root = ctx.sema.db.parse_or_expand(d.expr.file_id);
|
||||
let node = d.expr.value.to_node(&root);
|
||||
let root = ctx.sema.db.parse_or_expand(d.node.file_id);
|
||||
let node = d.node.value.to_node(&root);
|
||||
let expr = node.syntax().ancestors().find_map(ast::Expr::cast)?;
|
||||
|
||||
let node_to_add_unsafe_block = pick_best_node_to_add_unsafe_block(&expr)?;
|
||||
|
|
@ -40,7 +52,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Option<Vec<Ass
|
|||
let replacement = format!("unsafe {{ {} }}", node_to_add_unsafe_block.text());
|
||||
let edit = TextEdit::replace(node_to_add_unsafe_block.text_range(), replacement);
|
||||
let source_change =
|
||||
SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
|
||||
SourceChange::from_text_edit(d.node.file_id.original_file(ctx.sema.db), edit);
|
||||
Some(vec![fix("add_unsafe", "Add unsafe block", source_change, expr.syntax().text_range())])
|
||||
}
|
||||
|
||||
|
|
@ -110,7 +122,7 @@ fn main() {
|
|||
let x = &5_usize as *const usize;
|
||||
unsafe { let _y = *x; }
|
||||
let _z = *x;
|
||||
} //^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
} //^^💡 error: dereference of raw pointer is unsafe and requires an unsafe function or block
|
||||
"#,
|
||||
)
|
||||
}
|
||||
|
|
@ -136,9 +148,9 @@ unsafe fn unsafe_fn() {
|
|||
|
||||
fn main() {
|
||||
unsafe_fn();
|
||||
//^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block
|
||||
HasUnsafe.unsafe_fn();
|
||||
//^^^^^^^^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^^^^^^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block
|
||||
unsafe {
|
||||
unsafe_fn();
|
||||
HasUnsafe.unsafe_fn();
|
||||
|
|
@ -162,7 +174,7 @@ static mut STATIC_MUT: Ty = Ty { a: 0 };
|
|||
|
||||
fn main() {
|
||||
let _x = STATIC_MUT.a;
|
||||
//^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^^^^^💡 error: use of mutable static is unsafe and requires an unsafe function or block
|
||||
unsafe {
|
||||
let _x = STATIC_MUT.a;
|
||||
}
|
||||
|
|
@ -184,9 +196,9 @@ extern "C" {
|
|||
|
||||
fn main() {
|
||||
let _x = EXTERN;
|
||||
//^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^💡 error: use of extern static is unsafe and requires an unsafe function or block
|
||||
let _x = EXTERN_MUT;
|
||||
//^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^^^^^💡 error: use of mutable static is unsafe and requires an unsafe function or block
|
||||
unsafe {
|
||||
let _x = EXTERN;
|
||||
let _x = EXTERN_MUT;
|
||||
|
|
@ -234,7 +246,7 @@ extern "rust-intrinsic" {
|
|||
fn main() {
|
||||
let _ = bitreverse(12);
|
||||
let _ = floorf32(12.0);
|
||||
//^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
);
|
||||
|
|
@ -567,7 +579,7 @@ unsafe fn not_safe() -> u8 {
|
|||
fn main() {
|
||||
ed2021::safe();
|
||||
ed2024::not_safe();
|
||||
//^^^^^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^^^^^^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
)
|
||||
|
|
@ -591,7 +603,7 @@ unsafe fn foo(p: *mut i32) {
|
|||
#![warn(unsafe_op_in_unsafe_fn)]
|
||||
unsafe fn foo(p: *mut i32) {
|
||||
*p = 123;
|
||||
//^^💡 warn: this operation is unsafe and requires an unsafe function or block
|
||||
//^^💡 warn: dereference of raw pointer is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
)
|
||||
|
|
@ -618,17 +630,119 @@ unsafe extern {
|
|||
fn main() {
|
||||
f();
|
||||
g();
|
||||
//^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block
|
||||
h();
|
||||
//^^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block
|
||||
|
||||
let _ = S1;
|
||||
let _ = S2;
|
||||
//^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^💡 error: use of extern static is unsafe and requires an unsafe function or block
|
||||
let _ = S3;
|
||||
//^^💡 error: this operation is unsafe and requires an unsafe function or block
|
||||
//^^💡 error: use of extern static is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_unsafe_diagnostic_when_destructuring_union_with_wildcard() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
union Union { field: i32 }
|
||||
fn foo(v: &Union) {
|
||||
let Union { field: _ } = v;
|
||||
let Union { field: _ | _ } = v;
|
||||
Union { field: _ } = *v;
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn union_destructuring() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
union Union { field: u8 }
|
||||
fn foo(v @ Union { field: _field }: &Union) {
|
||||
// ^^^^^^ error: access to union field is unsafe and requires an unsafe function or block
|
||||
let Union { mut field } = v;
|
||||
// ^^^^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
let Union { field: 0..=255 } = v;
|
||||
// ^^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
let Union { field: 0
|
||||
// ^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
| 1..=255 } = v;
|
||||
// ^^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
Union { field } = *v;
|
||||
// ^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
match v {
|
||||
Union { field: _field } => {}
|
||||
// ^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
}
|
||||
if let Union { field: _field } = v {}
|
||||
// ^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
(|&Union { field }| { _ = field; })(v);
|
||||
// ^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn union_field_access() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
union Union { field: u8 }
|
||||
fn foo(v: &Union) {
|
||||
v.field;
|
||||
// ^^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn inline_asm() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
//- minicore: asm
|
||||
fn foo() {
|
||||
core::arch::asm!("");
|
||||
// ^^^^ error: use of inline assembly is unsafe and requires an unsafe function or block
|
||||
}
|
||||
"#,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsafe_op_in_unsafe_fn_dismissed_in_signature() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
#![warn(unsafe_op_in_unsafe_fn)]
|
||||
union Union { field: u32 }
|
||||
unsafe fn foo(Union { field: _field }: Union) {}
|
||||
"#,
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn union_assignment_allowed() {
|
||||
check_diagnostics(
|
||||
r#"
|
||||
union Union { field: u32 }
|
||||
fn foo(mut v: Union) {
|
||||
v.field = 123;
|
||||
(v.field,) = (123,);
|
||||
*&mut v.field = 123;
|
||||
// ^^^^^^^💡 error: access to union field is unsafe and requires an unsafe function or block
|
||||
}
|
||||
struct Struct { field: u32 }
|
||||
union Union2 { field: Struct }
|
||||
fn bar(mut v: Union2) {
|
||||
v.field.field = 123;
|
||||
}
|
||||
|
||||
"#,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue