rust-clippy/clippy_lints/src/booleans.rs

use crate::utils::{
    get_trait_def_id, implements_trait, in_macro, match_type, paths, snippet_opt, span_lint_and_sugg,
    span_lint_and_then, SpanlessEq,
};
use if_chain::if_chain;
use rustc::declare_lint_pass;
use rustc::hir::intravisit;
use rustc::hir::intravisit::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc_errors::Applicability;
use rustc_hir::*;
use rustc_session::declare_tool_lint;
use rustc_span::source_map::Span;
use syntax::ast::LitKind;

declare_clippy_lint! {
    /// **What it does:** Checks for boolean expressions that can be written more
    /// concisely.
    ///
    /// **Why is this bad?** Readability of boolean expressions suffers from
    /// unnecessary duplication.
    ///
    /// **Known problems:** Ignores short circuiting behavior of `||` and
    /// `&&`. Ignores `|`, `&` and `^`.
    ///
    /// **Example:**
    /// ```ignore
    /// if a && true  // should be: if a
    /// if !(a == b)  // should be: if a != b
    /// ```
    pub NONMINIMAL_BOOL,
    complexity,
    "boolean expressions that can be written more concisely"
}

declare_clippy_lint! {
    /// **What it does:** Checks for boolean expressions that contain terminals that
    /// can be eliminated.
    ///
    /// **Why is this bad?** This is most likely a logic bug.
    ///
    /// **Known problems:** Ignores short circuiting behavior.
    ///
    /// **Example:**
    /// ```ignore
    /// if a && b || a { ... }
    /// ```
    /// The `b` is unnecessary, the expression is equivalent to `if a`.
    pub LOGIC_BUG,
    correctness,
    "boolean expressions that contain terminals which can be eliminated"
}

// For each pairs, both orders are considered.
const METHODS_WITH_NEGATION: [(&str, &str); 2] = [("is_some", "is_none"), ("is_err", "is_ok")];

declare_lint_pass!(NonminimalBool => [NONMINIMAL_BOOL, LOGIC_BUG]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonminimalBool {
    fn check_fn(
        &mut self,
        cx: &LateContext<'a, 'tcx>,
        _: intravisit::FnKind<'tcx>,
        _: &'tcx FnDecl<'_>,
        body: &'tcx Body<'_>,
        _: Span,
        _: HirId,
    ) {
        NonminimalBoolVisitor { cx }.visit_body(body)
    }
}

struct NonminimalBoolVisitor<'a, 'tcx> {
    cx: &'a LateContext<'a, 'tcx>,
}

use quine_mc_cluskey::Bool;
struct Hir2Qmm<'a, 'tcx, 'v> {
    terminals: Vec<&'v Expr<'v>>,
    cx: &'a LateContext<'a, 'tcx>,
}

impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
    fn extract(&mut self, op: BinOpKind, a: &[&'v Expr<'_>], mut v: Vec<Bool>) -> Result<Vec<Bool>, String> {
        for a in a {
            if let ExprKind::Binary(binop, lhs, rhs) = &a.kind {
                if binop.node == op {
                    v = self.extract(op, &[lhs, rhs], v)?;
                    continue;
                }
            }
            v.push(self.run(a)?);
        }
        Ok(v)
    }

    fn run(&mut self, e: &'v Expr<'_>) -> Result<Bool, String> {
        fn negate(bin_op_kind: BinOpKind) -> Option<BinOpKind> {
            match bin_op_kind {
                BinOpKind::Eq => Some(BinOpKind::Ne),
                BinOpKind::Ne => Some(BinOpKind::Eq),
                BinOpKind::Gt => Some(BinOpKind::Le),
                BinOpKind::Ge => Some(BinOpKind::Lt),
                BinOpKind::Lt => Some(BinOpKind::Ge),
                BinOpKind::Le => Some(BinOpKind::Gt),
                _ => None,
            }
        }

        // prevent folding of `cfg!` macros and the like
        if !e.span.from_expansion() {
            match &e.kind {
                ExprKind::Unary(UnOp::UnNot, inner) => return Ok(Bool::Not(box self.run(inner)?)),
                ExprKind::Binary(binop, lhs, rhs) => match &binop.node {
                    BinOpKind::Or => return Ok(Bool::Or(self.extract(BinOpKind::Or, &[lhs, rhs], Vec::new())?)),
                    BinOpKind::And => return Ok(Bool::And(self.extract(BinOpKind::And, &[lhs, rhs], Vec::new())?)),
                    _ => (),
                },
                ExprKind::Lit(lit) => match lit.node {
                    LitKind::Bool(true) => return Ok(Bool::True),
                    LitKind::Bool(false) => return Ok(Bool::False),
                    _ => (),
                },
                _ => (),
            }
        }
        for (n, expr) in self.terminals.iter().enumerate() {
            if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e, expr) {
                #[allow(clippy::cast_possible_truncation)]
                return Ok(Bool::Term(n as u8));
            }

            if_chain! {
                if let ExprKind::Binary(e_binop, e_lhs, e_rhs) = &e.kind;
                if implements_ord(self.cx, e_lhs);
                if let ExprKind::Binary(expr_binop, expr_lhs, expr_rhs) = &expr.kind;
                if negate(e_binop.node) == Some(expr_binop.node);
                if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e_lhs, expr_lhs);
                if SpanlessEq::new(self.cx).ignore_fn().eq_expr(e_rhs, expr_rhs);
                then {
                    #[allow(clippy::cast_possible_truncation)]
                    return Ok(Bool::Not(Box::new(Bool::Term(n as u8))));
                }
            }
        }
        let n = self.terminals.len();
        self.terminals.push(e);
        if n < 32 {
            #[allow(clippy::cast_possible_truncation)]
            Ok(Bool::Term(n as u8))
        } else {
            Err("too many literals".to_owned())
        }
    }
}

struct SuggestContext<'a, 'tcx, 'v> {
    terminals: &'v [&'v Expr<'v>],
    cx: &'a LateContext<'a, 'tcx>,
    output: String,
}

impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
    fn recurse(&mut self, suggestion: &Bool) -> Option<()> {
        use quine_mc_cluskey::Bool::*;
        match suggestion {
            True => {
                self.output.push_str("true");
            },
            False => {
                self.output.push_str("false");
            },
            Not(inner) => match **inner {
                And(_) | Or(_) => {
                    self.output.push('!');
                    self.output.push('(');
                    self.recurse(inner);
                    self.output.push(')');
                },
                Term(n) => {
                    let terminal = self.terminals[n as usize];
                    if let Some(str) = simplify_not(self.cx, terminal) {
                        self.output.push_str(&str)
                    } else {
                        self.output.push('!');
                        let snip = snippet_opt(self.cx, terminal.span)?;
                        self.output.push_str(&snip);
                    }
                },
                True | False | Not(_) => {
                    self.output.push('!');
                    self.recurse(inner)?;
                },
            },
            And(v) => {
                for (index, inner) in v.iter().enumerate() {
                    if index > 0 {
                        self.output.push_str(" && ");
                    }
                    if let Or(_) = *inner {
                        self.output.push('(');
                        self.recurse(inner);
                        self.output.push(')');
                    } else {
                        self.recurse(inner);
                    }
                }
            },
            Or(v) => {
                for (index, inner) in v.iter().enumerate() {
                    if index > 0 {
                        self.output.push_str(" || ");
                    }
                    self.recurse(inner);
                }
            },
            &Term(n) => {
                let snip = snippet_opt(self.cx, self.terminals[n as usize].span)?;
                self.output.push_str(&snip);
            },
        }
        Some(())
    }
}

fn simplify_not(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option<String> {
    match &expr.kind {
        ExprKind::Binary(binop, lhs, rhs) => {
            if !implements_ord(cx, lhs) {
                return None;
            }

            match binop.node {
                BinOpKind::Eq => Some(" != "),
                BinOpKind::Ne => Some(" == "),
                BinOpKind::Lt => Some(" >= "),
                BinOpKind::Gt => Some(" <= "),
                BinOpKind::Le => Some(" > "),
                BinOpKind::Ge => Some(" < "),
                _ => None,
            }
            .and_then(|op| {
                Some(format!(
                    "{}{}{}",
                    snippet_opt(cx, lhs.span)?,
                    op,
                    snippet_opt(cx, rhs.span)?
                ))
            })
        },
        ExprKind::MethodCall(path, _, args) if args.len() == 1 => {
            let type_of_receiver = cx.tables.expr_ty(&args[0]);
            if !match_type(cx, type_of_receiver, &paths::OPTION) && !match_type(cx, type_of_receiver, &paths::RESULT) {
                return None;
            }
            METHODS_WITH_NEGATION
                .iter()
                .cloned()
                .flat_map(|(a, b)| vec![(a, b), (b, a)])
                .find(|&(a, _)| {
                    let path: &str = &path.ident.name.as_str();
                    a == path
                })
                .and_then(|(_, neg_method)| Some(format!("{}.{}()", snippet_opt(cx, args[0].span)?, neg_method)))
        },
        _ => None,
    }
}

fn suggest(cx: &LateContext<'_, '_>, suggestion: &Bool, terminals: &[&Expr<'_>]) -> String {
    let mut suggest_context = SuggestContext {
        terminals,
        cx,
        output: String::new(),
    };
    suggest_context.recurse(suggestion);
    suggest_context.output
}

fn simple_negate(b: Bool) -> Bool {
    use quine_mc_cluskey::Bool::*;
    match b {
        True => False,
        False => True,
        t @ Term(_) => Not(Box::new(t)),
        And(mut v) => {
            for el in &mut v {
                *el = simple_negate(::std::mem::replace(el, True));
            }
            Or(v)
        },
        Or(mut v) => {
            for el in &mut v {
                *el = simple_negate(::std::mem::replace(el, True));
            }
            And(v)
        },
        Not(inner) => *inner,
    }
}

#[derive(Default)]
struct Stats {
    terminals: [usize; 32],
    negations: usize,
    ops: usize,
}

fn terminal_stats(b: &Bool) -> Stats {
    fn recurse(b: &Bool, stats: &mut Stats) {
        match b {
            True | False => stats.ops += 1,
            Not(inner) => {
                match **inner {
                    And(_) | Or(_) => stats.ops += 1, // brackets are also operations
                    _ => stats.negations += 1,
                }
                recurse(inner, stats);
            },
            And(v) | Or(v) => {
                stats.ops += v.len() - 1;
                for inner in v {
                    recurse(inner, stats);
                }
            },
            &Term(n) => stats.terminals[n as usize] += 1,
        }
    }
    use quine_mc_cluskey::Bool::*;
    let mut stats = Stats::default();
    recurse(b, &mut stats);
    stats
}

impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
    fn bool_expr(&self, e: &'tcx Expr<'_>) {
        let mut h2q = Hir2Qmm {
            terminals: Vec::new(),
            cx: self.cx,
        };
        if let Ok(expr) = h2q.run(e) {
            if h2q.terminals.len() > 8 {
                // QMC has exponentially slow behavior as the number of terminals increases
                // 8 is reasonable, it takes approximately 0.2 seconds.
                // See #825
                return;
            }

            let stats = terminal_stats(&expr);
            let mut simplified = expr.simplify();
            for simple in Bool::Not(Box::new(expr)).simplify() {
                match simple {
                    Bool::Not(_) | Bool::True | Bool::False => {},
                    _ => simplified.push(Bool::Not(Box::new(simple.clone()))),
                }
                let simple_negated = simple_negate(simple);
                if simplified.iter().any(|s| *s == simple_negated) {
                    continue;
                }
                simplified.push(simple_negated);
            }
            let mut improvements = Vec::new();
            'simplified: for suggestion in &simplified {
                let simplified_stats = terminal_stats(suggestion);
                let mut improvement = false;
                for i in 0..32 {
                    // ignore any "simplifications" that end up requiring a terminal more often
                    // than in the original expression
                    if stats.terminals[i] < simplified_stats.terminals[i] {
                        continue 'simplified;
                    }
                    if stats.terminals[i] != 0 && simplified_stats.terminals[i] == 0 {
                        span_lint_and_then(
                            self.cx,
                            LOGIC_BUG,
                            e.span,
                            "this boolean expression contains a logic bug",
                            |db| {
                                db.span_help(
                                    h2q.terminals[i].span,
                                    "this expression can be optimized out by applying boolean operations to the \
                                     outer expression",
                                );
                                db.span_suggestion(
                                    e.span,
                                    "it would look like the following",
                                    suggest(self.cx, suggestion, &h2q.terminals),
                                    // nonminimal_bool can produce minimal but
                                    // not human readable expressions (#3141)
                                    Applicability::Unspecified,
                                );
                            },
                        );
                        // don't also lint `NONMINIMAL_BOOL`
                        return;
                    }
                    // if the number of occurrences of a terminal decreases or any of the stats
                    // decreases while none increases
                    improvement |= (stats.terminals[i] > simplified_stats.terminals[i])
                        || (stats.negations > simplified_stats.negations && stats.ops == simplified_stats.ops)
                        || (stats.ops > simplified_stats.ops && stats.negations == simplified_stats.negations);
                }
                if improvement {
                    improvements.push(suggestion);
                }
            }
            let nonminimal_bool_lint = |suggestions: Vec<_>| {
                span_lint_and_then(
                    self.cx,
                    NONMINIMAL_BOOL,
                    e.span,
                    "this boolean expression can be simplified",
                    |db| {
                        db.span_suggestions(
                            e.span,
                            "try",
                            suggestions.into_iter(),
                            // nonminimal_bool can produce minimal but
                            // not human readable expressions (#3141)
                            Applicability::Unspecified,
                        );
                    },
                );
            };
            if improvements.is_empty() {
                let mut visitor = NotSimplificationVisitor { cx: self.cx };
                visitor.visit_expr(e);
            } else {
                nonminimal_bool_lint(
                    improvements
                        .into_iter()
                        .map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals))
                        .collect(),
                );
            }
        }
    }
}

impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> {
    fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
        if in_macro(e.span) {
            return;
        }
        match &e.kind {
            ExprKind::Binary(binop, _, _) if binop.node == BinOpKind::Or || binop.node == BinOpKind::And => {
                self.bool_expr(e)
            },
            ExprKind::Unary(UnOp::UnNot, inner) => {
                if self.cx.tables.node_types()[inner.hir_id].is_bool() {
                    self.bool_expr(e);
                } else {
                    walk_expr(self, e);
                }
            },
            _ => walk_expr(self, e),
        }
    }
    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
        NestedVisitorMap::None
    }
}

fn implements_ord<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, expr: &Expr<'_>) -> bool {
    let ty = cx.tables.expr_ty(expr);
    get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[]))
}

struct NotSimplificationVisitor<'a, 'tcx> {
    cx: &'a LateContext<'a, 'tcx>,
}

impl<'a, 'tcx> Visitor<'tcx> for NotSimplificationVisitor<'a, 'tcx> {
    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
        if let ExprKind::Unary(UnOp::UnNot, inner) = &expr.kind {
            if let Some(suggestion) = simplify_not(self.cx, inner) {
                span_lint_and_sugg(
                    self.cx,
                    NONMINIMAL_BOOL,
                    expr.span,
                    "this boolean expression can be simplified",
                    "try",
                    suggestion,
                    Applicability::MachineApplicable,
                );
            }
        }

        walk_expr(self, expr);
    }
    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
        NestedVisitorMap::None
    }
}