diff --git a/clippy_lints/src/methods/map_identity.rs b/clippy_lints/src/methods/map_identity.rs index 98def66ca149..3ea2ad0efcc7 100644 --- a/clippy_lints/src/methods/map_identity.rs +++ b/clippy_lints/src/methods/map_identity.rs @@ -1,14 +1,16 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::source::snippet_with_applicability; 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_with_projections}; use rustc_errors::Applicability; -use rustc_hir::{self as hir, Node, PatKind}; -use rustc_lint::LateContext; +use rustc_hir::{self as hir, ExprKind, Node, PatKind}; +use rustc_lint::{LateContext, LintContext}; use rustc_span::{Span, Symbol, sym}; use super::MAP_IDENTITY; +const MSG: &str = "unnecessary map of the identity function"; + pub(super) fn check( cx: &LateContext<'_>, expr: &hir::Expr<'_>, @@ -23,26 +25,63 @@ 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 main_sugg = (call_span, String::new()); + let mut app = Applicability::MachineApplicable; + + let needs_to_be_mutable = cx.typeck_results().expr_ty_adjusted(expr).is_mutable_ptr(); + if needs_to_be_mutable && !is_mutable(cx, caller) { + if let Some(hir_id) = path_to_local_with_projections(caller) + && let Node::Pat(pat) = cx.tcx.hir_node(hir_id) + && let PatKind::Binding(_, _, ident, _) = pat.kind + { + // We can reach the binding -- suggest making it mutable + let suggs = vec![main_sugg, (ident.span.shrink_to_lo(), String::from("mut "))]; + + let ident = snippet_with_applicability(cx.sess(), ident.span, "_", &mut app); - span_lint_and_sugg( - cx, - MAP_IDENTITY, - sugg_span, - "unnecessary map of the identity function", - format!("remove the call to `{name}`"), - String::new(), - Applicability::MachineApplicable, - ); + span_lint_and_then(cx, MAP_IDENTITY, call_span, MSG, |diag| { + diag.multipart_suggestion( + format!("remove the call to `{name}`, and make `{ident}` mutable"), + suggs, + app, + ); + }); + } else { + // If we can't make the binding mutable, prevent the suggestion from being automatically applied, + // and add a complementary help message. + app = Applicability::Unspecified; + + 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, MAP_IDENTITY, call_span, MSG, |diag| { + diag.span_suggestion(main_sugg.0, format!("remove the call to `{name}`"), main_sugg.1, app); + + let note = if let Some(method_requiring_mut) = method_requiring_mut { + format!("this must be made mutable to use `{method_requiring_mut}`") + } else { + "this must be made mutable".to_string() + }; + diag.span_note(caller.span, note); + }); + } + } else { + span_lint_and_sugg( + cx, + MAP_IDENTITY, + main_sugg.0, + MSG, + format!("remove the call to `{name}`"), + main_sugg.1, + app, + ); + } } } diff --git a/tests/ui/map_identity.fixed b/tests/ui/map_identity.fixed index 6c971ba63384..a10aae8cc121 100644 --- a/tests/ui/map_identity.fixed +++ b/tests/ui/map_identity.fixed @@ -1,3 +1,4 @@ +//@require-annotations-for-level: ERROR #![warn(clippy::map_identity)] #![allow(clippy::needless_return, clippy::disallowed_names)] @@ -72,20 +73,33 @@ 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 + //~| HELP: remove the call to `map`, and make `it` mutable + + // lint + let mut index = [1, 2, 3].into_iter(); + let mut subindex = (index.by_ref().take(3), 42); + let _ = subindex.0.next(); + //~^ map_identity + //~| HELP: remove the call to `map`, and make `subindex` mutable // lint #[allow(unused_mut)] let mut it = [1, 2, 3].into_iter(); let _ = it.next(); //~^ map_identity + //~| HELP: remove the call to `map` // lint let it = [1, 2, 3].into_iter(); let _ = { it }.next(); //~^ map_identity + //~| HELP: remove the call to `map` } // same as `issue11764`, but for arrays diff --git a/tests/ui/map_identity.rs b/tests/ui/map_identity.rs index 59dcfcda3b6e..fc6e018f9248 100644 --- a/tests/ui/map_identity.rs +++ b/tests/ui/map_identity.rs @@ -1,3 +1,4 @@ +//@require-annotations-for-level: ERROR #![warn(clippy::map_identity)] #![allow(clippy::needless_return, clippy::disallowed_names)] @@ -78,20 +79,33 @@ 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 + //~| HELP: remove the call to `map`, and make `it` mutable + + // lint + let mut index = [1, 2, 3].into_iter(); + let subindex = (index.by_ref().take(3), 42); + let _ = subindex.0.map(|n| n).next(); + //~^ map_identity + //~| HELP: remove the call to `map`, and make `subindex` mutable // lint #[allow(unused_mut)] let mut it = [1, 2, 3].into_iter(); let _ = it.map(|x| x).next(); //~^ map_identity + //~| HELP: remove the call to `map` // lint let it = [1, 2, 3].into_iter(); let _ = { it }.map(|x| x).next(); //~^ map_identity + //~| HELP: remove the call to `map` } // same as `issue11764`, but for arrays diff --git a/tests/ui/map_identity.stderr b/tests/ui/map_identity.stderr index a50c0d6b87b5..8d19d62cdc64 100644 --- a/tests/ui/map_identity.stderr +++ b/tests/ui/map_identity.stderr @@ -1,5 +1,5 @@ error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:7:47 + --> tests/ui/map_identity.rs:8:47 | LL | let _: Vec<_> = x.iter().map(not_identity).map(|x| return x).collect(); | ^^^^^^^^^^^^^^^^^^ help: remove the call to `map` @@ -8,25 +8,25 @@ LL | let _: Vec<_> = x.iter().map(not_identity).map(|x| return x).collect(); = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:9:57 + --> tests/ui/map_identity.rs:10:57 | LL | let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect(); | ^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:9:29 + --> tests/ui/map_identity.rs:10:29 | LL | let _: Vec<_> = x.iter().map(std::convert::identity).map(|y| y).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:12:32 + --> tests/ui/map_identity.rs:13:32 | LL | let _: Option = Some(3).map(|x| x); | ^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:14:36 + --> tests/ui/map_identity.rs:15:36 | LL | let _: Result = Ok(-3).map(|x| { | ____________________________________^ @@ -36,19 +36,19 @@ LL | | }); | |______^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:25:36 + --> tests/ui/map_identity.rs:26:36 | LL | let _: Result = Ok(1).map_err(|a| a); | ^^^^^^^^^^^^^^^ help: remove the call to `map_err` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:36:22 + --> tests/ui/map_identity.rs:37:22 | LL | let _ = x.clone().map(|(x, y)| (x, y)); | ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:38:22 + --> tests/ui/map_identity.rs:39:22 | LL | let _ = x.clone().map(|(x, y)| { | ______________________^ @@ -58,76 +58,100 @@ LL | | }); | |______^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:42:22 + --> tests/ui/map_identity.rs:43:22 | LL | let _ = x.clone().map(|(x, y)| return (x, y)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:46:22 + --> tests/ui/map_identity.rs:47:22 | LL | let _ = y.clone().map(|(x, y, (z, (w,)))| (x, y, (z, (w,)))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:76:30 + --> tests/ui/map_identity.rs:77:30 | 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:86:15 + | +LL | let _ = it.map(|x| x).next(); + | ^^^^^^^^^^^ + | +help: remove the call to `map`, and make `it` mutable + | +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:93:23 + | +LL | let _ = subindex.0.map(|n| n).next(); + | ^^^^^^^^^^^ + | +help: remove the call to `map`, and make `subindex` mutable + | +LL ~ let mut subindex = (index.by_ref().take(3), 42); +LL ~ let _ = subindex.0.next(); + | + +error: unnecessary map of the identity function + --> tests/ui/map_identity.rs:100: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:106: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:119:30 | 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:131:26 + --> tests/ui/map_identity.rs:145:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:135:26 + --> tests/ui/map_identity.rs:149:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| foo::Foo { foo, bar }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:143:26 + --> tests/ui/map_identity.rs:157:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: foo, bar: bar }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:147:26 + --> tests/ui/map_identity.rs:161:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:157:26 + --> tests/ui/map_identity.rs:171:26 | LL | let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(foo, bar)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` error: unnecessary map of the identity function - --> tests/ui/map_identity.rs:161:26 + --> tests/ui/map_identity.rs:175:26 | LL | let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` -error: aborting due to 20 previous errors +error: aborting due to 22 previous errors