about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-06-07 21:27:46 +0000
committerbors <bors@rust-lang.org>2024-06-07 21:27:46 +0000
commitd553ebef573712d66470fa85e08d97a67375c243 (patch)
tree65e3a6adf4f1293d32e2582775d2c4dbba323983
parent1e407642e82e038f0d2f81aa71271649af23b097 (diff)
parent35b2aa99f3357a69d09f247ed5aff64cdd7765b1 (diff)
downloadrust-d553ebef573712d66470fa85e08d97a67375c243.tar.gz
rust-d553ebef573712d66470fa85e08d97a67375c243.zip
Auto merge of #12851 - samueltardieu:issue12846, r=y21
Add required parentheses around method receiver

Fix #12846

changelog: [`needless_bool`]: Add missing parentheses around method receiver
-rw-r--r--clippy_lints/src/methods/search_is_some.rs12
-rw-r--r--clippy_lints/src/needless_bool.rs13
-rw-r--r--clippy_utils/src/lib.rs11
-rw-r--r--tests/ui/needless_bool/fixable.fixed12
-rw-r--r--tests/ui/needless_bool/fixable.rs12
-rw-r--r--tests/ui/needless_bool/fixable.stderr20
6 files changed, 65 insertions, 15 deletions
diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs
index f5f1e94bbf4..3b2dd285b8c 100644
--- a/clippy_lints/src/methods/search_is_some.rs
+++ b/clippy_lints/src/methods/search_is_some.rs
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::sugg::deref_closure_args;
 use clippy_utils::ty::is_type_lang_item;
-use clippy_utils::{get_parent_expr, is_trait_method, strip_pat_refs};
+use clippy_utils::{is_receiver_of_method_call, is_trait_method, strip_pat_refs};
 use hir::ExprKind;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
@@ -156,13 +156,3 @@ pub(super) fn check<'tcx>(
         }
     }
 }
-
-fn is_receiver_of_method_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
-    if let Some(parent_expr) = get_parent_expr(cx, expr)
-        && let ExprKind::MethodCall(_, receiver, ..) = parent_expr.kind
-        && receiver.hir_id == expr.hir_id
-    {
-        return true;
-    }
-    false
-}
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index e1866eaa18a..9cb4fa41c73 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -6,8 +6,8 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{
-    higher, is_block_like, is_else_clause, is_expn_of, is_parent_stmt, peel_blocks, peel_blocks_with_stmt,
-    span_extract_comment, SpanlessEq,
+    get_parent_expr, higher, is_block_like, is_else_clause, is_expn_of, is_parent_stmt, is_receiver_of_method_call,
+    peel_blocks, peel_blocks_with_stmt, span_extract_comment, SpanlessEq,
 };
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -154,7 +154,10 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
                     snip = snip.blockify();
                 }
 
-                if condition_needs_parentheses(cond) && is_parent_stmt(cx, e.hir_id) {
+                if (condition_needs_parentheses(cond) && is_parent_stmt(cx, e.hir_id))
+                    || is_receiver_of_method_call(cx, e)
+                    || is_as_argument(cx, e)
+                {
                     snip = snip.maybe_par();
                 }
 
@@ -442,3 +445,7 @@ fn fetch_assign<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, bool)
         None
     }
 }
+
+fn is_as_argument(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
+    matches!(get_parent_expr(cx, e).map(|e| e.kind), Some(ExprKind::Cast(_, _)))
+}
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 49776bf5a5b..bd0911d5268 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -3401,3 +3401,14 @@ pub fn binary_expr_needs_parentheses(expr: &Expr<'_>) -> bool {
 
     contains_block(expr, false)
 }
+
+/// Returns true if the specified expression is in a receiver position.
+pub fn is_receiver_of_method_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    if let Some(parent_expr) = get_parent_expr(cx, expr)
+        && let ExprKind::MethodCall(_, receiver, ..) = parent_expr.kind
+        && receiver.hir_id == expr.hir_id
+    {
+        return true;
+    }
+    false
+}
diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed
index 3059de8f89c..ec63c4fd6a2 100644
--- a/tests/ui/needless_bool/fixable.fixed
+++ b/tests/ui/needless_bool/fixable.fixed
@@ -131,3 +131,15 @@ fn needless_bool_condition() -> bool {
 
     foo()
 }
+
+fn issue12846() {
+    let a = true;
+    let b = false;
+
+    // parentheses are needed here
+    let _x = (a && b).then(|| todo!());
+    let _x = (a && b) as u8;
+
+    // parentheses are not needed here
+    let _x = a.then(|| todo!());
+}
diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs
index b2cbe86e223..8694aa71590 100644
--- a/tests/ui/needless_bool/fixable.rs
+++ b/tests/ui/needless_bool/fixable.rs
@@ -191,3 +191,15 @@ fn needless_bool_condition() -> bool {
 
     foo()
 }
+
+fn issue12846() {
+    let a = true;
+    let b = false;
+
+    // parentheses are needed here
+    let _x = if a && b { true } else { false }.then(|| todo!());
+    let _x = if a && b { true } else { false } as u8;
+
+    // parentheses are not needed here
+    let _x = if a { true } else { false }.then(|| todo!());
+}
diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr
index 9746e931f50..99b5b998344 100644
--- a/tests/ui/needless_bool/fixable.stderr
+++ b/tests/ui/needless_bool/fixable.stderr
@@ -191,5 +191,23 @@ error: this if-then-else expression returns a bool literal
 LL |         if unsafe { no(4) } & 1 != 0 { true } else { false }
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)`
 
-error: aborting due to 21 previous errors
+error: this if-then-else expression returns a bool literal
+  --> tests/ui/needless_bool/fixable.rs:200:14
+   |
+LL |     let _x = if a && b { true } else { false }.then(|| todo!());
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(a && b)`
+
+error: this if-then-else expression returns a bool literal
+  --> tests/ui/needless_bool/fixable.rs:201:14
+   |
+LL |     let _x = if a && b { true } else { false } as u8;
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(a && b)`
+
+error: this if-then-else expression returns a bool literal
+  --> tests/ui/needless_bool/fixable.rs:204:14
+   |
+LL |     let _x = if a { true } else { false }.then(|| todo!());
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `a`
+
+error: aborting due to 24 previous errors