diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index a52a2369572e..c5bbd4edba9e 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -95,10 +95,10 @@ impl FunctionData { .map(Box::new); let rustc_allow_incoherent_impl = attrs.by_key(&sym::rustc_allow_incoherent_impl).exists(); if flags.contains(FnFlags::HAS_UNSAFE_KW) - && !crate_graph[krate].edition.at_least_2024() && attrs.by_key(&sym::rustc_deprecated_safe_2024).exists() { flags.remove(FnFlags::HAS_UNSAFE_KW); + flags.insert(FnFlags::DEPRECATED_SAFE_2024); } if attrs.by_key(&sym::target_feature).exists() { @@ -152,6 +152,10 @@ impl FunctionData { self.flags.contains(FnFlags::HAS_UNSAFE_KW) } + pub fn is_deprecated_safe_2024(&self) -> bool { + self.flags.contains(FnFlags::DEPRECATED_SAFE_2024) + } + pub fn is_safe(&self) -> bool { self.flags.contains(FnFlags::HAS_SAFE_KW) } diff --git a/crates/hir-def/src/expr_store/lower.rs b/crates/hir-def/src/expr_store/lower.rs index 3ce00b78a9e8..811fecf91f49 100644 --- a/crates/hir-def/src/expr_store/lower.rs +++ b/crates/hir-def/src/expr_store/lower.rs @@ -2238,17 +2238,27 @@ impl ExprCollector<'_> { let unsafe_arg_new = self.alloc_expr_desugared(Expr::Path(unsafe_arg_new)); let unsafe_arg_new = self.alloc_expr_desugared(Expr::Call { callee: unsafe_arg_new, args: Box::default() }); - let unsafe_arg_new = self.alloc_expr_desugared(Expr::Unsafe { + let mut unsafe_arg_new = self.alloc_expr_desugared(Expr::Unsafe { id: None, - // We collect the unused expressions here so that we still infer them instead of - // dropping them out of the expression tree - statements: fmt - .orphans - .into_iter() - .map(|expr| Statement::Expr { expr, has_semi: true }) - .collect(), + statements: Box::new([]), tail: Some(unsafe_arg_new), }); + if !fmt.orphans.is_empty() { + unsafe_arg_new = self.alloc_expr_desugared(Expr::Block { + id: None, + // We collect the unused expressions here so that we still infer them instead of + // dropping them out of the expression tree. We cannot store them in the `Unsafe` + // block because then unsafe blocks within them will get a false "unused unsafe" + // diagnostic (rustc has a notion of builtin unsafe blocks, but we don't). + statements: fmt + .orphans + .into_iter() + .map(|expr| Statement::Expr { expr, has_semi: true }) + .collect(), + tail: Some(unsafe_arg_new), + label: None, + }); + } let idx = self.alloc_expr( Expr::Call { diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs index 09fb5f6fd2d3..82e6ff821b76 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -951,6 +951,7 @@ bitflags::bitflags! { /// only very few functions with it. So we only encode its existence here, and lookup /// it if needed. const HAS_TARGET_FEATURE = 1 << 8; + const DEPRECATED_SAFE_2024 = 1 << 9; } } diff --git a/crates/hir-ty/src/diagnostics/unsafe_check.rs b/crates/hir-ty/src/diagnostics/unsafe_check.rs index 03218b4691b8..f6556924297a 100644 --- a/crates/hir-ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir-ty/src/diagnostics/unsafe_check.rs @@ -10,24 +10,26 @@ use hir_def::{ path::Path, resolver::{HasResolver, ResolveValueResult, Resolver, ValueNs}, type_ref::Rawness, - AdtId, DefWithBodyId, FieldId, VariantId, + AdtId, DefWithBodyId, FieldId, FunctionId, VariantId, }; +use span::Edition; use crate::{ db::HirDatabase, utils::is_fn_unsafe_to_call, InferenceResult, Interner, TargetFeatures, TyExt, TyKind, }; -/// 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, UnsafetyReason)>, bool) { +#[derive(Debug, Default)] +pub struct MissingUnsafeResult { + pub unsafe_exprs: Vec<(ExprOrPatId, UnsafetyReason)>, + /// If `fn_is_unsafe` is false, `unsafe_exprs` are hard errors. If true, they're `unsafe_op_in_unsafe_fn`. + pub fn_is_unsafe: bool, + pub deprecated_safe_calls: Vec, +} + +pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> MissingUnsafeResult { let _p = tracing::info_span!("missing_unsafe").entered(); - let mut res = Vec::new(); let is_unsafe = match def { DefWithBodyId::FunctionId(it) => db.function_data(it).is_unsafe(), DefWithBodyId::StaticId(_) @@ -37,11 +39,19 @@ pub fn missing_unsafe( | DefWithBodyId::FieldId(_) => false, }; + let mut res = MissingUnsafeResult { fn_is_unsafe: is_unsafe, ..MissingUnsafeResult::default() }; let body = db.body(def); let infer = db.infer(def); - let mut callback = |node, inside_unsafe_block, reason| { - if inside_unsafe_block == InsideUnsafeBlock::No { - res.push((node, reason)); + let mut callback = |diag| match diag { + UnsafeDiagnostic::UnsafeOperation { node, inside_unsafe_block, reason } => { + if inside_unsafe_block == InsideUnsafeBlock::No { + res.unsafe_exprs.push((node, reason)); + } + } + UnsafeDiagnostic::DeprecatedSafe2024 { node, inside_unsafe_block } => { + if inside_unsafe_block == InsideUnsafeBlock::No { + res.deprecated_safe_calls.push(node) + } } }; let mut visitor = UnsafeVisitor::new(db, &infer, &body, def, &mut callback); @@ -56,7 +66,7 @@ pub fn missing_unsafe( } } - (res, is_unsafe) + res } #[derive(Debug, Clone, Copy)] @@ -75,15 +85,31 @@ pub enum InsideUnsafeBlock { Yes, } +#[derive(Debug)] +enum UnsafeDiagnostic { + UnsafeOperation { + node: ExprOrPatId, + inside_unsafe_block: InsideUnsafeBlock, + reason: UnsafetyReason, + }, + /// A lint. + DeprecatedSafe2024 { node: ExprId, inside_unsafe_block: InsideUnsafeBlock }, +} + pub fn unsafe_expressions( db: &dyn HirDatabase, infer: &InferenceResult, def: DefWithBodyId, body: &Body, current: ExprId, - unsafe_expr_cb: &mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), + callback: &mut dyn FnMut(InsideUnsafeBlock), ) { - let mut visitor = UnsafeVisitor::new(db, infer, body, def, unsafe_expr_cb); + let mut visitor_callback = |diag| { + if let UnsafeDiagnostic::UnsafeOperation { inside_unsafe_block, .. } = diag { + callback(inside_unsafe_block); + } + }; + let mut visitor = UnsafeVisitor::new(db, infer, body, def, &mut visitor_callback); _ = visitor.resolver.update_to_inner_scope(db.upcast(), def, current); visitor.walk_expr(current); } @@ -97,8 +123,10 @@ struct UnsafeVisitor<'a> { inside_unsafe_block: InsideUnsafeBlock, inside_assignment: bool, inside_union_destructure: bool, - unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), + callback: &'a mut dyn FnMut(UnsafeDiagnostic), def_target_features: TargetFeatures, + // FIXME: This needs to be the edition of the span of each call. + edition: Edition, } impl<'a> UnsafeVisitor<'a> { @@ -107,13 +135,14 @@ impl<'a> UnsafeVisitor<'a> { infer: &'a InferenceResult, body: &'a Body, def: DefWithBodyId, - unsafe_expr_cb: &'a mut dyn FnMut(ExprOrPatId, InsideUnsafeBlock, UnsafetyReason), + unsafe_expr_cb: &'a mut dyn FnMut(UnsafeDiagnostic), ) -> Self { let resolver = def.resolver(db.upcast()); let def_target_features = match def { DefWithBodyId::FunctionId(func) => TargetFeatures::from_attrs(&db.attrs(func.into())), _ => TargetFeatures::default(), }; + let edition = db.crate_graph()[resolver.module().krate()].edition; Self { db, infer, @@ -123,13 +152,34 @@ impl<'a> UnsafeVisitor<'a> { inside_unsafe_block: InsideUnsafeBlock::No, inside_assignment: false, inside_union_destructure: false, - unsafe_expr_cb, + callback: unsafe_expr_cb, def_target_features, + edition, } } - fn call_cb(&mut self, node: ExprOrPatId, reason: UnsafetyReason) { - (self.unsafe_expr_cb)(node, self.inside_unsafe_block, reason); + fn on_unsafe_op(&mut self, node: ExprOrPatId, reason: UnsafetyReason) { + (self.callback)(UnsafeDiagnostic::UnsafeOperation { + node, + inside_unsafe_block: self.inside_unsafe_block, + reason, + }); + } + + fn check_call(&mut self, node: ExprId, func: FunctionId) { + let unsafety = is_fn_unsafe_to_call(self.db, func, &self.def_target_features, self.edition); + match unsafety { + crate::utils::Unsafety::Safe => {} + crate::utils::Unsafety::Unsafe => { + self.on_unsafe_op(node.into(), UnsafetyReason::UnsafeFnCall) + } + crate::utils::Unsafety::DeprecatedSafe2024 => { + (self.callback)(UnsafeDiagnostic::DeprecatedSafe2024 { + node, + inside_unsafe_block: self.inside_unsafe_block, + }) + } + } } fn walk_pats_top(&mut self, pats: impl Iterator, parent_expr: ExprId) { @@ -154,7 +204,9 @@ impl<'a> UnsafeVisitor<'a> { | Pat::Ref { .. } | Pat::Box { .. } | Pat::Expr(..) - | Pat::ConstBlock(..) => self.call_cb(current.into(), UnsafetyReason::UnionField), + | Pat::ConstBlock(..) => { + self.on_unsafe_op(current.into(), UnsafetyReason::UnionField) + } // `Or` only wraps other patterns, and `Missing`/`Wild` do not constitute a read. Pat::Missing | Pat::Wild | Pat::Or(_) => {} } @@ -189,9 +241,7 @@ impl<'a> UnsafeVisitor<'a> { 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.def_target_features) { - self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall); - } + self.check_call(current, func); } } Expr::Path(path) => { @@ -217,18 +267,13 @@ impl<'a> UnsafeVisitor<'a> { } } Expr::MethodCall { .. } => { - if self - .infer - .method_resolution(current) - .map(|(func, _)| is_fn_unsafe_to_call(self.db, func, &self.def_target_features)) - .unwrap_or(false) - { - self.call_cb(current.into(), UnsafetyReason::UnsafeFnCall); + if let Some((func, _)) = self.infer.method_resolution(current) { + self.check_call(current, func); } } Expr::UnaryOp { expr, op: UnaryOp::Deref } => { if let TyKind::Raw(..) = &self.infer[*expr].kind(Interner) { - self.call_cb(current.into(), UnsafetyReason::RawPtrDeref); + self.on_unsafe_op(current.into(), UnsafetyReason::RawPtrDeref); } } Expr::Unsafe { .. } => { @@ -243,7 +288,7 @@ impl<'a> UnsafeVisitor<'a> { self.walk_pats_top(std::iter::once(target), current); self.inside_assignment = old_inside_assignment; } - Expr::InlineAsm(_) => self.call_cb(current.into(), UnsafetyReason::InlineAsm), + Expr::InlineAsm(_) => self.on_unsafe_op(current.into(), UnsafetyReason::InlineAsm), // rustc allows union assignment to propagate through field accesses and casts. Expr::Cast { .. } => self.inside_assignment = inside_assignment, Expr::Field { .. } => { @@ -252,7 +297,7 @@ impl<'a> UnsafeVisitor<'a> { if let Some(Either::Left(FieldId { parent: VariantId::UnionId(_), .. })) = self.infer.field_resolution(current) { - self.call_cb(current.into(), UnsafetyReason::UnionField); + self.on_unsafe_op(current.into(), UnsafetyReason::UnionField); } } } @@ -287,9 +332,9 @@ impl<'a> UnsafeVisitor<'a> { 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); + self.on_unsafe_op(node, UnsafetyReason::MutableStatic); } else if static_data.is_extern && !static_data.has_safe_kw { - self.call_cb(node, UnsafetyReason::ExternStatic); + self.on_unsafe_op(node, UnsafetyReason::ExternStatic); } } } diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index 37de501e9878..4b159b7541e6 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -100,7 +100,9 @@ pub use mapping::{ }; pub use method_resolution::check_orphan_rules; pub use traits::TraitEnvironment; -pub use utils::{all_super_traits, direct_super_traits, is_fn_unsafe_to_call, TargetFeatures}; +pub use utils::{ + all_super_traits, direct_super_traits, is_fn_unsafe_to_call, TargetFeatures, Unsafety, +}; pub use variance::Variance; pub use chalk_ir::{ diff --git a/crates/hir-ty/src/utils.rs b/crates/hir-ty/src/utils.rs index ffd0b5b5e21a..c131e97bc4cb 100644 --- a/crates/hir-ty/src/utils.rs +++ b/crates/hir-ty/src/utils.rs @@ -24,6 +24,7 @@ use intern::{sym, Symbol}; use rustc_abi::TargetDataLayout; use rustc_hash::FxHashSet; use smallvec::{smallvec, SmallVec}; +use span::Edition; use stdx::never; use crate::{ @@ -292,21 +293,38 @@ impl TargetFeatures { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Unsafety { + Safe, + Unsafe, + /// A lint. + DeprecatedSafe2024, +} + pub fn is_fn_unsafe_to_call( db: &dyn HirDatabase, func: FunctionId, caller_target_features: &TargetFeatures, -) -> bool { + call_edition: Edition, +) -> Unsafety { let data = db.function_data(func); if data.is_unsafe() { - return true; + return Unsafety::Unsafe; } if data.has_target_feature() { // RFC 2396 . let callee_target_features = TargetFeatures::from_attrs(&db.attrs(func.into())); if !caller_target_features.enabled.is_superset(&callee_target_features.enabled) { - return true; + return Unsafety::Unsafe; + } + } + + if data.is_deprecated_safe_2024() { + if call_edition.at_least_2024() { + return Unsafety::Unsafe; + } else { + return Unsafety::DeprecatedSafe2024; } } @@ -319,14 +337,22 @@ pub fn is_fn_unsafe_to_call( if is_intrinsic_block { // legacy intrinsics // extern "rust-intrinsic" intrinsics are unsafe unless they have the rustc_safe_intrinsic attribute - !db.attrs(func.into()).by_key(&sym::rustc_safe_intrinsic).exists() + if db.attrs(func.into()).by_key(&sym::rustc_safe_intrinsic).exists() { + Unsafety::Safe + } else { + Unsafety::Unsafe + } } else { // Function in an `extern` block are always unsafe to call, except when // it is marked as `safe`. - !data.is_safe() + if data.is_safe() { + Unsafety::Safe + } else { + Unsafety::Unsafe + } } } - _ => false, + _ => Unsafety::Safe, } } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 066a322e32ba..64e982c42d7f 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -271,11 +271,17 @@ pub struct PrivateField { pub field: Field, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UnsafeLint { + HardError, + UnsafeOpInUnsafeFn, + DeprecatedSafe2024, +} + #[derive(Debug)] pub struct MissingUnsafe { pub node: InFile>>, - /// If true, the diagnostics is an `unsafe_op_in_unsafe_fn` lint instead of a hard error. - pub only_lint: bool, + pub lint: UnsafeLint, pub reason: UnsafetyReason, } diff --git a/crates/hir/src/display.rs b/crates/hir/src/display.rs index 6f4049705585..6f4168ab0867 100644 --- a/crates/hir/src/display.rs +++ b/crates/hir/src/display.rs @@ -82,7 +82,7 @@ impl HirDisplay for Function { } // FIXME: This will show `unsafe` for functions that are `#[target_feature]` but not unsafe // (they are conditionally unsafe to call). We probably should show something else. - if self.is_unsafe_to_call(db, None) { + if self.is_unsafe_to_call(db, None, f.edition()) { f.write_str("unsafe ")?; } if let Some(abi) = &data.abi { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 5b35b0168ac3..b414ef0d71d9 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2040,16 +2040,40 @@ impl DefWithBody { ); } - let (unsafe_exprs, only_lint) = hir_ty::diagnostics::missing_unsafe(db, self.into()); - for (node, reason) in unsafe_exprs { + let missing_unsafe = hir_ty::diagnostics::missing_unsafe(db, self.into()); + for (node, reason) in missing_unsafe.unsafe_exprs { match source_map.expr_or_pat_syntax(node) { - Ok(node) => acc.push(MissingUnsafe { node, only_lint, reason }.into()), + Ok(node) => acc.push( + MissingUnsafe { + node, + lint: if missing_unsafe.fn_is_unsafe { + UnsafeLint::UnsafeOpInUnsafeFn + } else { + UnsafeLint::HardError + }, + reason, + } + .into(), + ), Err(SyntheticSyntax) => { // FIXME: Here and elsewhere in this file, the `expr` was // desugared, report or assert that this doesn't happen. } } } + for node in missing_unsafe.deprecated_safe_calls { + match source_map.expr_syntax(node) { + Ok(node) => acc.push( + MissingUnsafe { + node: node.map(|it| it.wrap_left()), + lint: UnsafeLint::DeprecatedSafe2024, + reason: UnsafetyReason::UnsafeFnCall, + } + .into(), + ), + Err(SyntheticSyntax) => never!("synthetic DeprecatedSafe2024"), + } + } if let Ok(borrowck_results) = db.borrowck(self.into()) { for borrowck_result in borrowck_results.iter() { @@ -2425,11 +2449,19 @@ impl Function { db.attrs(self.id.into()).is_unstable() } - pub fn is_unsafe_to_call(self, db: &dyn HirDatabase, caller: Option) -> bool { + pub fn is_unsafe_to_call( + self, + db: &dyn HirDatabase, + caller: Option, + call_edition: Edition, + ) -> bool { let target_features = caller .map(|caller| hir_ty::TargetFeatures::from_attrs(&db.attrs(caller.id.into()))) .unwrap_or_default(); - hir_ty::is_fn_unsafe_to_call(db, self.id, &target_features) + matches!( + hir_ty::is_fn_unsafe_to_call(db, self.id, &target_features, call_edition), + hir_ty::Unsafety::Unsafe + ) } /// Whether this function declaration has a definition. diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 9f230c225151..7d6f11018175 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -1107,16 +1107,9 @@ impl SourceAnalyzer { if let Some(expanded_expr) = sm.macro_expansion_expr(macro_expr) { let mut is_unsafe = false; let mut walk_expr = |expr_id| { - unsafe_expressions( - db, - infer, - *def, - body, - expr_id, - &mut |_, inside_unsafe_block, _| { - is_unsafe |= inside_unsafe_block == InsideUnsafeBlock::No - }, - ) + unsafe_expressions(db, infer, *def, body, expr_id, &mut |inside_unsafe_block| { + is_unsafe |= inside_unsafe_block == InsideUnsafeBlock::No + }) }; match expanded_expr { ExprOrPatId::ExprId(expanded_expr) => walk_expr(expanded_expr), diff --git a/crates/hir/src/term_search/tactics.rs b/crates/hir/src/term_search/tactics.rs index 1ff9b6dec9c8..847304d503a8 100644 --- a/crates/hir/src/term_search/tactics.rs +++ b/crates/hir/src/term_search/tactics.rs @@ -15,6 +15,7 @@ use hir_ty::mir::BorrowKind; use hir_ty::TyBuilder; use itertools::Itertools; use rustc_hash::FxHashSet; +use span::Edition; use crate::{ Adt, AssocItem, GenericDef, GenericParam, HasAttrs, HasVisibility, Impl, ModuleDef, ScopeDef, @@ -365,7 +366,7 @@ pub(super) fn free_function<'a, DB: HirDatabase>( let ret_ty = it.ret_type_with_args(db, generics.iter().cloned()); // Filter out private and unsafe functions if !it.is_visible_from(db, module) - || it.is_unsafe_to_call(db, None) + || it.is_unsafe_to_call(db, None, Edition::CURRENT_FIXME) || it.is_unstable(db) || ctx.config.enable_borrowcheck && ret_ty.contains_reference(db) || ret_ty.is_raw_ptr() @@ -471,7 +472,7 @@ pub(super) fn impl_method<'a, DB: HirDatabase>( // Filter out private and unsafe functions if !it.is_visible_from(db, module) - || it.is_unsafe_to_call(db, None) + || it.is_unsafe_to_call(db, None, Edition::CURRENT_FIXME) || it.is_unstable(db) { return None; @@ -662,7 +663,7 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>( // Filter out private and unsafe functions if !it.is_visible_from(db, module) - || it.is_unsafe_to_call(db, None) + || it.is_unsafe_to_call(db, None, Edition::CURRENT_FIXME) || it.is_unstable(db) { return None; diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index 4931f8d09024..fd90613964af 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/crates/ide-completion/src/render/function.rs @@ -320,7 +320,7 @@ fn detail(ctx: &CompletionContext<'_>, func: hir::Function, edition: Edition) -> ret_ty = async_ret; } } - if func.is_unsafe_to_call(ctx.db, ctx.containing_function) { + if func.is_unsafe_to_call(ctx.db, ctx.containing_function, ctx.edition) { format_to!(detail, "unsafe "); } diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index cea8bc5e9590..d48464769c8d 100644 --- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -1,5 +1,5 @@ use hir::db::ExpandDatabase; -use hir::{HirFileIdExt, UnsafetyReason}; +use hir::{HirFileIdExt, UnsafeLint, UnsafetyReason}; use ide_db::text_edit::TextEdit; use ide_db::{assists::Assist, source_change::SourceChange}; use syntax::{ast, SyntaxNode}; @@ -11,10 +11,10 @@ use crate::{fix, Diagnostic, DiagnosticCode, DiagnosticsContext}; // // This diagnostic is triggered if an operation marked as `unsafe` is used outside of an `unsafe` function or block. pub(crate) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Diagnostic { - let code = if d.only_lint { - DiagnosticCode::RustcLint("unsafe_op_in_unsafe_fn") - } else { - DiagnosticCode::RustcHardError("E0133") + let code = match d.lint { + UnsafeLint::HardError => DiagnosticCode::RustcHardError("E0133"), + UnsafeLint::UnsafeOpInUnsafeFn => DiagnosticCode::RustcLint("unsafe_op_in_unsafe_fn"), + UnsafeLint::DeprecatedSafe2024 => DiagnosticCode::RustcLint("deprecated_safe_2024"), }; let operation = display_unsafety_reason(d.reason); Diagnostic::new_with_syntax_node_ptr( @@ -585,24 +585,58 @@ fn main() { r#" //- /ed2021.rs crate:ed2021 edition:2021 #[rustc_deprecated_safe_2024] -unsafe fn safe() -> u8 { +unsafe fn deprecated_safe() -> u8 { 0 } + //- /ed2024.rs crate:ed2024 edition:2024 #[rustc_deprecated_safe_2024] -unsafe fn not_safe() -> u8 { +unsafe fn deprecated_safe() -> u8 { 0 } -//- /main.rs crate:main deps:ed2021,ed2024 + +//- /dep1.rs crate:dep1 deps:ed2021,ed2024 edition:2021 +fn main() { + ed2021::deprecated_safe(); + ed2024::deprecated_safe(); +} + +//- /dep2.rs crate:dep2 deps:ed2021,ed2024 edition:2024 +fn main() { + ed2021::deprecated_safe(); + // ^^^^^^^^^^^^^^^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block + ed2024::deprecated_safe(); + // ^^^^^^^^^^^^^^^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block +} + +//- /dep3.rs crate:dep3 deps:ed2021,ed2024 edition:2021 +#![warn(deprecated_safe)] + fn main() { - ed2021::safe(); - ed2024::not_safe(); - //^^^^^^^^^^^^^^^^^^💡 error: call to unsafe function is unsafe and requires an unsafe function or block + ed2021::deprecated_safe(); + // ^^^^^^^^^^^^^^^^^^^^^^^^^💡 warn: call to unsafe function is unsafe and requires an unsafe function or block + ed2024::deprecated_safe(); + // ^^^^^^^^^^^^^^^^^^^^^^^^^💡 warn: call to unsafe function is unsafe and requires an unsafe function or block } "#, ) } + #[test] + fn orphan_unsafe_format_args() { + // Checks that we don't place orphan arguments for formatting under an unsafe block. + check_diagnostics( + r#" +//- minicore: fmt +fn foo() { + let p = 0xDEADBEEF as *const i32; + format_args!("", *p); + // ^^ error: dereference of raw pointer is unsafe and requires an unsafe function or block +} + "#, + ); + } + #[test] fn unsafe_op_in_unsafe_fn_allowed_by_default_in_edition_2021() { check_diagnostics( diff --git a/crates/ide/src/syntax_highlighting.rs b/crates/ide/src/syntax_highlighting.rs index f53f0aec0984..95f2bf113950 100644 --- a/crates/ide/src/syntax_highlighting.rs +++ b/crates/ide/src/syntax_highlighting.rs @@ -478,7 +478,15 @@ fn traverse( { continue; } - highlight_format_string(hl, sema, krate, &string, &expanded_string, range); + highlight_format_string( + hl, + sema, + krate, + &string, + &expanded_string, + range, + file_id.edition(), + ); if !string.is_raw() { highlight_escape_string(hl, &string, range.start()); @@ -526,6 +534,7 @@ fn traverse( &mut bindings_shadow_count, config.syntactic_name_ref_highlighting, name_like, + file_id.edition(), ), NodeOrToken::Token(token) => { highlight::token(sema, token, file_id.edition()).zip(Some(None)) diff --git a/crates/ide/src/syntax_highlighting/format.rs b/crates/ide/src/syntax_highlighting/format.rs index 7234108701a3..43a6bdad7e9b 100644 --- a/crates/ide/src/syntax_highlighting/format.rs +++ b/crates/ide/src/syntax_highlighting/format.rs @@ -4,6 +4,7 @@ use ide_db::{ syntax_helpers::format_string::{is_format_string, lex_format_specifiers, FormatSpecifier}, SymbolKind, }; +use span::Edition; use syntax::{ast, TextRange}; use crate::{ @@ -18,6 +19,7 @@ pub(super) fn highlight_format_string( string: &ast::String, expanded_string: &ast::String, range: TextRange, + edition: Edition, ) { if is_format_string(expanded_string) { // FIXME: Replace this with the HIR info we have now. @@ -39,7 +41,7 @@ pub(super) fn highlight_format_string( if let Some(res) = res { stack.add(HlRange { range, - highlight: highlight_def(sema, krate, Definition::from(res)), + highlight: highlight_def(sema, krate, Definition::from(res), edition), binding_hash: None, }) } diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs index a0c8b2856839..842b86d2d006 100644 --- a/crates/ide/src/syntax_highlighting/highlight.rs +++ b/crates/ide/src/syntax_highlighting/highlight.rs @@ -58,6 +58,7 @@ pub(super) fn name_like( bindings_shadow_count: &mut FxHashMap, syntactic_name_ref_highlighting: bool, name_like: ast::NameLike, + edition: Edition, ) -> Option<(Highlight, Option)> { let mut binding_hash = None; let highlight = match name_like { @@ -68,16 +69,17 @@ pub(super) fn name_like( &mut binding_hash, syntactic_name_ref_highlighting, name_ref, + edition, ), ast::NameLike::Name(name) => { - highlight_name(sema, bindings_shadow_count, &mut binding_hash, krate, name) + highlight_name(sema, bindings_shadow_count, &mut binding_hash, krate, name, edition) } ast::NameLike::Lifetime(lifetime) => match IdentClass::classify_lifetime(sema, &lifetime) { Some(IdentClass::NameClass(NameClass::Definition(def))) => { - highlight_def(sema, krate, def) | HlMod::Definition + highlight_def(sema, krate, def, edition) | HlMod::Definition } Some(IdentClass::NameRefClass(NameRefClass::Definition(def, _))) => { - highlight_def(sema, krate, def) + highlight_def(sema, krate, def, edition) } // FIXME: Fallback for 'static and '_, as we do not resolve these yet _ => SymbolKind::LifetimeParam.into(), @@ -234,16 +236,17 @@ fn highlight_name_ref( binding_hash: &mut Option, syntactic_name_ref_highlighting: bool, name_ref: ast::NameRef, + edition: Edition, ) -> Highlight { let db = sema.db; - if let Some(res) = highlight_method_call_by_name_ref(sema, krate, &name_ref) { + if let Some(res) = highlight_method_call_by_name_ref(sema, krate, &name_ref, edition) { return res; } let name_class = match NameRefClass::classify(sema, &name_ref) { Some(name_kind) => name_kind, None if syntactic_name_ref_highlighting => { - return highlight_name_ref_by_syntax(name_ref, sema, krate) + return highlight_name_ref_by_syntax(name_ref, sema, krate, edition) } // FIXME: This is required for helper attributes used by proc-macros, as those do not map down // to anything when used. @@ -267,7 +270,7 @@ fn highlight_name_ref( *binding_hash = Some(calc_binding_hash(&name, *shadow_count)) }; - let mut h = highlight_def(sema, krate, def); + let mut h = highlight_def(sema, krate, def, edition); match def { Definition::Local(local) if is_consumed_lvalue(name_ref.syntax(), &local, db) => { @@ -305,7 +308,7 @@ fn highlight_name_ref( h } NameRefClass::FieldShorthand { field_ref, .. } => { - highlight_def(sema, krate, field_ref.into()) + highlight_def(sema, krate, field_ref.into(), edition) } NameRefClass::ExternCrateShorthand { decl, krate: resolved_krate } => { let mut h = HlTag::Symbol(SymbolKind::Module).into(); @@ -341,6 +344,7 @@ fn highlight_name( binding_hash: &mut Option, krate: hir::Crate, name: ast::Name, + edition: Edition, ) -> Highlight { let name_kind = NameClass::classify(sema, &name); if let Some(NameClass::Definition(Definition::Local(local))) = &name_kind { @@ -351,7 +355,7 @@ fn highlight_name( }; match name_kind { Some(NameClass::Definition(def)) => { - let mut h = highlight_def(sema, krate, def) | HlMod::Definition; + let mut h = highlight_def(sema, krate, def, edition) | HlMod::Definition; if let Definition::Trait(trait_) = &def { if trait_.is_unsafe(sema.db) { h |= HlMod::Unsafe; @@ -359,7 +363,7 @@ fn highlight_name( } h } - Some(NameClass::ConstReference(def)) => highlight_def(sema, krate, def), + Some(NameClass::ConstReference(def)) => highlight_def(sema, krate, def, edition), Some(NameClass::PatFieldShorthand { field_ref, .. }) => { let mut h = HlTag::Symbol(SymbolKind::Field).into(); if let hir::VariantDef::Union(_) = field_ref.parent_def(sema.db) { @@ -379,6 +383,7 @@ pub(super) fn highlight_def( sema: &Semantics<'_, RootDatabase>, krate: hir::Crate, def: Definition, + edition: Edition, ) -> Highlight { let db = sema.db; let mut h = match def { @@ -431,7 +436,8 @@ pub(super) fn highlight_def( // highlighted as unsafe, even when the current target features set is a superset (RFC 2396). // We probably should consider checking the current function, but I found no easy way to do // that (also I'm worried about perf). There's also an instance below. - if func.is_unsafe_to_call(db, None) { + // FIXME: This should be the edition of the call. + if func.is_unsafe_to_call(db, None, edition) { h |= HlMod::Unsafe; } if func.is_async(db) { @@ -579,21 +585,23 @@ fn highlight_method_call_by_name_ref( sema: &Semantics<'_, RootDatabase>, krate: hir::Crate, name_ref: &ast::NameRef, + edition: Edition, ) -> Option { let mc = name_ref.syntax().parent().and_then(ast::MethodCallExpr::cast)?; - highlight_method_call(sema, krate, &mc) + highlight_method_call(sema, krate, &mc, edition) } fn highlight_method_call( sema: &Semantics<'_, RootDatabase>, krate: hir::Crate, method_call: &ast::MethodCallExpr, + edition: Edition, ) -> Option { let func = sema.resolve_method_call(method_call)?; let mut h = SymbolKind::Method.into(); - if func.is_unsafe_to_call(sema.db, None) || sema.is_unsafe_method_call(method_call) { + if func.is_unsafe_to_call(sema.db, None, edition) || sema.is_unsafe_method_call(method_call) { h |= HlMod::Unsafe; } if func.is_async(sema.db) { @@ -679,6 +687,7 @@ fn highlight_name_ref_by_syntax( name: ast::NameRef, sema: &Semantics<'_, RootDatabase>, krate: hir::Crate, + edition: Edition, ) -> Highlight { let default = HlTag::UnresolvedReference; @@ -689,7 +698,7 @@ fn highlight_name_ref_by_syntax( match parent.kind() { METHOD_CALL_EXPR => ast::MethodCallExpr::cast(parent) - .and_then(|it| highlight_method_call(sema, krate, &it)) + .and_then(|it| highlight_method_call(sema, krate, &it, edition)) .unwrap_or_else(|| SymbolKind::Method.into()), FIELD_EXPR => { let h = HlTag::Symbol(SymbolKind::Field);