Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add msrv config for map_clone #8280

Merged
merged 2 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion clippy_lints/src/borrow_as_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use clippy_utils::{meets_msrv, msrvs};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};

Expand Down Expand Up @@ -94,4 +94,6 @@ impl<'tcx> LateLintPass<'tcx> for BorrowAsPtr {
}
}
}

extract_msrv_attr!(LateContext);
}
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(needless_question_mark::NeedlessQuestionMark));
store.register_late_pass(move || Box::new(casts::Casts::new(msrv)));
store.register_early_pass(move || Box::new(unnested_or_patterns::UnnestedOrPatterns::new(msrv)));
store.register_late_pass(move || Box::new(map_clone::MapClone::new(msrv)));

store.register_late_pass(|| Box::new(size_of_in_element_count::SizeOfInElementCount));
store.register_late_pass(|| Box::new(same_name_method::SameNameMethod));
Expand All @@ -591,7 +592,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
msrv,
))
});
store.register_late_pass(|| Box::new(map_clone::MapClone));
store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore));
store.register_late_pass(|| Box::new(shadow::Shadow::default()));
store.register_late_pass(|| Box::new(unit_types::UnitTypes));
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/manual_bits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clippy_utils::{match_def_path, meets_msrv, msrvs, paths};
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, GenericArg, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::{self, Ty};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
Expand Down Expand Up @@ -72,6 +72,8 @@ impl<'tcx> LateLintPass<'tcx> for ManualBits {
}
}
}

extract_msrv_attr!(LateContext);
}

fn get_one_size_of_ty<'tcx>(
Expand Down
66 changes: 39 additions & 27 deletions clippy_lints/src/map_clone.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{is_copy, is_type_diagnostic_item};
use clippy_utils::{is_trait_method, peel_blocks};
use clippy_utils::{is_trait_method, meets_msrv, msrvs, peel_blocks};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::mir::Mutability;
use rustc_middle::ty;
use rustc_middle::ty::adjustment::Adjust;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::Ident;
use rustc_span::{sym, Span};

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

declare_lint_pass!(MapClone => [MAP_CLONE]);
pub struct MapClone {
msrv: Option<RustcVersion>,
}

impl_lint_pass!(MapClone => [MAP_CLONE]);

impl MapClone {
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}

impl<'tcx> LateLintPass<'tcx> for MapClone {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) {
Expand All @@ -65,15 +76,15 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
hir::BindingAnnotation::Unannotated, .., name, None
) = inner.kind {
if ident_eq(name, closure_expr) {
lint(cx, e.span, args[0].span, true);
self.lint_explicit_closure(cx, e.span, args[0].span, true);
}
},
hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, .., name, None) => {
match closure_expr.kind {
hir::ExprKind::Unary(hir::UnOp::Deref, inner) => {
if ident_eq(name, inner) {
if let ty::Ref(.., Mutability::Not) = cx.typeck_results().expr_ty(inner).kind() {
lint(cx, e.span, args[0].span, true);
self.lint_explicit_closure(cx, e.span, args[0].span, true);
}
}
},
Expand All @@ -90,7 +101,7 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
if let ty::Ref(_, ty, mutability) = obj_ty.kind() {
if matches!(mutability, Mutability::Not) {
let copy = is_copy(cx, ty);
lint(cx, e.span, args[0].span, copy);
self.lint_explicit_closure(cx, e.span, args[0].span, copy);
}
} else {
lint_needless_cloning(cx, e.span, args[0].span);
Expand All @@ -105,6 +116,8 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
}
}
}

extract_msrv_attr!(LateContext);
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
}

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

fn lint(cx: &LateContext<'_>, replace: Span, root: Span, copied: bool) {
let mut applicability = Applicability::MachineApplicable;
if copied {
span_lint_and_sugg(
cx,
MAP_CLONE,
replace,
"you are using an explicit closure for copying elements",
"consider calling the dedicated `copied` method",
format!(
"{}.copied()",
snippet_with_applicability(cx, root, "..", &mut applicability)
),
applicability,
);
} else {
impl MapClone {
fn lint_explicit_closure(&self, cx: &LateContext<'_>, replace: Span, root: Span, is_copy: bool) {
let mut applicability = Applicability::MachineApplicable;
let message = if is_copy {
"you are using an explicit closure for copying elements"
} else {
"you are using an explicit closure for cloning elements"
};
let sugg_method = if is_copy && meets_msrv(self.msrv.as_ref(), &msrvs::ITERATOR_COPIED) {
"copied"
} else {
"cloned"
};

span_lint_and_sugg(
cx,
MAP_CLONE,
replace,
"you are using an explicit closure for cloning elements",
"consider calling the dedicated `cloned` method",
message,
&format!("consider calling the dedicated `{}` method", sugg_method),
format!(
"{}.cloned()",
snippet_with_applicability(cx, root, "..", &mut applicability)
"{}.{}()",
snippet_with_applicability(cx, root, "..", &mut applicability),
sugg_method,
),
applicability,
);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,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, 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.
/// 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.
///
/// The minimum rust version that the project supports
(msrv: Option<String> = None),
Expand Down
24 changes: 22 additions & 2 deletions tests/ui-toml/min_rust_version/min_rust_version.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![allow(clippy::redundant_clone)]
#![warn(clippy::manual_non_exhaustive)]
#![allow(clippy::redundant_clone, clippy::unnecessary_operation)]
#![warn(clippy::manual_non_exhaustive, clippy::borrow_as_ptr, clippy::manual_bits)]

use std::mem::{size_of, size_of_val};
use std::ops::Deref;

mod enums {
Expand Down Expand Up @@ -68,11 +69,30 @@ fn check_index_refutable_slice() {
}
}

fn map_clone_suggest_copied() {
// This should still trigger the lint but suggest `cloned()` instead of `copied()`
let _: Option<u64> = Some(&16).map(|b| *b);
}

fn borrow_as_ptr() {
let val = 1;
let _p = &val as *const i32;

let mut val_mut = 1;
let _p_mut = &mut val_mut as *mut i32;
}

fn manual_bits() {
size_of::<i8>() * 8;
size_of_val(&0u32) * 8;
}

fn main() {
option_as_ref_deref();
match_like_matches();
match_same_arms();
match_same_arms2();
manual_strip_msrv();
check_index_refutable_slice();
borrow_as_ptr();
}
10 changes: 10 additions & 0 deletions tests/ui-toml/min_rust_version/min_rust_version.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: you are using an explicit closure for copying elements
--> $DIR/min_rust_version.rs:74:26
|
LL | let _: Option<u64> = Some(&16).map(|b| *b);
| ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `Some(&16).cloned()`
|
= note: `-D clippy::map-clone` implied by `-D warnings`

error: aborting due to previous error