Better handle blocks with just one expression

6 files changed, 33 insertions, 12 deletions

diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs
index 247b5e270f8..27c548bed9f 100644
--- a/clippy_lints/src/strings.rs
+++ b/clippy_lints/src/strings.rs
@@ -14,6 +14,8 @@ use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
 use rustc_span::sym;
 
+use std::ops::ControlFlow;
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for string appends of the form `x = x + y` (without
@@ -454,13 +456,21 @@ fn is_parent_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rustc_spa
 }
 
 fn is_called_from_map_like(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<rustc_span::Span> {
-    let parent = get_parent_expr(cx, expr)?;
-
-    if matches!(parent.kind, ExprKind::Closure(_)) {
-        is_parent_map_like(cx, parent)
-    } else {
-        None
-    }
+    // Look for a closure as parent of `expr`, discarding simple blocks
+    let parent_closure = cx
+        .tcx
+        .hir_parent_iter(expr.hir_id)
+        .try_fold(expr.hir_id, |child_hir_id, (_, node)| match node {
+            // Check that the child expression is the only expression in the block
+            Node::Block(block) if block.stmts.is_empty() && block.expr.map(|e| e.hir_id) == Some(child_hir_id) => {
+                ControlFlow::Continue(block.hir_id)
+            },
+            Node::Expr(expr) if matches!(expr.kind, ExprKind::Block(..)) => ControlFlow::Continue(expr.hir_id),
+            Node::Expr(expr) if matches!(expr.kind, ExprKind::Closure(_)) => ControlFlow::Break(Some(expr)),
+            _ => ControlFlow::Break(None),
+        })
+        .break_value()?;
+    is_parent_map_like(cx, parent_closure?)
 }
 
 fn suggest_cloned_string_to_string(cx: &LateContext<'_>, span: rustc_span::Span) {
diff --git a/tests/ui/string_to_string.rs b/tests/ui/string_to_string.rs
index 8c97d8c16f9..7c5bd8a897b 100644
--- a/tests/ui/string_to_string.rs
+++ b/tests/ui/string_to_string.rs
@@ -15,7 +15,6 @@ fn main() {
     });
     //~^^ string_to_string
 
-    // Should not lint.
     let x = Some(String::new());
     let _ = x.unwrap_or_else(|| v.to_string());
     //~^ string_to_string
diff --git a/tests/ui/string_to_string.stderr b/tests/ui/string_to_string.stderr
index c0e2e75f51a..99eea06f18e 100644
--- a/tests/ui/string_to_string.stderr
+++ b/tests/ui/string_to_string.stderr
@@ -17,7 +17,7 @@ LL |         x.to_string()
    = help: consider using `.clone()`
 
 error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string.rs:20:33
+  --> tests/ui/string_to_string.rs:19:33
    |
 LL |     let _ = x.unwrap_or_else(|| v.to_string());
    |                                 ^^^^^^^^^^^^^
diff --git a/tests/ui/string_to_string_in_map.fixed b/tests/ui/string_to_string_in_map.fixed
index b004fd736ad..efc085539f1 100644
--- a/tests/ui/string_to_string_in_map.fixed
+++ b/tests/ui/string_to_string_in_map.fixed
@@ -9,6 +9,9 @@ fn main() {
     //~^ string_to_string
     let _ = variable2.cloned();
     //~^ string_to_string
+    #[rustfmt::skip]
+    let _ = variable2.cloned();
+    //~^ string_to_string
 
     let _ = vec![String::new()].iter().cloned().collect::<Vec<_>>();
     //~^ string_to_string
diff --git a/tests/ui/string_to_string_in_map.rs b/tests/ui/string_to_string_in_map.rs
index 387abe84205..5bf1d7ba5a2 100644
--- a/tests/ui/string_to_string_in_map.rs
+++ b/tests/ui/string_to_string_in_map.rs
@@ -9,6 +9,9 @@ fn main() {
     //~^ string_to_string
     let _ = variable2.map(|x| x.to_string());
     //~^ string_to_string
+    #[rustfmt::skip]
+    let _ = variable2.map(|x| { x.to_string() });
+    //~^ string_to_string
 
     let _ = vec![String::new()].iter().map(String::to_string).collect::<Vec<_>>();
     //~^ string_to_string
diff --git a/tests/ui/string_to_string_in_map.stderr b/tests/ui/string_to_string_in_map.stderr
index 44e2119c100..35aeed656ee 100644
--- a/tests/ui/string_to_string_in_map.stderr
+++ b/tests/ui/string_to_string_in_map.stderr
@@ -17,16 +17,22 @@ LL |     let _ = variable2.map(|x| x.to_string());
    |                       ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
 
 error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:13:40
+  --> tests/ui/string_to_string_in_map.rs:13:23
+   |
+LL |     let _ = variable2.map(|x| { x.to_string() });
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
+
+error: `to_string()` called on a `String`
+  --> tests/ui/string_to_string_in_map.rs:16:40
    |
 LL |     let _ = vec![String::new()].iter().map(String::to_string).collect::<Vec<_>>();
    |                                        ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
 
 error: `to_string()` called on a `String`
-  --> tests/ui/string_to_string_in_map.rs:15:40
+  --> tests/ui/string_to_string_in_map.rs:18:40
    |
 LL |     let _ = vec![String::new()].iter().map(|x| x.to_string()).collect::<Vec<_>>();
    |                                        ^^^^^^^^^^^^^^^^^^^^^^ help: try: `cloned()`
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors