Skip to content

Commit

Permalink
Auto merge of #12511 - humannum14916:assigning_clones_msrv, r=Alexendoo
Browse files Browse the repository at this point in the history
`assigning_clones` should respect MSRV

Fixes: #12502

This PR fixes the `assigning_clones` lint suggesting to use `clone_from` or `clone_into` on incompatible MSRVs.

`assigning_clones` will suggest using either `clone_from` or `clone_into`, both of which were stabilized in 1.63. If the current MSRV is below 1.63, the lint should not trigger.

changelog: [`assigning_clones`]: don't lint when the MSRV is below 1.63.
  • Loading branch information
bors committed Mar 19, 2024
2 parents 3f338b3 + 7c0b2dd commit 89aba8d
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 11 deletions.
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Expand Up @@ -602,6 +602,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
**Affected lints:**
* [`almost_complete_range`](https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range)
* [`approx_constant`](https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant)
* [`assigning_clones`](https://rust-lang.github.io/rust-clippy/master/index.html#assigning_clones)
* [`borrow_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr)
* [`cast_abs_to_unsigned`](https://rust-lang.github.io/rust-clippy/master/index.html#cast_abs_to_unsigned)
* [`checked_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#checked_conversions)
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Expand Up @@ -262,7 +262,7 @@ define_Conf! {
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS.
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES.
///
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
#[default_text = ""]
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/msrvs.rs
Expand Up @@ -23,6 +23,7 @@ msrv_aliases! {
1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
1,68,0 { PATH_MAIN_SEPARATOR_STR }
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
1,63,0 { ASSIGNING_CLONES }
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }
1,59,0 { THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST }
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY }
Expand Down
23 changes: 21 additions & 2 deletions clippy_lints/src/assigning_clones.rs
@@ -1,3 +1,4 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::HirNode;
use clippy_utils::sugg::Sugg;
Expand All @@ -6,7 +7,7 @@ use rustc_errors::Applicability;
use rustc_hir::{self as hir, Expr, ExprKind, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Instance, Mutability};
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;
use rustc_span::def_id::DefId;
use rustc_span::symbol::sym;
use rustc_span::ExpnKind;
Expand Down Expand Up @@ -49,10 +50,26 @@ declare_clippy_lint! {
perf,
"assigning the result of cloning may be inefficient"
}
declare_lint_pass!(AssigningClones => [ASSIGNING_CLONES]);

pub struct AssigningClones {
msrv: Msrv,
}

impl AssigningClones {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
}
}

impl_lint_pass!(AssigningClones => [ASSIGNING_CLONES]);

impl<'tcx> LateLintPass<'tcx> for AssigningClones {
fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx hir::Expr<'_>) {
if !self.msrv.meets(msrvs::ASSIGNING_CLONES) {
return;
}

// Do not fire the lint in macros
let expn_data = assign_expr.span().ctxt().outer_expn_data();
match expn_data.kind {
Expand All @@ -72,6 +89,8 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
suggest(cx, assign_expr, lhs, &call);
}
}

extract_msrv_attr!(LateContext);
}

// Try to resolve the call to `Clone::clone` or `ToOwned::to_owned`.
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Expand Up @@ -1122,7 +1122,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv())));
store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
store.register_late_pass(move |_| Box::new(assigning_clones::AssigningClones::new(msrv())));
store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
store.register_late_pass(|_| Box::new(manual_unwrap_or_default::ManualUnwrapOrDefault));
store.register_late_pass(|_| Box::new(integer_division_remainder_used::IntegerDivisionRemainderUsed));
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/assigning_clones.fixed
Expand Up @@ -128,6 +128,17 @@ fn ignore_generic_clone<T: Clone>(a: &mut T, b: &T) {
*a = b.clone();
}

#[clippy::msrv = "1.62"]
fn msrv_1_62(mut a: String, b: &str) {
// Should not be linted, as clone_into wasn't stabilized until 1.63
a = b.to_owned();
}

#[clippy::msrv = "1.63"]
fn msrv_1_63(mut a: String, b: &str) {
b.clone_into(&mut a);
}

macro_rules! clone_inside {
($a:expr, $b: expr) => {
$a = $b.clone();
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/assigning_clones.rs
Expand Up @@ -128,6 +128,17 @@ fn ignore_generic_clone<T: Clone>(a: &mut T, b: &T) {
*a = b.clone();
}

#[clippy::msrv = "1.62"]
fn msrv_1_62(mut a: String, b: &str) {
// Should not be linted, as clone_into wasn't stabilized until 1.63
a = b.to_owned();
}

#[clippy::msrv = "1.63"]
fn msrv_1_63(mut a: String, b: &str) {
a = b.to_owned();
}

macro_rules! clone_inside {
($a:expr, $b: expr) => {
$a = $b.clone();
Expand Down
20 changes: 13 additions & 7 deletions tests/ui/assigning_clones.stderr
Expand Up @@ -68,40 +68,46 @@ LL | a = b.clone();
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:145:5
--> tests/ui/assigning_clones.rs:139:5
|
LL | a = b.to_owned();
| ^^^^^^^^^^^^^^^^ help: use `clone_into()`: `b.clone_into(&mut a)`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:156:5
|
LL | *mut_string = ref_str.to_owned();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(mut_string)`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:149:5
--> tests/ui/assigning_clones.rs:160:5
|
LL | mut_string = ref_str.to_owned();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut mut_string)`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:170:5
--> tests/ui/assigning_clones.rs:181:5
|
LL | **mut_box_string = ref_str.to_owned();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:174:5
--> tests/ui/assigning_clones.rs:185:5
|
LL | **mut_box_string = ref_str.to_owned();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:178:5
--> tests/ui/assigning_clones.rs:189:5
|
LL | *mut_thing = ToOwned::to_owned(ref_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, mut_thing)`

error: assigning the result of `ToOwned::to_owned()` may be inefficient
--> tests/ui/assigning_clones.rs:182:5
--> tests/ui/assigning_clones.rs:193:5
|
LL | mut_thing = ToOwned::to_owned(ref_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`

error: aborting due to 17 previous errors
error: aborting due to 18 previous errors

0 comments on commit 89aba8d

Please sign in to comment.