Skip to content

Commit

Permalink
Add required parentheses around method receiver
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed May 25, 2024
1 parent 5aae5f6 commit c8e943b
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 15 deletions.
12 changes: 1 addition & 11 deletions clippy_lints/src/methods/search_is_some.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
8 changes: 5 additions & 3 deletions clippy_lints/src/needless_bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_else_clause, is_expn_of, is_parent_stmt, peel_blocks, peel_blocks_with_stmt, span_extract_comment,
SpanlessEq,
higher, 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;
Expand Down Expand Up @@ -161,7 +161,9 @@ 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)
{
snip = snip.maybe_par();
}

Expand Down
11 changes: 11 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3370,3 +3370,14 @@ pub fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool {
Node::Stmt(..) | Node::Block(Block { stmts: &[], .. })
)
}

/// 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
}
11 changes: 11 additions & 0 deletions tests/ui/needless_bool/fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,14 @@ fn needless_bool_condition() -> bool {

foo()
}

fn issue12846() {
let a = true;
let b = false;

// parentheses are needed here
let _x = (a && b).then(|| todo!());

// parentheses are not needed here
let _x = a.then(|| todo!());
}
11 changes: 11 additions & 0 deletions tests/ui/needless_bool/fixable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,14 @@ 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!());

// parentheses are not needed here
let _x = if a { true } else { false }.then(|| todo!());
}
14 changes: 13 additions & 1 deletion tests/ui/needless_bool/fixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -191,5 +191,17 @@ 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:203:14
|
LL | let _x = if a { true } else { false }.then(|| todo!());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `a`

error: aborting due to 23 previous errors

0 comments on commit c8e943b

Please sign in to comment.