Skip to content

Issue4983 bool updates #5365

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions clippy_lints/src/needless_bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
//! This lint is **warn** by default

use crate::utils::sugg::Sugg;
use crate::utils::{higher, parent_node_is_if_expr, span_lint, span_lint_and_sugg};
use crate::utils::{higher, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, StmtKind};
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, StmtKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Spanned;
use rustc_span::Span;

declare_clippy_lint! {
/// **What it does:** Checks for expressions of the form `if c { true } else {
Expand Down Expand Up @@ -188,6 +190,34 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison {
}
}

struct ExpressionInfoWithSpan {
one_side_is_unary_not: bool,
left_span: Span,
right_span: Span,
}

fn is_unary_not(e: &Expr<'_>) -> (bool, Span) {
if_chain! {
if let ExprKind::Unary(unop, operand) = e.kind;
if let UnOp::UnNot = unop;
then {
return (true, operand.span);
}
};
(false, e.span)
}

fn one_side_is_unary_not<'tcx>(left_side: &'tcx Expr<'_>, right_side: &'tcx Expr<'_>) -> ExpressionInfoWithSpan {
let left = is_unary_not(left_side);
let right = is_unary_not(right_side);

ExpressionInfoWithSpan {
one_side_is_unary_not: left.0 != right.0,
left_span: left.1,
right_span: right.1,
}
}

fn check_comparison<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
e: &'tcx Expr<'_>,
Expand All @@ -199,10 +229,30 @@ fn check_comparison<'a, 'tcx>(
) {
use self::Expression::{Bool, Other};

if let ExprKind::Binary(_, ref left_side, ref right_side) = e.kind {
if let ExprKind::Binary(op, ref left_side, ref right_side) = e.kind {
let (l_ty, r_ty) = (cx.tables.expr_ty(left_side), cx.tables.expr_ty(right_side));
if l_ty.is_bool() && r_ty.is_bool() {
let mut applicability = Applicability::MachineApplicable;

if let BinOpKind::Eq = op.node {
let expression_info = one_side_is_unary_not(&left_side, &right_side);
if expression_info.one_side_is_unary_not {
span_lint_and_sugg(
cx,
BOOL_COMPARISON,
e.span,
"This comparison might be written more concisely",
"try simplifying it as shown",
format!(
"{} != {}",
snippet_with_applicability(cx, expression_info.left_span, "..", &mut applicability),
snippet_with_applicability(cx, expression_info.right_span, "..", &mut applicability)
),
applicability,
)
}
}

match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
(Bool(true), Other) => left_true.map_or((), |(h, m)| {
suggest_bool_comparison(cx, e, right_side, applicability, m, h)
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/bool_comparison.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,19 @@ fn issue3703() {
if Foo < false {}
if false < Foo {}
}

#[allow(dead_code)]
fn issue4983() {
let a = true;
let b = false;

if a != b {};
if a != b {};
if a == b {};
if !a == !b {};

if b != a {};
if b != a {};
if b == a {};
if !b == !a {};
}
16 changes: 16 additions & 0 deletions tests/ui/bool_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,19 @@ fn issue3703() {
if Foo < false {}
if false < Foo {}
}

#[allow(dead_code)]
fn issue4983() {
let a = true;
let b = false;

if a == !b {};
if !a == b {};
if a == b {};
if !a == !b {};

if b == !a {};
if !b == a {};
if b == a {};
if !b == !a {};
}
26 changes: 25 additions & 1 deletion tests/ui/bool_comparison.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,29 @@ error: order comparisons between booleans can be simplified
LL | if x > y {
| ^^^^^ help: try simplifying it as shown: `x & !y`

error: aborting due to 14 previous errors
error: This comparison might be written more concisely
--> $DIR/bool_comparison.rs:120:8
|
LL | if a == !b {};
| ^^^^^^^ help: try simplifying it as shown: `a != b`

error: This comparison might be written more concisely
--> $DIR/bool_comparison.rs:121:8
|
LL | if !a == b {};
| ^^^^^^^ help: try simplifying it as shown: `a != b`

error: This comparison might be written more concisely
--> $DIR/bool_comparison.rs:125:8
|
LL | if b == !a {};
| ^^^^^^^ help: try simplifying it as shown: `b != a`

error: This comparison might be written more concisely
--> $DIR/bool_comparison.rs:126:8
|
LL | if !b == a {};
| ^^^^^^^ help: try simplifying it as shown: `b != a`

error: aborting due to 18 previous errors