Skip to content

Commit

Permalink
Do not remove identity mapping if mandatory mutability would be lost (#…
Browse files Browse the repository at this point in the history
…13905)

Removing `.map(identity)` may result in invalid code if the receiver of
`map()` is an immutable binding, and the result of `map()` is used as
the receiver of a method call expecting a mutable reference.

Fix #13904

changelog: [`map_identity`]: do not lint if this would cause mandatory
mutability to be lost
  • Loading branch information
Alexendoo authored Jan 7, 2025
2 parents b7b69b1 + a9fe043 commit f5ca68f
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 3 deletions.
15 changes: 13 additions & 2 deletions clippy_lints/src/methods/map_identity.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_expr_untyped_identity_function, is_trait_method};
use clippy_utils::{is_expr_untyped_identity_function, is_trait_method, path_to_local};
use rustc_ast::BindingMode;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::{self as hir, Node, PatKind};
use rustc_lint::LateContext;
use rustc_span::{Span, sym};

Expand All @@ -24,6 +25,16 @@ pub(super) fn check(
&& is_expr_untyped_identity_function(cx, map_arg)
&& let Some(sugg_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;
}

span_lint_and_sugg(
cx,
MAP_IDENTITY,
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/map_identity.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,18 @@ fn issue11764() {
// no match ergonomics for `(i32, i32)`
let _ = x.iter().copied();
}

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
#[allow(unused_mut)]
let mut it = [1, 2, 3].into_iter();
let _ = it.next();

// lint
let it = [1, 2, 3].into_iter();
let _ = { it }.next();
}
15 changes: 15 additions & 0 deletions tests/ui/map_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,18 @@ fn issue11764() {
// no match ergonomics for `(i32, i32)`
let _ = x.iter().copied().map(|(x, y)| (x, y));
}

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
#[allow(unused_mut)]
let mut it = [1, 2, 3].into_iter();
let _ = it.map(|x| x).next();

// lint
let it = [1, 2, 3].into_iter();
let _ = { it }.map(|x| x).next();
}
14 changes: 13 additions & 1 deletion tests/ui/map_identity.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,17 @@ error: unnecessary map of the identity function
LL | let _ = x.iter().copied().map(|(x, y)| (x, y));
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`

error: aborting due to 11 previous errors
error: unnecessary map of the identity function
--> tests/ui/map_identity.rs:77: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:81:19
|
LL | let _ = { it }.map(|x| x).next();
| ^^^^^^^^^^^ help: remove the call to `map`

error: aborting due to 13 previous errors

0 comments on commit f5ca68f

Please sign in to comment.