Lint precedence possible ambiguity between closure and method call

`|x: String| {x}.clone()` applies `clone()` to {x}, while
`|x: String| -> String {x}.clone()` applies `.clone()` to
the closure because a closure with an explicit return type
requires a block.

This extends the `precedence` lint to suggest using parentheses
around the closure definition when the closure would be cloned,
as in `(|x: String| -> String {x}).clone()`.

Author: samueltardieu

clippy_lints/src/precedence.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::snippet_with_applicability;
 use rustc_ast::ast::BinOpKind::{Add, BitAnd, BitOr, BitXor, Div, Mul, Rem, Shl, Shr, Sub};
 use rustc_ast::ast::{BinOpKind, Expr, ExprKind};
@@ -10,7 +10,8 @@ use rustc_span::source_map::Spanned;
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for operations where precedence may be unclear and suggests to add parentheses.
-    /// It catches a mixed usage of arithmetic and bit shifting/combining operators without parentheses
+    /// It catches a mixed usage of arithmetic and bit shifting/combining operators,
+    /// as well as method calls applied to closures.
     ///
     /// ### Why is this bad?
     /// Not everyone knows the precedence of those operators by
@@ -109,6 +110,19 @@ impl EarlyLintPass for Precedence {
                 },
                 _ => (),
             }
+        } else if let ExprKind::MethodCall(method_call) = &expr.kind
+            && let ExprKind::Closure(closure) = &method_call.receiver.kind
+        {
+            span_lint_and_then(cx, PRECEDENCE, expr.span, "precedence might not be obvious", |diag| {
+                diag.multipart_suggestion(
+                    "consider parenthesizing the closure",
+                    vec![
+                        (closure.fn_decl_span.shrink_to_lo(), String::from("(")),
+                        (closure.body.span.shrink_to_hi(), String::from(")")),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+            });
         }
     }
 }

tests/ui/precedence.fixed

@@ -1,7 +1,12 @@
 #![warn(clippy::precedence)]
-#![allow(unused_must_use, clippy::no_effect, clippy::unnecessary_operation)]
-#![allow(clippy::identity_op)]
-#![allow(clippy::eq_op)]
+#![allow(
+    unused_must_use,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::clone_on_copy,
+    clippy::identity_op,
+    clippy::eq_op
+)]
 
 macro_rules! trip {
     ($a:expr) => {
@@ -35,3 +40,24 @@ fn main() {
     let b = 3;
     trip!(b * 8);
 }
+
+struct W(u8);
+impl Clone for W {
+    fn clone(&self) -> Self {
+        W(1)
+    }
+}
+
+fn closure_method_call() {
+    // Do not lint when the method call is applied to the block, both inside the closure
+    let f = |x: W| { x }.clone();
+    assert!(matches!(f(W(0)), W(1)));
+
+    let f = (|x: W| -> _ { x }).clone();
+    assert!(matches!(f(W(0)), W(0)));
+    //~^^ precedence
+
+    let f = (move |x: W| -> _ { x }).clone();
+    assert!(matches!(f(W(0)), W(0)));
+    //~^^ precedence
+}

tests/ui/precedence.rs

@@ -1,7 +1,12 @@
 #![warn(clippy::precedence)]
-#![allow(unused_must_use, clippy::no_effect, clippy::unnecessary_operation)]
-#![allow(clippy::identity_op)]
-#![allow(clippy::eq_op)]
+#![allow(
+    unused_must_use,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::clone_on_copy,
+    clippy::identity_op,
+    clippy::eq_op
+)]
 
 macro_rules! trip {
     ($a:expr) => {
@@ -35,3 +40,24 @@ fn main() {
     let b = 3;
     trip!(b * 8);
 }
+
+struct W(u8);
+impl Clone for W {
+    fn clone(&self) -> Self {
+        W(1)
+    }
+}
+
+fn closure_method_call() {
+    // Do not lint when the method call is applied to the block, both inside the closure
+    let f = |x: W| { x }.clone();
+    assert!(matches!(f(W(0)), W(1)));
+
+    let f = |x: W| -> _ { x }.clone();
+    assert!(matches!(f(W(0)), W(0)));
+    //~^^ precedence
+
+    let f = move |x: W| -> _ { x }.clone();
+    assert!(matches!(f(W(0)), W(0)));
+    //~^^ precedence
+}

tests/ui/precedence.stderr

@@ -1,5 +1,5 @@
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:16:5
+  --> tests/ui/precedence.rs:21:5
    |
 LL |     1 << 2 + 3;
    |     ^^^^^^^^^^ help: consider parenthesizing your expression: `1 << (2 + 3)`
@@ -8,40 +8,62 @@ LL |     1 << 2 + 3;
    = help: to override `-D warnings` add `#[allow(clippy::precedence)]`
 
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:18:5
+  --> tests/ui/precedence.rs:23:5
    |
 LL |     1 + 2 << 3;
    |     ^^^^^^^^^^ help: consider parenthesizing your expression: `(1 + 2) << 3`
 
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:20:5
+  --> tests/ui/precedence.rs:25:5
    |
 LL |     4 >> 1 + 1;
    |     ^^^^^^^^^^ help: consider parenthesizing your expression: `4 >> (1 + 1)`
 
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:22:5
+  --> tests/ui/precedence.rs:27:5
    |
 LL |     1 + 3 >> 2;
    |     ^^^^^^^^^^ help: consider parenthesizing your expression: `(1 + 3) >> 2`
 
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:24:5
+  --> tests/ui/precedence.rs:29:5
    |
 LL |     1 ^ 1 - 1;
    |     ^^^^^^^^^ help: consider parenthesizing your expression: `1 ^ (1 - 1)`
 
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:26:5
+  --> tests/ui/precedence.rs:31:5
    |
 LL |     3 | 2 - 1;
    |     ^^^^^^^^^ help: consider parenthesizing your expression: `3 | (2 - 1)`
 
 error: operator precedence might not be obvious
-  --> tests/ui/precedence.rs:28:5
+  --> tests/ui/precedence.rs:33:5
    |
 LL |     3 & 5 - 2;
    |     ^^^^^^^^^ help: consider parenthesizing your expression: `3 & (5 - 2)`
 
-error: aborting due to 7 previous errors
+error: precedence might not be obvious
+  --> tests/ui/precedence.rs:56:13
+   |
+LL |     let f = |x: W| -> _ { x }.clone();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider parenthesizing the closure
+   |
+LL |     let f = (|x: W| -> _ { x }).clone();
+   |             +                 +
+
+error: precedence might not be obvious
+  --> tests/ui/precedence.rs:60:13
+   |
+LL |     let f = move |x: W| -> _ { x }.clone();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider parenthesizing the closure
+   |
+LL |     let f = (move |x: W| -> _ { x }).clone();
+   |             +                      +
+
+error: aborting due to 9 previous errors