From 9e7858545a2c0427120065431809653e3bb78ce2 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 14 Jan 2022 23:45:05 +0100 Subject: [PATCH 1/2] Add `msrv` config for `map_clone` --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/map_clone.rs | 66 +++++++++++-------- clippy_lints/src/utils/conf.rs | 2 +- .../min_rust_version/min_rust_version.rs | 5 ++ .../min_rust_version/min_rust_version.stderr | 10 +++ 5 files changed, 56 insertions(+), 29 deletions(-) create mode 100644 tests/ui-toml/min_rust_version/min_rust_version.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 79e9882fe..ed25d6fd2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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)); @@ -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)); diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 174c7da28..5b203fd3d 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -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}; @@ -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, +} + +impl_lint_pass!(MapClone => [MAP_CLONE]); + +impl MapClone { + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} impl<'tcx> LateLintPass<'tcx> for MapClone { fn check_expr(&mut self, cx: &LateContext<'_>, e: &hir::Expr<'_>) { @@ -65,7 +76,7 @@ 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) => { @@ -73,7 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for MapClone { 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); } } }, @@ -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); @@ -105,6 +116,8 @@ impl<'tcx> LateLintPass<'tcx> for MapClone { } } } + + extract_msrv_attr!(LateContext); } fn ident_eq(name: Ident, path: &hir::Expr<'_>) -> bool { @@ -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 { +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 copying elements", - "consider calling the dedicated `copied` method", + message, + &format!("consider calling the dedicated `{}` method", sugg_method), format!( - "{}.copied()", - snippet_with_applicability(cx, root, "..", &mut applicability) - ), - applicability, - ); - } else { - span_lint_and_sugg( - cx, - MAP_CLONE, - replace, - "you are using an explicit closure for cloning elements", - "consider calling the dedicated `cloned` method", - format!( - "{}.cloned()", - snippet_with_applicability(cx, root, "..", &mut applicability) + "{}.{}()", + snippet_with_applicability(cx, root, "..", &mut applicability), + sugg_method, ), applicability, ); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index d6deb50cc..0cc6f5206 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -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. /// /// The minimum rust version that the project supports (msrv: Option = None), diff --git a/tests/ui-toml/min_rust_version/min_rust_version.rs b/tests/ui-toml/min_rust_version/min_rust_version.rs index 8e1049265..213252f7b 100644 --- a/tests/ui-toml/min_rust_version/min_rust_version.rs +++ b/tests/ui-toml/min_rust_version/min_rust_version.rs @@ -68,6 +68,11 @@ fn check_index_refutable_slice() { } } +fn map_clone_suggest_copied() { + // This should still trigger the lint but suggest `cloned()` instead of `copied()` + let _: Option = Some(&16).map(|b| *b); +} + fn main() { option_as_ref_deref(); match_like_matches(); diff --git a/tests/ui-toml/min_rust_version/min_rust_version.stderr b/tests/ui-toml/min_rust_version/min_rust_version.stderr new file mode 100644 index 000000000..f226c012c --- /dev/null +++ b/tests/ui-toml/min_rust_version/min_rust_version.stderr @@ -0,0 +1,10 @@ +error: you are using an explicit closure for copying elements + --> $DIR/min_rust_version.rs:73:26 + | +LL | let _: Option = 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 + From 1afeb7106514557e19b2eb48529a9a96879ca4d3 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Sat, 15 Jan 2022 00:39:06 +0100 Subject: [PATCH 2/2] Track `msrv` attribute for `manual_bits` and `borrow_as_prt` --- clippy_lints/src/borrow_as_ptr.rs | 4 +++- clippy_lints/src/manual_bits.rs | 4 +++- clippy_lints/src/utils/conf.rs | 2 +- .../min_rust_version/min_rust_version.rs | 19 +++++++++++++++++-- .../min_rust_version/min_rust_version.stderr | 2 +- 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/borrow_as_ptr.rs b/clippy_lints/src/borrow_as_ptr.rs index b8f5217af..265cdeab0 100644 --- a/clippy_lints/src/borrow_as_ptr.rs +++ b/clippy_lints/src/borrow_as_ptr.rs @@ -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}; @@ -94,4 +94,6 @@ impl<'tcx> LateLintPass<'tcx> for BorrowAsPtr { } } } + + extract_msrv_attr!(LateContext); } diff --git a/clippy_lints/src/manual_bits.rs b/clippy_lints/src/manual_bits.rs index 50bf2527e..a72ef1d2d 100644 --- a/clippy_lints/src/manual_bits.rs +++ b/clippy_lints/src/manual_bits.rs @@ -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}; @@ -72,6 +72,8 @@ impl<'tcx> LateLintPass<'tcx> for ManualBits { } } } + + extract_msrv_attr!(LateContext); } fn get_one_size_of_ty<'tcx>( diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 0cc6f5206..c9d99617c 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -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, MAP_CLONE. + /// 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 = None), diff --git a/tests/ui-toml/min_rust_version/min_rust_version.rs b/tests/ui-toml/min_rust_version/min_rust_version.rs index 213252f7b..1e3ec123a 100644 --- a/tests/ui-toml/min_rust_version/min_rust_version.rs +++ b/tests/ui-toml/min_rust_version/min_rust_version.rs @@ -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 { @@ -73,6 +74,19 @@ fn map_clone_suggest_copied() { let _: Option = 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::() * 8; + size_of_val(&0u32) * 8; +} + fn main() { option_as_ref_deref(); match_like_matches(); @@ -80,4 +94,5 @@ fn main() { match_same_arms2(); manual_strip_msrv(); check_index_refutable_slice(); + borrow_as_ptr(); } diff --git a/tests/ui-toml/min_rust_version/min_rust_version.stderr b/tests/ui-toml/min_rust_version/min_rust_version.stderr index f226c012c..a1e7361c0 100644 --- a/tests/ui-toml/min_rust_version/min_rust_version.stderr +++ b/tests/ui-toml/min_rust_version/min_rust_version.stderr @@ -1,5 +1,5 @@ error: you are using an explicit closure for copying elements - --> $DIR/min_rust_version.rs:73:26 + --> $DIR/min_rust_version.rs:74:26 | LL | let _: Option = Some(&16).map(|b| *b); | ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `Some(&16).cloned()`