From 5f651da2de3e4141df42a47ef47762b642c447bc Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Fri, 3 Nov 2023 23:23:59 +0100 Subject: [PATCH 1/2] fix iter_kv_map dont suggest into_keys and into_values if msrv is to low --- clippy_config/src/msrvs.rs | 1 + clippy_lints/src/methods/iter_kv_map.rs | 5 ++ clippy_lints/src/methods/mod.rs | 2 +- tests/ui/iter_kv_map.fixed | 43 +++++++++++++++++ tests/ui/iter_kv_map.rs | 43 +++++++++++++++++ tests/ui/iter_kv_map.stderr | 62 ++++++++++++++++++++++++- 6 files changed, 154 insertions(+), 2 deletions(-) diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index 011d54629d41..b3ef666e3063 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -23,6 +23,7 @@ msrv_aliases! { 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE } 1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY } 1,55,0 { SEEK_REWIND } + 1,54,0 { INTO_KEYS } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } 1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS } diff --git a/clippy_lints/src/methods/iter_kv_map.rs b/clippy_lints/src/methods/iter_kv_map.rs index c5dbb6ad98be..e1b934d36ea8 100644 --- a/clippy_lints/src/methods/iter_kv_map.rs +++ b/clippy_lints/src/methods/iter_kv_map.rs @@ -1,6 +1,7 @@ #![allow(unused_imports)] use super::ITER_KV_MAP; +use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::ty::is_type_diagnostic_item; @@ -21,7 +22,11 @@ pub(super) fn check<'tcx>( expr: &'tcx Expr<'tcx>, // .iter().map(|(_, v_| v)) recv: &'tcx Expr<'tcx>, // hashmap m_arg: &'tcx Expr<'tcx>, // |(_, v)| v + msrv: &Msrv, ) { + if map_type == "into_iter" && !msrv.meets(msrvs::INTO_KEYS) { + return; + } if !expr.span.from_expansion() && let ExprKind::Closure(c) = m_arg.kind && let Body { diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 57c3913944f3..d9ab0f3b8cae 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4255,7 +4255,7 @@ impl Methods { map_clone::check(cx, expr, recv, m_arg, &self.msrv); match method_call(recv) { Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => { - iter_kv_map::check(cx, map_name, expr, recv2, m_arg); + iter_kv_map::check(cx, map_name, expr, recv2, m_arg, &self.msrv); }, Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( cx, diff --git a/tests/ui/iter_kv_map.fixed b/tests/ui/iter_kv_map.fixed index 566a5b690d84..2cbf972fca5f 100644 --- a/tests/ui/iter_kv_map.fixed +++ b/tests/ui/iter_kv_map.fixed @@ -89,3 +89,46 @@ fn main() { // Don't let a mut interfere. let _ = map.clone().into_values().count(); } + +#[clippy::msrv = "1.53"] +fn msrv_1_53() { + let map: HashMap = HashMap::new(); + + // Don't lint because into_iter is not supported + let _ = map.clone().into_iter().map(|(key, _)| key).collect::>(); + let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::>(); + + let _ = map.clone().into_iter().map(|(_, val)| val).collect::>(); + let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::>(); + + // Lint + let _ = map.keys().collect::>(); + //~^ ERROR: iterating on a map's keys + let _ = map.values().collect::>(); + //~^ ERROR: iterating on a map's values + let _ = map.values().map(|v| v + 2).collect::>(); + //~^ ERROR: iterating on a map's values +} + +#[clippy::msrv = "1.54"] +fn msrv_1_54() { + // Lint all + let map: HashMap = HashMap::new(); + + let _ = map.clone().into_keys().collect::>(); + //~^ ERROR: iterating on a map's keys + let _ = map.clone().into_keys().map(|key| key + 2).collect::>(); + //~^ ERROR: iterating on a map's keys + + let _ = map.clone().into_values().collect::>(); + //~^ ERROR: iterating on a map's values + let _ = map.clone().into_values().map(|val| val + 2).collect::>(); + //~^ ERROR: iterating on a map's values + + let _ = map.keys().collect::>(); + //~^ ERROR: iterating on a map's keys + let _ = map.values().collect::>(); + //~^ ERROR: iterating on a map's values + let _ = map.values().map(|v| v + 2).collect::>(); + //~^ ERROR: iterating on a map's values +} diff --git a/tests/ui/iter_kv_map.rs b/tests/ui/iter_kv_map.rs index d85e501da487..6a9a4267a765 100644 --- a/tests/ui/iter_kv_map.rs +++ b/tests/ui/iter_kv_map.rs @@ -93,3 +93,46 @@ fn main() { // Don't let a mut interfere. let _ = map.clone().into_iter().map(|(_, mut val)| val).count(); } + +#[clippy::msrv = "1.53"] +fn msrv_1_53() { + let map: HashMap = HashMap::new(); + + // Don't lint because into_iter is not supported + let _ = map.clone().into_iter().map(|(key, _)| key).collect::>(); + let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::>(); + + let _ = map.clone().into_iter().map(|(_, val)| val).collect::>(); + let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::>(); + + // Lint + let _ = map.iter().map(|(key, _)| key).collect::>(); + //~^ ERROR: iterating on a map's keys + let _ = map.iter().map(|(_, value)| value).collect::>(); + //~^ ERROR: iterating on a map's values + let _ = map.iter().map(|(_, v)| v + 2).collect::>(); + //~^ ERROR: iterating on a map's values +} + +#[clippy::msrv = "1.54"] +fn msrv_1_54() { + // Lint all + let map: HashMap = HashMap::new(); + + let _ = map.clone().into_iter().map(|(key, _)| key).collect::>(); + //~^ ERROR: iterating on a map's keys + let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::>(); + //~^ ERROR: iterating on a map's keys + + let _ = map.clone().into_iter().map(|(_, val)| val).collect::>(); + //~^ ERROR: iterating on a map's values + let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::>(); + //~^ ERROR: iterating on a map's values + + let _ = map.iter().map(|(key, _)| key).collect::>(); + //~^ ERROR: iterating on a map's keys + let _ = map.iter().map(|(_, value)| value).collect::>(); + //~^ ERROR: iterating on a map's values + let _ = map.iter().map(|(_, v)| v + 2).collect::>(); + //~^ ERROR: iterating on a map's values +} diff --git a/tests/ui/iter_kv_map.stderr b/tests/ui/iter_kv_map.stderr index 62155b7f838e..471615978e1e 100644 --- a/tests/ui/iter_kv_map.stderr +++ b/tests/ui/iter_kv_map.stderr @@ -201,5 +201,65 @@ error: iterating on a map's values LL | let _ = map.clone().into_iter().map(|(_, mut val)| val).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()` -error: aborting due to 28 previous errors +error: iterating on a map's keys + --> $DIR/iter_kv_map.rs:109:13 + | +LL | let _ = map.iter().map(|(key, _)| key).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` + +error: iterating on a map's values + --> $DIR/iter_kv_map.rs:111:13 + | +LL | let _ = map.iter().map(|(_, value)| value).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()` + +error: iterating on a map's values + --> $DIR/iter_kv_map.rs:113:13 + | +LL | let _ = map.iter().map(|(_, v)| v + 2).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)` + +error: iterating on a map's keys + --> $DIR/iter_kv_map.rs:122:13 + | +LL | let _ = map.clone().into_iter().map(|(key, _)| key).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys()` + +error: iterating on a map's keys + --> $DIR/iter_kv_map.rs:124:13 + | +LL | let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys().map(|key| key + 2)` + +error: iterating on a map's values + --> $DIR/iter_kv_map.rs:127:13 + | +LL | let _ = map.clone().into_iter().map(|(_, val)| val).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()` + +error: iterating on a map's values + --> $DIR/iter_kv_map.rs:129:13 + | +LL | let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|val| val + 2)` + +error: iterating on a map's keys + --> $DIR/iter_kv_map.rs:132:13 + | +LL | let _ = map.iter().map(|(key, _)| key).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` + +error: iterating on a map's values + --> $DIR/iter_kv_map.rs:134:13 + | +LL | let _ = map.iter().map(|(_, value)| value).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()` + +error: iterating on a map's values + --> $DIR/iter_kv_map.rs:136:13 + | +LL | let _ = map.iter().map(|(_, v)| v + 2).collect::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)` + +error: aborting due to 38 previous errors From a20f61b1941a6ac7be6e6f72102357d79db3b98f Mon Sep 17 00:00:00 2001 From: Matthias Richter Date: Sat, 4 Nov 2023 01:22:04 +0100 Subject: [PATCH 2/2] add iter_kv_map to msrv config --- book/src/lint_configuration.md | 1 + clippy_config/src/conf.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 841a5b6d0077..f1fdf9b4d5c0 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -150,6 +150,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio * [`tuple_array_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions) * [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold) * [`manual_hash_one`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one) +* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map) ## `cognitive-complexity-threshold` diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 47259776921b..f6eda5e178ac 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -249,7 +249,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. + /// 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. /// /// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml` #[default_text = ""]