1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136

use rustc::lint::*;
use syntax::ast::*;
use syntax::codemap::Spanned;
use utils::{in_macro, snippet, span_lint_and_sugg};

/// **What it does:** Checks for operations where precedence may be unclear
/// and suggests to add parentheses. Currently it catches the following:
/// * mixed usage of arithmetic and bit shifting/combining operators without
/// parentheses
/// * a "negative" numeric literal (which is really a unary `-` followed by a
/// numeric literal)
///   followed by a method call
///
/// **Why is this bad?** Not everyone knows the precedence of those operators by
/// heart, so expressions like these may trip others trying to reason about the
/// code.
///
/// **Known problems:** None.
///
/// **Example:**
/// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7
/// * `-1i32.abs()` equals -1, while `(-1i32).abs()` equals 1
declare_clippy_lint! {
    pub PRECEDENCE,
    complexity,
    "operations where precedence may be unclear"
}

#[derive(Copy, Clone)]
pub struct Precedence;

impl LintPass for Precedence {
    fn get_lints(&self) -> LintArray {
        lint_array!(PRECEDENCE)
    }
}

impl EarlyLintPass for Precedence {
    fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
        if in_macro(expr.span) {
            return;
        }

        if let ExprKind::Binary(Spanned { node: op, .. }, ref left, ref right) = expr.node {
            let span_sugg = |expr: &Expr, sugg| {
                span_lint_and_sugg(
                    cx,
                    PRECEDENCE,
                    expr.span,
                    "operator precedence can trip the unwary",
                    "consider parenthesizing your expression",
                    sugg,
                );
            };

            if !is_bit_op(op) {
                return;
            }
            match (is_arith_expr(left), is_arith_expr(right)) {
                (true, true) => {
                    let sugg = format!(
                        "({}) {} ({})",
                        snippet(cx, left.span, ".."),
                        op.to_string(),
                        snippet(cx, right.span, "..")
                    );
                    span_sugg(expr, sugg);
                },
                (true, false) => {
                    let sugg = format!(
                        "({}) {} {}",
                        snippet(cx, left.span, ".."),
                        op.to_string(),
                        snippet(cx, right.span, "..")
                    );
                    span_sugg(expr, sugg);
                },
                (false, true) => {
                    let sugg = format!(
                        "{} {} ({})",
                        snippet(cx, left.span, ".."),
                        op.to_string(),
                        snippet(cx, right.span, "..")
                    );
                    span_sugg(expr, sugg);
                },
                (false, false) => (),
            }
        }

        if let ExprKind::Unary(UnOp::Neg, ref rhs) = expr.node {
            if let ExprKind::MethodCall(_, ref args) = rhs.node {
                if let Some(slf) = args.first() {
                    if let ExprKind::Lit(ref lit) = slf.node {
                        match lit.node {
                            LitKind::Int(..) | LitKind::Float(..) | LitKind::FloatUnsuffixed(..) => {
                                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(cx, rhs.span, "..")),
                                );
                            },
                            _ => (),
                        }
                    }
                }
            }
        }
    }
}

fn is_arith_expr(expr: &Expr) -> bool {
    match expr.node {
        ExprKind::Binary(Spanned { node: op, .. }, _, _) => is_arith_op(op),
        _ => false,
    }
}

fn is_bit_op(op: BinOpKind) -> bool {
    use syntax::ast::BinOpKind::*;
    match op {
        BitXor | BitAnd | BitOr | Shl | Shr => true,
        _ => false,
    }
}

fn is_arith_op(op: BinOpKind) -> bool {
    use syntax::ast::BinOpKind::*;
    match op {
        Add | Sub | Mul | Div | Rem => true,
        _ => false,
    }
}