about summary refs log tree commit diff
path: root/clippy_lints
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-03-21 19:51:56 +0000
committerbors <bors@rust-lang.org>2022-03-21 19:51:56 +0000
commitff0a3687d8ade4d621e50f3765d7e3034491096b (patch)
tree368fa29c84b1e49f962015a23263ce6cf575579f /clippy_lints
parent47b93ea5ef65d7beab630f0975b96c872610ccd2 (diff)
parent5b6295d663fe5f2cc825f8a74578b0afab58c560 (diff)
downloadrust-ff0a3687d8ade4d621e50f3765d7e3034491096b.tar.gz
rust-ff0a3687d8ade4d621e50f3765d7e3034491096b.zip
Auto merge of #8520 - J-ZhengLi:issue8506, r=xFrednet
fix suggestion on `[map_flatten]` being cropped causing possible information loss

fixes #8506

Multi-line suggestion given by the lint is missing its bottom part, which could potentially contains useful information about the fix.

---

changelog: [`map_flatten`]: Long suggestions will now be splitup into two help messages
Diffstat (limited to 'clippy_lints')
-rw-r--r--clippy_lints/src/methods/map_flatten.rs118
-rw-r--r--clippy_lints/src/methods/mod.rs2
-rw-r--r--clippy_lints/src/utils/internal_lints/metadata_collector.rs3
3 files changed, 57 insertions, 66 deletions
diff --git a/clippy_lints/src/methods/map_flatten.rs b/clippy_lints/src/methods/map_flatten.rs
index e1212c31cfb..f447940ea3b 100644
--- a/clippy_lints/src/methods/map_flatten.rs
+++ b/clippy_lints/src/methods/map_flatten.rs
@@ -1,83 +1,73 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_sugg_for_edges;
 use clippy_utils::is_trait_method;
-use clippy_utils::source::snippet;
+use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::ty::is_type_diagnostic_item;
 use rustc_errors::Applicability;
-use rustc_hir as hir;
+use rustc_hir::Expr;
 use rustc_lint::LateContext;
 use rustc_middle::ty;
-use rustc_span::symbol::sym;
+use rustc_span::{symbol::sym, Span};
 
 use super::MAP_FLATTEN;
 
 /// lint use of `map().flatten()` for `Iterators` and 'Options'
-pub(super) fn check<'tcx>(
-    cx: &LateContext<'tcx>,
-    expr: &'tcx hir::Expr<'_>,
-    recv: &'tcx hir::Expr<'_>,
-    map_arg: &'tcx hir::Expr<'_>,
-) {
-    // lint if caller of `.map().flatten()` is an Iterator
-    if is_trait_method(cx, expr, sym::Iterator) {
-        let map_closure_ty = cx.typeck_results().expr_ty(map_arg);
-        let is_map_to_option = match map_closure_ty.kind() {
-            ty::Closure(_, _) | ty::FnDef(_, _) | ty::FnPtr(_) => {
-                let map_closure_sig = match map_closure_ty.kind() {
-                    ty::Closure(_, substs) => substs.as_closure().sig(),
-                    _ => map_closure_ty.fn_sig(cx.tcx),
-                };
-                let map_closure_return_ty = cx.tcx.erase_late_bound_regions(map_closure_sig.output());
-                is_type_diagnostic_item(cx, map_closure_return_ty, sym::Option)
-            },
-            _ => false,
-        };
-
-        let method_to_use = if is_map_to_option {
-            // `(...).map(...)` has type `impl Iterator<Item=Option<...>>
-            "filter_map"
-        } else {
-            // `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>>
-            "flat_map"
-        };
-        let func_snippet = snippet(cx, map_arg.span, "..");
-        let hint = format!(".{0}({1})", method_to_use, func_snippet);
-        span_lint_and_sugg(
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) {
+    if let Some((caller_ty_name, method_to_use)) = try_get_caller_ty_name_and_method_name(cx, expr, recv, map_arg) {
+        let mut applicability = Applicability::MachineApplicable;
+        let help_msgs = [
+            &format!("try replacing `map` with `{}`", method_to_use),
+            "and remove the `.flatten()`",
+        ];
+        let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability);
+        span_lint_and_sugg_for_edges(
             cx,
             MAP_FLATTEN,
-            expr.span.with_lo(recv.span.hi()),
-            "called `map(..).flatten()` on an `Iterator`",
-            &format!("try using `{}` instead", method_to_use),
-            hint,
-            Applicability::MachineApplicable,
+            expr.span.with_lo(map_span.lo()),
+            &format!("called `map(..).flatten()` on `{}`", caller_ty_name),
+            &help_msgs,
+            format!("{}({})", method_to_use, closure_snippet),
+            applicability,
         );
     }
+}
 
-    // lint if caller of `.map().flatten()` is an Option or Result
-    let caller_type = match cx.typeck_results().expr_ty(recv).kind() {
-        ty::Adt(adt, _) => {
+fn try_get_caller_ty_name_and_method_name(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    caller_expr: &Expr<'_>,
+    map_arg: &Expr<'_>,
+) -> Option<(&'static str, &'static str)> {
+    if is_trait_method(cx, expr, sym::Iterator) {
+        if is_map_to_option(cx, map_arg) {
+            // `(...).map(...)` has type `impl Iterator<Item=Option<...>>
+            Some(("Iterator", "filter_map"))
+        } else {
+            // `(...).map(...)` has type `impl Iterator<Item=impl Iterator<...>>
+            Some(("Iterator", "flat_map"))
+        }
+    } else {
+        if let ty::Adt(adt, _) = cx.typeck_results().expr_ty(caller_expr).kind() {
             if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) {
-                "Option"
+                return Some(("Option", "and_then"));
             } else if cx.tcx.is_diagnostic_item(sym::Result, adt.did()) {
-                "Result"
-            } else {
-                return;
+                return Some(("Result", "and_then"));
             }
-        },
-        _ => {
-            return;
-        },
-    };
+        }
+        None
+    }
+}
 
-    let func_snippet = snippet(cx, map_arg.span, "..");
-    let hint = format!(".and_then({})", func_snippet);
-    let lint_info = format!("called `map(..).flatten()` on an `{}`", caller_type);
-    span_lint_and_sugg(
-        cx,
-        MAP_FLATTEN,
-        expr.span.with_lo(recv.span.hi()),
-        &lint_info,
-        "try using `and_then` instead",
-        hint,
-        Applicability::MachineApplicable,
-    );
+fn is_map_to_option(cx: &LateContext<'_>, map_arg: &Expr<'_>) -> bool {
+    let map_closure_ty = cx.typeck_results().expr_ty(map_arg);
+    match map_closure_ty.kind() {
+        ty::Closure(_, _) | ty::FnDef(_, _) | ty::FnPtr(_) => {
+            let map_closure_sig = match map_closure_ty.kind() {
+                ty::Closure(_, substs) => substs.as_closure().sig(),
+                _ => map_closure_ty.fn_sig(cx.tcx),
+            };
+            let map_closure_return_ty = cx.tcx.erase_late_bound_regions(map_closure_sig.output());
+            is_type_diagnostic_item(cx, map_closure_return_ty, sym::Option)
+        },
+        _ => false,
+    }
 }
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index aa9f86f292c..822f40179ee 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -2377,7 +2377,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                 flat_map_option::check(cx, expr, arg, span);
             },
             (name @ "flatten", args @ []) => match method_call(recv) {
-                Some(("map", [recv, map_arg], _)) => map_flatten::check(cx, expr, recv, map_arg),
+                Some(("map", [recv, map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
                 Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
                 _ => {},
             },
diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs
index a617422bbeb..b3fad6ce7b6 100644
--- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs
+++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs
@@ -85,7 +85,7 @@ macro_rules! CONFIGURATION_VALUE_TEMPLATE {
     };
 }
 
-const LINT_EMISSION_FUNCTIONS: [&[&str]; 7] = [
+const LINT_EMISSION_FUNCTIONS: [&[&str]; 8] = [
     &["clippy_utils", "diagnostics", "span_lint"],
     &["clippy_utils", "diagnostics", "span_lint_and_help"],
     &["clippy_utils", "diagnostics", "span_lint_and_note"],
@@ -93,6 +93,7 @@ const LINT_EMISSION_FUNCTIONS: [&[&str]; 7] = [
     &["clippy_utils", "diagnostics", "span_lint_and_sugg"],
     &["clippy_utils", "diagnostics", "span_lint_and_then"],
     &["clippy_utils", "diagnostics", "span_lint_hir_and_then"],
+    &["clippy_utils", "diagnostics", "span_lint_and_sugg_for_edges"],
 ];
 const SUGGESTION_DIAGNOSTIC_BUILDER_METHODS: [(&str, bool); 9] = [
     ("span_suggestion", false),