From c236c0fb5694948353b5fcbe46010cf04d5a7107 Mon Sep 17 00:00:00 2001
From: Michael Wright <mikerite@lavabit.com>
Date: Thu, 20 Aug 2020 06:34:48 +0200
Subject: [PATCH] Fix false positive in `PRECEDENCE` lint

Extend the lint to handle chains of methods combined with unary negation.

Closes #5924
---
 clippy_lints/src/precedence.rs | 59 +++++++++++++++++-----------------
 tests/ui/precedence.fixed      |  8 +++++
 tests/ui/precedence.rs         |  8 +++++
 tests/ui/precedence.stderr     | 20 +++++++++++-
 4 files changed, 65 insertions(+), 30 deletions(-)

diff --git a/clippy_lints/src/precedence.rs b/clippy_lints/src/precedence.rs
index 4797771e7bd..c9d18c3cb72 100644
--- a/clippy_lints/src/precedence.rs
+++ b/clippy_lints/src/precedence.rs
@@ -1,4 +1,5 @@
 use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
+use if_chain::if_chain;
 use rustc_ast::ast::{BinOpKind, Expr, ExprKind, LitKind, UnOp};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass};
@@ -102,36 +103,36 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
             }
         }
 
-        if let ExprKind::Unary(UnOp::Neg, ref rhs) = expr.kind {
-            if let ExprKind::MethodCall(ref path_segment, ref args, _) = rhs.kind {
+        if let ExprKind::Unary(UnOp::Neg, operand) = &expr.kind {
+            let mut arg = operand;
+
+            let mut all_odd = true;
+            while let ExprKind::MethodCall(path_segment, args, _) = &arg.kind {
                 let path_segment_str = path_segment.ident.name.as_str();
-                if let Some(slf) = args.first() {
-                    if let ExprKind::Lit(ref lit) = slf.kind {
-                        match lit.kind {
-                            LitKind::Int(..) | LitKind::Float(..) => {
-                                if ALLOWED_ODD_FUNCTIONS
-                                    .iter()
-                                    .any(|odd_function| **odd_function == *path_segment_str)
-                                {
-                                    return;
-                                }
-                                let mut applicability = Applicability::MachineApplicable;
-                                span_lint_and_sugg(
-                                    cx,
-                                    PRECEDENCE,
-                                    expr.span,
-                                    "unary minus has lower precedence than method call",
-                                    "consider adding parentheses to clarify your intent",
-                                    format!(
-                                        "-({})",
-                                        snippet_with_applicability(cx, rhs.span, "..", &mut applicability)
-                                    ),
-                                    applicability,
-                                );
-                            },
-                            _ => (),
-                        }
-                    }
+                all_odd &= ALLOWED_ODD_FUNCTIONS
+                    .iter()
+                    .any(|odd_function| **odd_function == *path_segment_str);
+                arg = args.first().expect("A method always has a receiver.");
+            }
+
+            if_chain! {
+                if !all_odd;
+                if let ExprKind::Lit(lit) = &arg.kind;
+                if let LitKind::Int(..) | LitKind::Float(..) = &lit.kind;
+                then {
+                    let mut applicability = Applicability::MachineApplicable;
+                    span_lint_and_sugg(
+                        cx,
+                        PRECEDENCE,
+                        expr.span,
+                        "unary minus has lower precedence than method call",
+                        "consider adding parentheses to clarify your intent",
+                        format!(
+                            "-({})",
+                            snippet_with_applicability(cx, operand.span, "..", &mut applicability)
+                        ),
+                        applicability,
+                    );
                 }
             }
         }
diff --git a/tests/ui/precedence.fixed b/tests/ui/precedence.fixed
index 4d284ae1319..163bd044c17 100644
--- a/tests/ui/precedence.fixed
+++ b/tests/ui/precedence.fixed
@@ -48,6 +48,14 @@ fn main() {
     let _ = -1f64.to_degrees();
     let _ = -1f64.to_radians();
 
+    // Chains containing any non-odd function should trigger (issue #5924)
+    let _ = -(1.0_f64.cos().cos());
+    let _ = -(1.0_f64.cos().sin());
+    let _ = -(1.0_f64.sin().cos());
+
+    // Chains of odd functions shouldn't trigger
+    let _ = -1f64.sin().sin();
+
     let b = 3;
     trip!(b * 8);
 }
diff --git a/tests/ui/precedence.rs b/tests/ui/precedence.rs
index 2d08e82f349..8c849e3209b 100644
--- a/tests/ui/precedence.rs
+++ b/tests/ui/precedence.rs
@@ -48,6 +48,14 @@ fn main() {
     let _ = -1f64.to_degrees();
     let _ = -1f64.to_radians();
 
+    // Chains containing any non-odd function should trigger (issue #5924)
+    let _ = -1.0_f64.cos().cos();
+    let _ = -1.0_f64.cos().sin();
+    let _ = -1.0_f64.sin().cos();
+
+    // Chains of odd functions shouldn't trigger
+    let _ = -1f64.sin().sin();
+
     let b = 3;
     trip!(b * 8);
 }
diff --git a/tests/ui/precedence.stderr b/tests/ui/precedence.stderr
index a2ed5392bfc..03d585b3975 100644
--- a/tests/ui/precedence.stderr
+++ b/tests/ui/precedence.stderr
@@ -54,5 +54,23 @@ error: unary minus has lower precedence than method call
 LL |     -1f32.abs();
    |     ^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1f32.abs())`
 
-error: aborting due to 9 previous errors
+error: unary minus has lower precedence than method call
+  --> $DIR/precedence.rs:52:13
+   |
+LL |     let _ = -1.0_f64.cos().cos();
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.cos().cos())`
+
+error: unary minus has lower precedence than method call
+  --> $DIR/precedence.rs:53:13
+   |
+LL |     let _ = -1.0_f64.cos().sin();
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.cos().sin())`
+
+error: unary minus has lower precedence than method call
+  --> $DIR/precedence.rs:54:13
+   |
+LL |     let _ = -1.0_f64.sin().cos();
+   |             ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.sin().cos())`
+
+error: aborting due to 12 previous errors
 
-- 
2.44.0