Handle Result on map_flatten lint

Adds a check on `.map(..).flatten()` on `Result` type that follows the behaviour on `Option` type.
rust-lang · Aug 1, 2021 · 69d4390 · 69d4390
1 parent 05fa78f
commit 69d4390
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 21 deletions.
diff --git a/clippy_lints/src/methods/map_flatten.rs b/clippy_lints/src/methods/map_flatten.rs
@@ -52,18 +52,32 @@ pub(super) fn check<'tcx>(
         );
     }
 
-    // lint if caller of `.map().flatten()` is an Option
-    if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::option_type) {
-        let func_snippet = snippet(cx, map_arg.span, "..");
-        let hint = format!(".and_then({})", func_snippet);
-        span_lint_and_sugg(
-            cx,
-            MAP_FLATTEN,
-            expr.span.with_lo(recv.span.hi()),
-            "called `map(..).flatten()` on an `Option`",
-            "try using `and_then` instead",
-            hint,
-            Applicability::MachineApplicable,
-        );
-    }
+    // 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, _) => {
+            if cx.tcx.is_diagnostic_item(sym::option_type, adt.did) {
+                "Option"
+            } else if cx.tcx.is_diagnostic_item(sym::result_type, adt.did) {
+                "Result"
+            } else {
+                return;
+            }
+        },
+        _ => {
+            return;
+        },
+    };
+
+    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,
+    );
 }
diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed
@@ -5,6 +5,7 @@
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::map_identity)]
 #![allow(clippy::unnecessary_wraps)]
+#![feature(result_flattening)]
 
 fn main() {
     // mapping to Option on Iterator
@@ -23,4 +24,7 @@ fn main() {
 
     // mapping to Option on Option
     let _: Option<_> = (Some(Some(1))).and_then(|x| x);
+
+    // mapping to Result on Result
+    let _: Result<_, &str> = (Ok(Ok(1))).and_then(|x| x);
 }
diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs
@@ -5,6 +5,7 @@
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::map_identity)]
 #![allow(clippy::unnecessary_wraps)]
+#![feature(result_flattening)]
 
 fn main() {
     // mapping to Option on Iterator
@@ -23,4 +24,7 @@ fn main() {
 
     // mapping to Option on Option
     let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
+
+    // mapping to Result on Result
+    let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten();
 }
diff --git a/tests/ui/map_flatten.stderr b/tests/ui/map_flatten.stderr
@@ -1,40 +1,46 @@
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:16:46
+  --> $DIR/map_flatten.rs:17:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)`
    |
    = note: `-D clippy::map-flatten` implied by `-D warnings`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:17:46
+  --> $DIR/map_flatten.rs:18:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:18:46
+  --> $DIR/map_flatten.rs:19:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:19:46
+  --> $DIR/map_flatten.rs:20:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:22:46
+  --> $DIR/map_flatten.rs:23:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)`
 
 error: called `map(..).flatten()` on an `Option`
-  --> $DIR/map_flatten.rs:25:39
+  --> $DIR/map_flatten.rs:26:39
    |
 LL |     let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
    |                                       ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)`
 
-error: aborting due to 6 previous errors
+error: called `map(..).flatten()` on an `Result`
+  --> $DIR/map_flatten.rs:29:41
+   |
+LL |     let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten();
+   |                                         ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)`
+
+error: aborting due to 7 previous errors