Skip to content

Commit 8bb4690

Browse files
committed
Auto merge of #8280 - xFrednet:8276-map-clone-msrv, r=flip1995
Add `msrv` config for `map_clone` Just a small PR to have some fun with Clippy and to clear my head a bit 😅 --- changelog: [`map_clone`]: The suggestion takes `msrv` into account changelog: Track `msrv` attribute for `manual_bits` and `borrow_as_prt` fixes: #8276
2 parents 0b143e3 + 1afeb71 commit 8bb4690

File tree

7 files changed

+79
-33
lines changed

7 files changed

+79
-33
lines changed

clippy_lints/src/borrow_as_ptr.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use clippy_utils::{meets_msrv, msrvs};
55
use if_chain::if_chain;
66
use rustc_errors::Applicability;
77
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, TyKind};
8-
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
99
use rustc_semver::RustcVersion;
1010
use rustc_session::{declare_tool_lint, impl_lint_pass};
1111

@@ -94,4 +94,6 @@ impl<'tcx> LateLintPass<'tcx> for BorrowAsPtr {
9494
}
9595
}
9696
}
97+
98+
extract_msrv_attr!(LateContext);
9799
}

clippy_lints/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
581581
store.register_late_pass(move || Box::new(needless_question_mark::NeedlessQuestionMark));
582582
store.register_late_pass(move || Box::new(casts::Casts::new(msrv)));
583583
store.register_early_pass(move || Box::new(unnested_or_patterns::UnnestedOrPatterns::new(msrv)));
584+
store.register_late_pass(move || Box::new(map_clone::MapClone::new(msrv)));
584585

585586
store.register_late_pass(|| Box::new(size_of_in_element_count::SizeOfInElementCount));
586587
store.register_late_pass(|| Box::new(same_name_method::SameNameMethod));
@@ -591,7 +592,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
591592
msrv,
592593
))
593594
});
594-
store.register_late_pass(|| Box::new(map_clone::MapClone));
595595
store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore));
596596
store.register_late_pass(|| Box::new(shadow::Shadow::default()));
597597
store.register_late_pass(|| Box::new(unit_types::UnitTypes));

clippy_lints/src/manual_bits.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::{match_def_path, meets_msrv, msrvs, paths};
44
use rustc_ast::ast::LitKind;
55
use rustc_errors::Applicability;
66
use rustc_hir::{BinOpKind, Expr, ExprKind, GenericArg, QPath};
7-
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_lint::{LateContext, LateLintPass, LintContext};
88
use rustc_middle::ty::{self, Ty};
99
use rustc_semver::RustcVersion;
1010
use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -72,6 +72,8 @@ impl<'tcx> LateLintPass<'tcx> for ManualBits {
7272
}
7373
}
7474
}
75+
76+
extract_msrv_attr!(LateContext);
7577
}
7678

7779
fn get_one_size_of_ty<'tcx>(

clippy_lints/src/map_clone.rs

+39-27
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::ty::{is_copy, is_type_diagnostic_item};
4-
use clippy_utils::{is_trait_method, peel_blocks};
4+
use clippy_utils::{is_trait_method, meets_msrv, msrvs, peel_blocks};
55
use if_chain::if_chain;
66
use rustc_errors::Applicability;
77
use rustc_hir as hir;
8-
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
99
use rustc_middle::mir::Mutability;
1010
use rustc_middle::ty;
1111
use rustc_middle::ty::adjustment::Adjust;
12-
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
use rustc_semver::RustcVersion;
13+
use rustc_session::{declare_tool_lint, impl_lint_pass};
1314
use rustc_span::symbol::Ident;
1415
use rustc_span::{sym, Span};
1516

@@ -42,7 +43,17 @@ declare_clippy_lint! {
4243
"using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types"
4344
}
4445

45-
declare_lint_pass!(MapClone => [MAP_CLONE]);
46+
pub struct MapClone {
47+
msrv: Option<RustcVersion>,
48+
}
49+
50+
impl_lint_pass!(MapClone => [MAP_CLONE]);
51+
52+
impl MapClone {
53+
pub fn new(msrv: Option<RustcVersion>) -> Self {
54+
Self { msrv }
55+
}
56+
}
4657

4758
impl<'tcx> LateLintPass<'tcx> for MapClone {
4859
fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) {
@@ -65,15 +76,15 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
6576
hir::BindingAnnotation::Unannotated, .., name, None
6677
) = inner.kind {
6778
if ident_eq(name, closure_expr) {
68-
lint(cx, e.span, args[0].span, true);
79+
self.lint_explicit_closure(cx, e.span, args[0].span, true);
6980
}
7081
},
7182
hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, .., name, None) => {
7283
match closure_expr.kind {
7384
hir::ExprKind::Unary(hir::UnOp::Deref, inner) => {
7485
if ident_eq(name, inner) {
7586
if let ty::Ref(.., Mutability::Not) = cx.typeck_results().expr_ty(inner).kind() {
76-
lint(cx, e.span, args[0].span, true);
87+
self.lint_explicit_closure(cx, e.span, args[0].span, true);
7788
}
7889
}
7990
},
@@ -90,7 +101,7 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
90101
if let ty::Ref(_, ty, mutability) = obj_ty.kind() {
91102
if matches!(mutability, Mutability::Not) {
92103
let copy = is_copy(cx, ty);
93-
lint(cx, e.span, args[0].span, copy);
104+
self.lint_explicit_closure(cx, e.span, args[0].span, copy);
94105
}
95106
} else {
96107
lint_needless_cloning(cx, e.span, args[0].span);
@@ -105,6 +116,8 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
105116
}
106117
}
107118
}
119+
120+
extract_msrv_attr!(LateContext);
108121
}
109122

110123
fn ident_eq(name: Ident, path: &hir::Expr<'_>) -> bool {
@@ -127,31 +140,30 @@ fn lint_needless_cloning(cx: &LateContext<'_>, root: Span, receiver: Span) {
127140
);
128141
}
129142

130-
fn lint(cx: &LateContext<'_>, replace: Span, root: Span, copied: bool) {
131-
let mut applicability = Applicability::MachineApplicable;
132-
if copied {
133-
span_lint_and_sugg(
134-
cx,
135-
MAP_CLONE,
136-
replace,
137-
"you are using an explicit closure for copying elements",
138-
"consider calling the dedicated `copied` method",
139-
format!(
140-
"{}.copied()",
141-
snippet_with_applicability(cx, root, "..", &mut applicability)
142-
),
143-
applicability,
144-
);
145-
} else {
143+
impl MapClone {
144+
fn lint_explicit_closure(&self, cx: &LateContext<'_>, replace: Span, root: Span, is_copy: bool) {
145+
let mut applicability = Applicability::MachineApplicable;
146+
let message = if is_copy {
147+
"you are using an explicit closure for copying elements"
148+
} else {
149+
"you are using an explicit closure for cloning elements"
150+
};
151+
let sugg_method = if is_copy && meets_msrv(self.msrv.as_ref(), &msrvs::ITERATOR_COPIED) {
152+
"copied"
153+
} else {
154+
"cloned"
155+
};
156+
146157
span_lint_and_sugg(
147158
cx,
148159
MAP_CLONE,
149160
replace,
150-
"you are using an explicit closure for cloning elements",
151-
"consider calling the dedicated `cloned` method",
161+
message,
162+
&format!("consider calling the dedicated `{}` method", sugg_method),
152163
format!(
153-
"{}.cloned()",
154-
snippet_with_applicability(cx, root, "..", &mut applicability)
164+
"{}.{}()",
165+
snippet_with_applicability(cx, root, "..", &mut applicability),
166+
sugg_method,
155167
),
156168
applicability,
157169
);

clippy_lints/src/utils/conf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ define_Conf! {
156156
///
157157
/// Suppress lints whenever the suggested change would cause breakage for other crates.
158158
(avoid_breaking_exported_api: bool = true),
159-
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, 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.
159+
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, 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.
160160
///
161161
/// The minimum rust version that the project supports
162162
(msrv: Option<String> = None),

tests/ui-toml/min_rust_version/min_rust_version.rs

+22-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
#![allow(clippy::redundant_clone)]
2-
#![warn(clippy::manual_non_exhaustive)]
1+
#![allow(clippy::redundant_clone, clippy::unnecessary_operation)]
2+
#![warn(clippy::manual_non_exhaustive, clippy::borrow_as_ptr, clippy::manual_bits)]
33

4+
use std::mem::{size_of, size_of_val};
45
use std::ops::Deref;
56

67
mod enums {
@@ -68,11 +69,30 @@ fn check_index_refutable_slice() {
6869
}
6970
}
7071

72+
fn map_clone_suggest_copied() {
73+
// This should still trigger the lint but suggest `cloned()` instead of `copied()`
74+
let _: Option<u64> = Some(&16).map(|b| *b);
75+
}
76+
77+
fn borrow_as_ptr() {
78+
let val = 1;
79+
let _p = &val as *const i32;
80+
81+
let mut val_mut = 1;
82+
let _p_mut = &mut val_mut as *mut i32;
83+
}
84+
85+
fn manual_bits() {
86+
size_of::<i8>() * 8;
87+
size_of_val(&0u32) * 8;
88+
}
89+
7190
fn main() {
7291
option_as_ref_deref();
7392
match_like_matches();
7493
match_same_arms();
7594
match_same_arms2();
7695
manual_strip_msrv();
7796
check_index_refutable_slice();
97+
borrow_as_ptr();
7898
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: you are using an explicit closure for copying elements
2+
--> $DIR/min_rust_version.rs:74:26
3+
|
4+
LL | let _: Option<u64> = Some(&16).map(|b| *b);
5+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `Some(&16).cloned()`
6+
|
7+
= note: `-D clippy::map-clone` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)