diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9c1e1f670..0eec18a91 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -140,6 +140,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::double_parens::DOUBLE_PARENS_INFO, crate::drop_forget_ref::DROP_NON_DROP_INFO, crate::drop_forget_ref::FORGET_NON_DROP_INFO, + crate::drop_forget_ref::MEM_FORGET_INFO, crate::drop_forget_ref::UNDROPPED_MANUALLY_DROPS_INFO, crate::duplicate_mod::DUPLICATE_MOD_INFO, crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO, @@ -310,7 +311,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::matches::TRY_ERR_INFO, crate::matches::WILDCARD_ENUM_MATCH_ARM_INFO, crate::matches::WILDCARD_IN_OR_PATTERNS_INFO, - crate::mem_forget::MEM_FORGET_INFO, crate::mem_replace::MEM_REPLACE_OPTION_WITH_NONE_INFO, crate::mem_replace::MEM_REPLACE_WITH_DEFAULT_INFO, crate::mem_replace::MEM_REPLACE_WITH_UNINIT_INFO, diff --git a/clippy_lints/src/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index 9c60edb17..6f8261ed8 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -6,6 +6,7 @@ use rustc_hir::{Arm, Expr, ExprKind, LangItem, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; +use std::borrow::Cow; declare_clippy_lint! { /// ### What it does @@ -76,6 +77,27 @@ declare_clippy_lint! { "use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `std::mem::forget(t)` where `t` is + /// `Drop` or has a field that implements `Drop`. + /// + /// ### Why is this bad? + /// `std::mem::forget(t)` prevents `t` from running its + /// destructor, possibly causing leaks. + /// + /// ### Example + /// ```rust + /// # use std::mem; + /// # use std::rc::Rc; + /// mem::forget(Rc::new(55)) + /// ``` + #[clippy::version = "pre 1.29.0"] + pub MEM_FORGET, + restriction, + "`mem::forget` usage on `Drop` types, likely to cause memory leaks" +} + const DROP_NON_DROP_SUMMARY: &str = "call to `std::mem::drop` with a value that does not implement `Drop`. \ Dropping such a type only extends its contained lifetimes"; const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \ @@ -84,7 +106,8 @@ const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value t declare_lint_pass!(DropForgetRef => [ DROP_NON_DROP, FORGET_NON_DROP, - UNDROPPED_MANUALLY_DROPS + UNDROPPED_MANUALLY_DROPS, + MEM_FORGET, ]); impl<'tcx> LateLintPass<'tcx> for DropForgetRef { @@ -97,7 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef { let arg_ty = cx.typeck_results().expr_ty(arg); let is_copy = is_copy(cx, arg_ty); let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr); - let (lint, msg) = match fn_name { + let (lint, msg, note_span) = match fn_name { // early return for uplifted lints: dropping_references, dropping_copy_types, forgetting_references, forgetting_copy_types sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => return, sym::mem_forget if arg_ty.is_ref() => return, @@ -121,19 +144,34 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef { || drop_is_single_call_in_arm ) => { - (DROP_NON_DROP, DROP_NON_DROP_SUMMARY) - }, - sym::mem_forget if !arg_ty.needs_drop(cx.tcx, cx.param_env) => { - (FORGET_NON_DROP, FORGET_NON_DROP_SUMMARY) + (DROP_NON_DROP, DROP_NON_DROP_SUMMARY.into(), Some(arg.span)) }, + sym::mem_forget => { + if arg_ty.needs_drop(cx.tcx, cx.param_env) { + ( + MEM_FORGET, + Cow::Owned(format!( + "usage of `mem::forget` on {}", + if arg_ty.ty_adt_def().map_or(false, |def| def.has_dtor(cx.tcx)) { + "`Drop` type" + } else { + "type with `Drop` fields" + } + )), + None, + ) + } else { + (FORGET_NON_DROP, FORGET_NON_DROP_SUMMARY.into(), Some(arg.span)) + } + } _ => return, }; span_lint_and_note( cx, lint, expr.span, - msg, - Some(arg.span), + &msg, + note_span, &format!("argument has type `{arg_ty}`"), ); } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0008f8f6d..6d77b828f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -196,7 +196,6 @@ mod manual_strip; mod map_unit_fn; mod match_result_ok; mod matches; -mod mem_forget; mod mem_replace; mod methods; mod min_ident_chars; @@ -744,7 +743,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let missing_docs_in_crate_items = conf.missing_docs_in_crate_items; store.register_late_pass(move |_| Box::new(doc::DocMarkdown::new(doc_valid_idents.clone()))); store.register_late_pass(|_| Box::new(neg_multiply::NegMultiply)); - store.register_late_pass(|_| Box::new(mem_forget::MemForget)); store.register_late_pass(|_| Box::new(let_if_seq::LetIfSeq)); store.register_late_pass(|_| Box::new(mixed_read_write_in_expression::EvalOrderDependence)); store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(missing_docs_in_crate_items))); diff --git a/clippy_lints/src/mem_forget.rs b/clippy_lints/src/mem_forget.rs deleted file mode 100644 index d6c235b5a..000000000 --- a/clippy_lints/src/mem_forget.rs +++ /dev/null @@ -1,46 +0,0 @@ -use clippy_utils::diagnostics::span_lint; -use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for usage of `std::mem::forget(t)` where `t` is - /// `Drop`. - /// - /// ### Why is this bad? - /// `std::mem::forget(t)` prevents `t` from running its - /// destructor, possibly causing leaks. - /// - /// ### Example - /// ```rust - /// # use std::mem; - /// # use std::rc::Rc; - /// mem::forget(Rc::new(55)) - /// ``` - #[clippy::version = "pre 1.29.0"] - pub MEM_FORGET, - restriction, - "`mem::forget` usage on `Drop` types, likely to cause memory leaks" -} - -declare_lint_pass!(MemForget => [MEM_FORGET]); - -impl<'tcx> LateLintPass<'tcx> for MemForget { - fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - if let ExprKind::Call(path_expr, [ref first_arg, ..]) = e.kind { - if let ExprKind::Path(ref qpath) = path_expr.kind { - if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() { - if cx.tcx.is_diagnostic_item(sym::mem_forget, def_id) { - let forgot_ty = cx.typeck_results().expr_ty(first_arg); - - if forgot_ty.ty_adt_def().map_or(false, |def| def.has_dtor(cx.tcx)) { - span_lint(cx, MEM_FORGET, e.span, "usage of `mem::forget` on `Drop` type"); - } - } - } - } - } - } -} diff --git a/tests/ui/mem_forget.rs b/tests/ui/mem_forget.rs index edb9d87d0..b6c8d9e53 100644 --- a/tests/ui/mem_forget.rs +++ b/tests/ui/mem_forget.rs @@ -19,5 +19,8 @@ fn main() { let eight: Vec = vec![8]; forgetSomething(eight); + let string = String::new(); + std::mem::forget(string); + std::mem::forget(7); } diff --git a/tests/ui/mem_forget.stderr b/tests/ui/mem_forget.stderr index a90d8b165..8004b2aa8 100644 --- a/tests/ui/mem_forget.stderr +++ b/tests/ui/mem_forget.stderr @@ -4,6 +4,7 @@ error: usage of `mem::forget` on `Drop` type LL | memstuff::forget(six); | ^^^^^^^^^^^^^^^^^^^^^ | + = note: argument has type `std::sync::Arc` = note: `-D clippy::mem-forget` implied by `-D warnings` error: usage of `mem::forget` on `Drop` type @@ -11,12 +12,24 @@ error: usage of `mem::forget` on `Drop` type | LL | std::mem::forget(seven); | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: argument has type `std::rc::Rc` error: usage of `mem::forget` on `Drop` type --> $DIR/mem_forget.rs:20:5 | LL | forgetSomething(eight); | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: argument has type `std::vec::Vec` -error: aborting due to 3 previous errors +error: usage of `mem::forget` on type with `Drop` fields + --> $DIR/mem_forget.rs:23:5 + | +LL | std::mem::forget(string); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: argument has type `std::string::String` + +error: aborting due to 4 previous errors