From 8221f9e7950120c940511ca914c9724cc89b56d5 Mon Sep 17 00:00:00 2001 From: hamidreza kalbasi Date: Mon, 16 Aug 2021 19:34:41 +0430 Subject: [PATCH] add derivable impls lint --- CHANGELOG.md | 1 + clippy_lints/src/derivable_impls.rs | 108 ++++++++++++++++++ clippy_lints/src/lib.rs | 5 + clippy_lints/src/mem_replace.rs | 85 +++++--------- clippy_utils/src/lib.rs | 61 +++++++++- clippy_utils/src/ty.rs | 7 ++ tests/ui/derivable_impls.rs | 170 ++++++++++++++++++++++++++++ tests/ui/derivable_impls.stderr | 77 +++++++++++++ tests/ui/mem_replace.fixed | 20 ++++ tests/ui/mem_replace.rs | 20 ++++ tests/ui/mem_replace.stderr | 20 +++- 11 files changed, 516 insertions(+), 58 deletions(-) create mode 100644 clippy_lints/src/derivable_impls.rs create mode 100644 tests/ui/derivable_impls.rs create mode 100644 tests/ui/derivable_impls.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index d86414a87f4..57df49a2723 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2613,6 +2613,7 @@ Released 2018-09-13 [`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver [`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof +[`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs new file mode 100644 index 00000000000..b4c4ca016aa --- /dev/null +++ b/clippy_lints/src/derivable_impls.rs @@ -0,0 +1,108 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::{in_macro, is_automatically_derived, is_default_equivalent, remove_blocks}; +use rustc_hir::{ + def::{DefKind, Res}, + Body, Expr, ExprKind, Impl, ImplItemKind, Item, ItemKind, Node, QPath, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TypeFoldable; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Detects manual `std::default::Default` implementations that are identical to a derived implementation. + /// + /// ### Why is this bad? + /// It is less concise. + /// + /// ### Example + /// ```rust + /// struct Foo { + /// bar: bool + /// } + /// + /// impl std::default::Default for Foo { + /// fn default() -> Self { + /// Self { + /// bar: false + /// } + /// } + /// } + /// ``` + /// + /// Could be written as: + /// + /// ```rust + /// #[derive(Default)] + /// struct Foo { + /// bar: bool + /// } + /// ``` + /// + /// ### Known problems + /// Derive macros [sometimes use incorrect bounds](https://github.com/rust-lang/rust/issues/26925) + /// in generic types and the user defined `impl` maybe is more generalized or + /// specialized than what derive will produce. This lint can't detect the manual `impl` + /// has exactly equal bounds, and therefore this lint is disabled for types with + /// generic parameters. + /// + pub DERIVABLE_IMPLS, + complexity, + "manual implementation of the `Default` trait which is equal to a derive" +} + +declare_lint_pass!(DerivableImpls => [DERIVABLE_IMPLS]); + +fn is_path_self(e: &Expr<'_>) -> bool { + if let ExprKind::Path(QPath::Resolved(_, p)) = e.kind { + matches!(p.res, Res::SelfCtor(..) | Res::Def(DefKind::Ctor(..), _)) + } else { + false + } +} + +impl<'tcx> LateLintPass<'tcx> for DerivableImpls { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + if_chain! { + if let ItemKind::Impl(Impl { + of_trait: Some(ref trait_ref), + items: [child], + .. + }) = item.kind; + if let attrs = cx.tcx.hir().attrs(item.hir_id()); + if !is_automatically_derived(attrs); + if !in_macro(item.span); + if let Some(def_id) = trait_ref.trait_def_id(); + if cx.tcx.is_diagnostic_item(sym::Default, def_id); + if let impl_item_hir = child.id.hir_id(); + if let Some(Node::ImplItem(impl_item)) = cx.tcx.hir().find(impl_item_hir); + if let ImplItemKind::Fn(_, b) = &impl_item.kind; + if let Body { value: func_expr, .. } = cx.tcx.hir().body(*b); + if let Some(adt_def) = cx.tcx.type_of(item.def_id).ty_adt_def(); + then { + if cx.tcx.type_of(item.def_id).definitely_has_param_types_or_consts(cx.tcx) { + return; + } + let should_emit = match remove_blocks(func_expr).kind { + ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)), + ExprKind::Call(callee, args) + if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)), + ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)), + _ => false, + }; + if should_emit { + let path_string = cx.tcx.def_path_str(adt_def.did); + span_lint_and_help( + cx, + DERIVABLE_IMPLS, + item.span, + "this `impl` can be derived", + None, + &format!("try annotating `{}` with `#[derive(Default)]`", path_string), + ); + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2b76eacb7d6..88c1ee1ba0e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -187,6 +187,7 @@ mod dbg_macro; mod default; mod default_numeric_fallback; mod dereference; +mod derivable_impls; mod derive; mod disallowed_method; mod disallowed_script_idents; @@ -586,6 +587,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: default::FIELD_REASSIGN_WITH_DEFAULT, default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK, dereference::EXPLICIT_DEREF_METHODS, + derivable_impls::DERIVABLE_IMPLS, derive::DERIVE_HASH_XOR_EQ, derive::DERIVE_ORD_XOR_PARTIAL_ORD, derive::EXPL_IMPL_CLONE_ON_COPY, @@ -1204,6 +1206,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(copies::IFS_SAME_COND), LintId::of(copies::IF_SAME_THEN_ELSE), LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT), + LintId::of(derivable_impls::DERIVABLE_IMPLS), LintId::of(derive::DERIVE_HASH_XOR_EQ), LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD), LintId::of(doc::MISSING_SAFETY_DOC), @@ -1589,6 +1592,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(booleans::NONMINIMAL_BOOL), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::UNNECESSARY_CAST), + LintId::of(derivable_impls::DERIVABLE_IMPLS), LintId::of(double_comparison::DOUBLE_COMPARISONS), LintId::of(double_parens::DOUBLE_PARENS), LintId::of(duration_subsec::DURATION_SUBSEC), @@ -1937,6 +1941,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(panic_unimplemented::PanicUnimplemented)); store.register_late_pass(|| Box::new(strings::StringLitAsBytes)); store.register_late_pass(|| Box::new(derive::Derive)); + store.register_late_pass(|| Box::new(derivable_impls::DerivableImpls)); store.register_late_pass(|| Box::new(get_last_with_len::GetLastWithLen)); store.register_late_pass(|| Box::new(drop_forget_ref::DropForgetRef)); store.register_late_pass(|| Box::new(empty_enum::EmptyEnum)); diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index 3d071c9081b..1e6057a8fe9 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, snippet_with_applicability}; -use clippy_utils::{in_macro, is_diag_trait_item, is_lang_ctor, match_def_path, meets_msrv, msrvs, paths}; +use clippy_utils::ty::is_non_aggregate_primitive_type; +use clippy_utils::{in_macro, is_default_equivalent, is_lang_ctor, match_def_path, meets_msrv, msrvs, paths}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::def_id::DefId; use rustc_hir::LangItem::OptionNone; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, QPath}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -194,64 +194,37 @@ fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<' } } -/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent" -/// constructor from the std library -fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool { - let std_types_symbols = &[ - sym::string_type, - sym::vec_type, - sym::vecdeque_type, - sym::LinkedList, - sym::hashmap_type, - sym::BTreeMap, - sym::hashset_type, - sym::BTreeSet, - sym::BinaryHeap, - ]; - - if let QPath::TypeRelative(_, method) = path { - if method.ident.name == sym::new { - if let Some(impl_did) = cx.tcx.impl_of_method(def_id) { - if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() { - return std_types_symbols - .iter() - .any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did)); - } - } +fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) { + // disable lint for primitives + let expr_type = cx.typeck_results().expr_ty_adjusted(src); + if is_non_aggregate_primitive_type(expr_type) { + return; + } + // disable lint for Option since it is covered in another lint + if let ExprKind::Path(q) = &src.kind { + if is_lang_ctor(cx, q, OptionNone) { + return; } } - false -} + if is_default_equivalent(cx, src) && !in_external_macro(cx.tcx.sess, expr_span) { + span_lint_and_then( + cx, + MEM_REPLACE_WITH_DEFAULT, + expr_span, + "replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`", + |diag| { + if !in_macro(expr_span) { + let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, "")); -fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) { - if_chain! { - if let ExprKind::Call(repl_func, _) = src.kind; - if !in_external_macro(cx.tcx.sess, expr_span); - if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; - if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); - if is_diag_trait_item(cx, repl_def_id, sym::Default) - || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath); - - then { - span_lint_and_then( - cx, - MEM_REPLACE_WITH_DEFAULT, - expr_span, - "replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`", - |diag| { - if !in_macro(expr_span) { - let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, "")); - - diag.span_suggestion( - expr_span, - "consider using", - suggestion, - Applicability::MachineApplicable - ); - } + diag.span_suggestion( + expr_span, + "consider using", + suggestion, + Applicability::MachineApplicable, + ); } - ); - } + }, + ); } } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 40d7963a5e5..757485d19d2 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -70,7 +70,7 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor}; -use rustc_hir::LangItem::{ResultErr, ResultOk}; +use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; use rustc_hir::{ def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat, @@ -644,6 +644,65 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) - false } +/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent" +/// constructor from the std library +fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool { + let std_types_symbols = &[ + sym::string_type, + sym::vec_type, + sym::vecdeque_type, + sym::LinkedList, + sym::hashmap_type, + sym::BTreeMap, + sym::hashset_type, + sym::BTreeSet, + sym::BinaryHeap, + ]; + + if let QPath::TypeRelative(_, method) = path { + if method.ident.name == sym::new { + if let Some(impl_did) = cx.tcx.impl_of_method(def_id) { + if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() { + return std_types_symbols + .iter() + .any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did)); + } + } + } + } + false +} + +/// Returns true if the expr is equal to `Default::default()` of it's type when evaluated. +/// It doesn't cover all cases, for example indirect function calls (some of std +/// functions are supported) but it is the best we have. +pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { + match &e.kind { + ExprKind::Lit(lit) => match lit.node { + LitKind::Bool(false) | LitKind::Int(0, _) => true, + LitKind::Str(s, _) => s.is_empty(), + _ => false, + }, + ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default_equivalent(cx, x)), + ExprKind::Repeat(x, _) => is_default_equivalent(cx, x), + ExprKind::Call(repl_func, _) => if_chain! { + if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; + if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); + if is_diag_trait_item(cx, repl_def_id, sym::Default) + || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath); + then { + true + } + else { + false + } + }, + ExprKind::Path(qpath) => is_lang_ctor(cx, qpath, OptionNone), + ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])), + _ => false, + } +} + /// Checks if the top level expression can be moved into a closure as is. /// Currently checks for: /// * Break/Continue outside the given loop HIR ids. diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 7f7fbdc9f21..d6f9ebe89bc 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -224,6 +224,13 @@ fn is_normalizable_helper<'tcx>( result } +/// Returns true iff the given type is a non aggregate primitive (a bool or char, any integer or +/// floating-point number type). For checking aggregation of primitive types (e.g. tuples and slices +/// of primitive type) see `is_recursively_primitive_type` +pub fn is_non_aggregate_primitive_type(ty: Ty<'_>) -> bool { + matches!(ty.kind(), ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_)) +} + /// Returns true iff the given type is a primitive (a bool or char, any integer or floating-point /// number type, a str, or an array, slice, or tuple of those types). pub fn is_recursively_primitive_type(ty: Ty<'_>) -> bool { diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs new file mode 100644 index 00000000000..336a743de72 --- /dev/null +++ b/tests/ui/derivable_impls.rs @@ -0,0 +1,170 @@ +use std::collections::HashMap; + +struct FooDefault<'a> { + a: bool, + b: i32, + c: u64, + d: Vec, + e: FooND1, + f: FooND2, + g: HashMap, + h: (i32, Vec), + i: [Vec; 3], + j: [i32; 5], + k: Option, + l: &'a [i32], +} + +impl std::default::Default for FooDefault<'_> { + fn default() -> Self { + Self { + a: false, + b: 0, + c: 0u64, + d: vec![], + e: Default::default(), + f: FooND2::default(), + g: HashMap::new(), + h: (0, vec![]), + i: [vec![], vec![], vec![]], + j: [0; 5], + k: None, + l: &[], + } + } +} + +struct TupleDefault(bool, i32, u64); + +impl std::default::Default for TupleDefault { + fn default() -> Self { + Self(false, 0, 0u64) + } +} + +struct FooND1 { + a: bool, +} + +impl std::default::Default for FooND1 { + fn default() -> Self { + Self { a: true } + } +} + +struct FooND2 { + a: i32, +} + +impl std::default::Default for FooND2 { + fn default() -> Self { + Self { a: 5 } + } +} + +struct FooNDNew { + a: bool, +} + +impl FooNDNew { + fn new() -> Self { + Self { a: true } + } +} + +impl Default for FooNDNew { + fn default() -> Self { + Self::new() + } +} + +struct FooNDVec(Vec); + +impl Default for FooNDVec { + fn default() -> Self { + Self(vec![5, 12]) + } +} + +struct StrDefault<'a>(&'a str); + +impl Default for StrDefault<'_> { + fn default() -> Self { + Self("") + } +} + +#[derive(Default)] +struct AlreadyDerived(i32, bool); + +macro_rules! mac { + () => { + 0 + }; + ($e:expr) => { + struct X(u32); + impl Default for X { + fn default() -> Self { + Self($e) + } + } + }; +} + +mac!(0); + +struct Y(u32); +impl Default for Y { + fn default() -> Self { + Self(mac!()) + } +} + +struct RustIssue26925 { + a: Option, +} + +// We should watch out for cases where a manual impl is needed because a +// derive adds different type bounds (https://github.com/rust-lang/rust/issues/26925). +// For example, a struct with Option does not require T: Default, but a derive adds +// that type bound anyways. So until #26925 get fixed we should disable lint +// for the following case +impl Default for RustIssue26925 { + fn default() -> Self { + Self { a: None } + } +} + +struct SpecializedImpl { + a: A, + b: B, +} + +impl Default for SpecializedImpl { + fn default() -> Self { + Self { + a: T::default(), + b: T::default(), + } + } +} + +struct WithoutSelfCurly { + a: bool, +} + +impl Default for WithoutSelfCurly { + fn default() -> Self { + WithoutSelfCurly { a: false } + } +} + +struct WithoutSelfParan(bool); + +impl Default for WithoutSelfParan { + fn default() -> Self { + WithoutSelfParan(false) + } +} + +fn main() {} diff --git a/tests/ui/derivable_impls.stderr b/tests/ui/derivable_impls.stderr new file mode 100644 index 00000000000..4ed64fade02 --- /dev/null +++ b/tests/ui/derivable_impls.stderr @@ -0,0 +1,77 @@ +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:18:1 + | +LL | / impl std::default::Default for FooDefault<'_> { +LL | | fn default() -> Self { +LL | | Self { +LL | | a: false, +... | +LL | | } +LL | | } + | |_^ + | + = note: `-D clippy::derivable-impls` implied by `-D warnings` + = help: try annotating `FooDefault` with `#[derive(Default)]` + +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:39:1 + | +LL | / impl std::default::Default for TupleDefault { +LL | | fn default() -> Self { +LL | | Self(false, 0, 0u64) +LL | | } +LL | | } + | |_^ + | + = help: try annotating `TupleDefault` with `#[derive(Default)]` + +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:91:1 + | +LL | / impl Default for StrDefault<'_> { +LL | | fn default() -> Self { +LL | | Self("") +LL | | } +LL | | } + | |_^ + | + = help: try annotating `StrDefault` with `#[derive(Default)]` + +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:117:1 + | +LL | / impl Default for Y { +LL | | fn default() -> Self { +LL | | Self(mac!()) +LL | | } +LL | | } + | |_^ + | + = help: try annotating `Y` with `#[derive(Default)]` + +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:156:1 + | +LL | / impl Default for WithoutSelfCurly { +LL | | fn default() -> Self { +LL | | WithoutSelfCurly { a: false } +LL | | } +LL | | } + | |_^ + | + = help: try annotating `WithoutSelfCurly` with `#[derive(Default)]` + +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:164:1 + | +LL | / impl Default for WithoutSelfParan { +LL | | fn default() -> Self { +LL | | WithoutSelfParan(false) +LL | | } +LL | | } + | |_^ + | + = help: try annotating `WithoutSelfParan` with `#[derive(Default)]` + +error: aborting due to 6 previous errors + diff --git a/tests/ui/mem_replace.fixed b/tests/ui/mem_replace.fixed index 3b6224254a0..b609ba65946 100644 --- a/tests/ui/mem_replace.fixed +++ b/tests/ui/mem_replace.fixed @@ -51,9 +51,29 @@ fn replace_with_default() { let mut binary_heap: BinaryHeap = BinaryHeap::new(); let _ = std::mem::take(&mut binary_heap); + + let mut tuple = (vec![1, 2], BinaryHeap::::new()); + let _ = std::mem::take(&mut tuple); + + let mut refstr = "hello"; + let _ = std::mem::take(&mut refstr); + + let mut slice: &[i32] = &[1, 2, 3]; + let _ = std::mem::take(&mut slice); +} + +// lint is disabled for primitives because in this case `take` +// has no clear benefit over `replace` and sometimes is harder to read +fn dont_lint_primitive() { + let mut pbool = true; + let _ = std::mem::replace(&mut pbool, false); + + let mut pint = 5; + let _ = std::mem::replace(&mut pint, 0); } fn main() { replace_option_with_none(); replace_with_default(); + dont_lint_primitive(); } diff --git a/tests/ui/mem_replace.rs b/tests/ui/mem_replace.rs index 0a36db9e921..93f6dcdec83 100644 --- a/tests/ui/mem_replace.rs +++ b/tests/ui/mem_replace.rs @@ -51,9 +51,29 @@ fn replace_with_default() { let mut binary_heap: BinaryHeap = BinaryHeap::new(); let _ = std::mem::replace(&mut binary_heap, BinaryHeap::new()); + + let mut tuple = (vec![1, 2], BinaryHeap::::new()); + let _ = std::mem::replace(&mut tuple, (vec![], BinaryHeap::new())); + + let mut refstr = "hello"; + let _ = std::mem::replace(&mut refstr, ""); + + let mut slice: &[i32] = &[1, 2, 3]; + let _ = std::mem::replace(&mut slice, &[]); +} + +// lint is disabled for primitives because in this case `take` +// has no clear benefit over `replace` and sometimes is harder to read +fn dont_lint_primitive() { + let mut pbool = true; + let _ = std::mem::replace(&mut pbool, false); + + let mut pint = 5; + let _ = std::mem::replace(&mut pint, 0); } fn main() { replace_option_with_none(); replace_with_default(); + dont_lint_primitive(); } diff --git a/tests/ui/mem_replace.stderr b/tests/ui/mem_replace.stderr index f8aa1538bff..90dc6c95f85 100644 --- a/tests/ui/mem_replace.stderr +++ b/tests/ui/mem_replace.stderr @@ -98,5 +98,23 @@ error: replacing a value of type `T` with `T::default()` is better expressed usi LL | let _ = std::mem::replace(&mut binary_heap, BinaryHeap::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut binary_heap)` -error: aborting due to 16 previous errors +error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take` + --> $DIR/mem_replace.rs:56:13 + | +LL | let _ = std::mem::replace(&mut tuple, (vec![], BinaryHeap::new())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut tuple)` + +error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take` + --> $DIR/mem_replace.rs:59:13 + | +LL | let _ = std::mem::replace(&mut refstr, ""); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut refstr)` + +error: replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take` + --> $DIR/mem_replace.rs:62:13 + | +LL | let _ = std::mem::replace(&mut slice, &[]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::mem::take(&mut slice)` + +error: aborting due to 19 previous errors