From 95fc598e27dd5029fd06cea1e8166b52200ff7e5 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 13 Jul 2025 16:29:39 +0200 Subject: [PATCH 1/2] suggest making the variable mutable if necessary --- clippy_lints/src/methods/map_identity.rs | 54 ++++++++++++++++-------- tests/ui/map_identity.fixed | 9 ++-- tests/ui/map_identity.rs | 5 ++- tests/ui/map_identity.stderr | 20 +++++++-- tests/ui/map_identity_unfixable.rs | 9 ++++ tests/ui/map_identity_unfixable.stderr | 16 +++++++ 6 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 tests/ui/map_identity_unfixable.rs create mode 100644 tests/ui/map_identity_unfixable.stderr diff --git a/clippy_lints/src/methods/map_identity.rs b/clippy_lints/src/methods/map_identity.rs index 98def66ca149..81a82d697046 100644 --- a/clippy_lints/src/methods/map_identity.rs +++ b/clippy_lints/src/methods/map_identity.rs @@ -1,7 +1,6 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_expr_untyped_identity_function, is_trait_method, path_to_local}; -use rustc_ast::BindingMode; +use clippy_utils::{is_expr_untyped_identity_function, is_mutable, is_trait_method, path_to_local}; use rustc_errors::Applicability; use rustc_hir::{self as hir, Node, PatKind}; use rustc_lint::LateContext; @@ -23,26 +22,47 @@ pub(super) fn check( || is_type_diagnostic_item(cx, caller_ty, sym::Result) || is_type_diagnostic_item(cx, caller_ty, sym::Option)) && is_expr_untyped_identity_function(cx, map_arg) - && let Some(sugg_span) = expr.span.trim_start(caller.span) + && let Some(call_span) = expr.span.trim_start(caller.span) { - // If the result of `.map(identity)` is used as a mutable reference, - // the caller must not be an immutable binding. - if cx.typeck_results().expr_ty_adjusted(expr).is_mutable_ptr() - && let Some(hir_id) = path_to_local(caller) - && let Node::Pat(pat) = cx.tcx.hir_node(hir_id) - && !matches!(pat.kind, PatKind::Binding(BindingMode::MUT, ..)) - { - return; + let mut sugg = vec![(call_span, String::new())]; + let mut apply = true; + if !is_mutable(cx, caller) { + if let Some(hir_id) = path_to_local(caller) + && let Node::Pat(pat) = cx.tcx.hir_node(hir_id) + && let PatKind::Binding(_, _, ident, _) = pat.kind + { + sugg.push((ident.span.shrink_to_lo(), String::from("mut "))); + } else { + // If we can't make the binding mutable, make the suggestion `Unspecified` to prevent it from being + // automatically applied, and add a complementary help message. + apply = false; + } } - span_lint_and_sugg( + let method_requiring_mut = String::from("random_method"); // TODO + + span_lint_and_then( cx, MAP_IDENTITY, - sugg_span, + call_span, "unnecessary map of the identity function", - format!("remove the call to `{name}`"), - String::new(), - Applicability::MachineApplicable, + |diag| { + diag.multipart_suggestion( + format!("remove the call to `{name}`"), + sugg, + if apply { + Applicability::MachineApplicable + } else { + Applicability::Unspecified + }, + ); + if !apply { + diag.span_note( + caller.span, + format!("this must be made mutable to use `{method_requiring_mut}`"), + ); + } + }, ); } } diff --git a/tests/ui/map_identity.fixed b/tests/ui/map_identity.fixed index b82d3e6d9567..00e626eeb24a 100644 --- a/tests/ui/map_identity.fixed +++ b/tests/ui/map_identity.fixed @@ -72,9 +72,12 @@ fn issue11764() { } fn issue13904() { - // don't lint: `it.next()` would not be legal as `it` is immutable - let it = [1, 2, 3].into_iter(); - let _ = it.map(|x| x).next(); + // lint, but there's a catch: + // when we remove the `.map()`, `it.next()` would require `it` to be mutable + // therefore, include that in the suggestion as well + let mut it = [1, 2, 3].into_iter(); + let _ = it.next(); + //~^ map_identity // lint #[allow(unused_mut)] diff --git a/tests/ui/map_identity.rs b/tests/ui/map_identity.rs index c295bf872701..fbffccfbb9ee 100644 --- a/tests/ui/map_identity.rs +++ b/tests/ui/map_identity.rs @@ -78,9 +78,12 @@ fn issue11764() { } fn issue13904() { - // don't lint: `it.next()` would not be legal as `it` is immutable + // lint, but there's a catch: + // when we remove the `.map()`, `it.next()` would require `it` to be mutable + // therefore, include that in the suggestion as well let it = [1, 2, 3].into_iter(); let _ = it.map(|x| x).next(); + //~^ map_identity // lint #[allow(unused_mut)] diff --git a/tests/ui/map_identity.stderr b/tests/ui/map_identity.stderr index 9b624a0dc755..6bdbfde09ae8 100644 --- a/tests/ui/map_identity.stderr +++ b/tests/ui/map_identity.stderr @@ -76,22 +76,34 @@ LL | let _ = x.iter().copied().map(|(x, y)| (x, y)); | ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:88:15 + --> tests/ui/map_identity.rs:85:15 + | +LL | let _ = it.map(|x| x).next(); + | ^^^^^^^^^^^ + | +help: remove the call to `map` + | +LL ~ let mut it = [1, 2, 3].into_iter(); +LL ~ let _ = it.next(); + | + +error: unnecessary map of the identity function + --> tests/ui/map_identity.rs:91:15 | LL | let _ = it.map(|x| x).next(); | ^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:93:19 + --> tests/ui/map_identity.rs:96:19 | LL | let _ = { it }.map(|x| x).next(); | ^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:105:30 + --> tests/ui/map_identity.rs:108:30 | LL | let _ = x.iter().copied().map(|[x, y]| [x, y]); | ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` -error: aborting due to 14 previous errors +error: aborting due to 15 previous errors diff --git a/tests/ui/map_identity_unfixable.rs b/tests/ui/map_identity_unfixable.rs new file mode 100644 index 000000000000..5a5106e76d16 --- /dev/null +++ b/tests/ui/map_identity_unfixable.rs @@ -0,0 +1,9 @@ +//@no-rustfix +#![warn(clippy::map_identity)] + +fn main() { + let mut index = [true, true, false, false, false, true].iter(); + let subindex = (index.by_ref().take(3), 42); + let _ = subindex.0.map(|n| n).next(); + //~^ map_identity +} diff --git a/tests/ui/map_identity_unfixable.stderr b/tests/ui/map_identity_unfixable.stderr new file mode 100644 index 000000000000..0036c3d5cac0 --- /dev/null +++ b/tests/ui/map_identity_unfixable.stderr @@ -0,0 +1,16 @@ +error: unnecessary map of the identity function + --> tests/ui/map_identity_unfixable.rs:7:23 + | +LL | let _ = subindex.0.map(|n| n).next(); + | ^^^^^^^^^^^ help: remove the call to `map` + | +note: this must be made mutable to use `random_method` + --> tests/ui/map_identity_unfixable.rs:7:13 + | +LL | let _ = subindex.0.map(|n| n).next(); + | ^^^^^^^^^^ + = note: `-D clippy::map-identity` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` + +error: aborting due to 1 previous error + From 4ae3029cd0c4aa3d07b8c8caddf1da62e448ee22 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 13 Jul 2025 19:53:36 +0200 Subject: [PATCH 2/2] say what exactly requires the variable to be mutable --- clippy_lints/src/methods/map_identity.rs | 22 ++++++++++++++++------ tests/ui/map_identity_unfixable.stderr | 2 +- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/methods/map_identity.rs b/clippy_lints/src/methods/map_identity.rs index 81a82d697046..6497cdcaf11c 100644 --- a/clippy_lints/src/methods/map_identity.rs +++ b/clippy_lints/src/methods/map_identity.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{is_expr_untyped_identity_function, is_mutable, is_trait_method, path_to_local}; use rustc_errors::Applicability; -use rustc_hir::{self as hir, Node, PatKind}; +use rustc_hir::{self as hir, ExprKind, Node, PatKind}; use rustc_lint::LateContext; use rustc_span::{Span, Symbol, sym}; @@ -39,7 +39,13 @@ pub(super) fn check( } } - let method_requiring_mut = String::from("random_method"); // TODO + let method_requiring_mut = if let Node::Expr(expr) = cx.tcx.parent_hir_node(expr.hir_id) + && let ExprKind::MethodCall(method, ..) = expr.kind + { + Some(method.ident) + } else { + None + }; span_lint_and_then( cx, @@ -57,10 +63,14 @@ pub(super) fn check( }, ); if !apply { - diag.span_note( - caller.span, - format!("this must be made mutable to use `{method_requiring_mut}`"), - ); + if let Some(method_requiring_mut) = method_requiring_mut { + diag.span_note( + caller.span, + format!("this must be made mutable to use `{method_requiring_mut}`"), + ); + } else { + diag.span_note(caller.span, "this must be made mutable".to_string()); + } } }, ); diff --git a/tests/ui/map_identity_unfixable.stderr b/tests/ui/map_identity_unfixable.stderr index 0036c3d5cac0..4d6a46df41cc 100644 --- a/tests/ui/map_identity_unfixable.stderr +++ b/tests/ui/map_identity_unfixable.stderr @@ -4,7 +4,7 @@ error: unnecessary map of the identity function LL | let _ = subindex.0.map(|n| n).next(); | ^^^^^^^^^^^ help: remove the call to `map` | -note: this must be made mutable to use `random_method` +note: this must be made mutable to use `next` --> tests/ui/map_identity_unfixable.rs:7:13 | LL | let _ = subindex.0.map(|n| n).next();