about summary refs log tree commit diff
path: root/clippy_lints/src/methods
diff options
context:
space:
mode:
authorBen Boeckel <mathstuf@gmail.com>2020-03-25 21:13:24 -0400
committerCameron Steffen <cam.steffen94@gmail.com>2021-03-14 16:31:55 -0500
commitecf0c76c369a80c1a2de61e25c94b7df5dc7d164 (patch)
tree9808234cbfa0fa90c64b47e708e592c9e9d06c87 /clippy_lints/src/methods
parent52c25e9136f533c350fa1916b5bf5103f69c0f4d (diff)
Fix suspicious_map false positives
Diffstat (limited to 'clippy_lints/src/methods')
-rw-r--r--clippy_lints/src/methods/mod.rs2
-rw-r--r--clippy_lints/src/methods/suspicious_map.rs45
2 files changed, 36 insertions, 11 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 7fd14c4f9b1..45e906cf468 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -1739,7 +1739,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 unnecessary_filter_map::check(cx, expr, arg_lists[0]);
                 filter_map_identity::check(cx, expr, arg_lists[0], method_spans[0]);
             },
-            ["count", "map"] => suspicious_map::check(cx, expr),
+            ["count", "map"] => suspicious_map::check(cx, expr, arg_lists[1], arg_lists[0]),
             ["assume_init"] => uninit_assumed_init::check(cx, &arg_lists[0][0], expr),
             ["unwrap_or", arith @ ("checked_add" | "checked_sub" | "checked_mul")] => {
                 manual_saturating_arithmetic::check(cx, expr, &arg_lists, &arith["checked_".len()..])
diff --git a/clippy_lints/src/methods/suspicious_map.rs b/clippy_lints/src/methods/suspicious_map.rs
index e135a826dc4..0ffa71de30c 100644
--- a/clippy_lints/src/methods/suspicious_map.rs
+++ b/clippy_lints/src/methods/suspicious_map.rs
@@ -1,16 +1,41 @@
-use crate::utils::span_lint_and_help;
+use crate::utils::usage::mutated_variables;
+use crate::utils::{expr_or_init, is_trait_method, span_lint_and_help};
+use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
+use rustc_span::sym;
 
 use super::SUSPICIOUS_MAP;
 
-pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
-    span_lint_and_help(
-        cx,
-        SUSPICIOUS_MAP,
-        expr.span,
-        "this call to `map()` won't have an effect on the call to `count()`",
-        None,
-        "make sure you did not confuse `map` with `filter` or `for_each`",
-    );
+pub fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &hir::Expr<'_>,
+    map_args: &[hir::Expr<'_>],
+    count_args: &[hir::Expr<'_>],
+) {
+    if_chain! {
+        if let [count_recv] = count_args;
+        if let [_, map_arg] = map_args;
+        if is_trait_method(cx, count_recv, sym::Iterator);
+        let closure = expr_or_init(cx, map_arg);
+        if let Some(body_id) = cx.tcx.hir().maybe_body_owned_by(closure.hir_id);
+        let closure_body = cx.tcx.hir().body(body_id);
+        if !cx.typeck_results().expr_ty(&closure_body.value).is_unit();
+        then {
+            if let Some(map_mutated_vars) = mutated_variables(&closure_body.value, cx) {
+                // A variable is used mutably inside of the closure. Suppress the lint.
+                if !map_mutated_vars.is_empty() {
+                    return;
+                }
+            }
+            span_lint_and_help(
+                cx,
+                SUSPICIOUS_MAP,
+                expr.span,
+                "this call to `map()` won't have an effect on the call to `count()`",
+                None,
+                "make sure you did not confuse `map` with `filter` or `for_each`",
+            );
+        }
+    }
 }