summaryrefslogtreecommitdiffstats
path: root/src/tools/clippy/clippy_lints/src/precedence.rs
blob: e6e3ad05ad70abbee15e673653f9b076a43affff (plain)
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
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use if_chain::if_chain;
use rustc_ast::ast::{BinOpKind, Expr, ExprKind, LitKind, UnOp};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Spanned;

const ALLOWED_ODD_FUNCTIONS: [&str; 14] = [
    "asin",
    "asinh",
    "atan",
    "atanh",
    "cbrt",
    "fract",
    "round",
    "signum",
    "sin",
    "sinh",
    "tan",
    "tanh",
    "to_degrees",
    "to_radians",
];

declare_clippy_lint! {
    /// ### 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.
    ///
    /// ### Example
    /// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7
    /// * `-1i32.abs()` equals -1, while `(-1i32).abs()` equals 1
    #[clippy::version = "pre 1.29.0"]
    pub PRECEDENCE,
    complexity,
    "operations where precedence may be unclear"
}

declare_lint_pass!(Precedence => [PRECEDENCE]);

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

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

            if !is_bit_op(op) {
                return;
            }
            let mut applicability = Applicability::MachineApplicable;
            match (is_arith_expr(left), is_arith_expr(right)) {
                (true, true) => {
                    let sugg = format!(
                        "({}) {} ({})",
                        snippet_with_applicability(cx, left.span, "..", &mut applicability),
                        op.to_string(),
                        snippet_with_applicability(cx, right.span, "..", &mut applicability)
                    );
                    span_sugg(expr, sugg, applicability);
                },
                (true, false) => {
                    let sugg = format!(
                        "({}) {} {}",
                        snippet_with_applicability(cx, left.span, "..", &mut applicability),
                        op.to_string(),
                        snippet_with_applicability(cx, right.span, "..", &mut applicability)
                    );
                    span_sugg(expr, sugg, applicability);
                },
                (false, true) => {
                    let sugg = format!(
                        "{} {} ({})",
                        snippet_with_applicability(cx, left.span, "..", &mut applicability),
                        op.to_string(),
                        snippet_with_applicability(cx, right.span, "..", &mut applicability)
                    );
                    span_sugg(expr, sugg, applicability);
                },
                (false, false) => (),
            }
        }

        if let ExprKind::Unary(UnOp::Neg, operand) = &expr.kind {
            let mut arg = operand;

            let mut all_odd = true;
            while let ExprKind::MethodCall(path_segment, receiver, _, _) = &arg.kind {
                let path_segment_str = path_segment.ident.name.as_str();
                all_odd &= ALLOWED_ODD_FUNCTIONS
                    .iter()
                    .any(|odd_function| **odd_function == *path_segment_str);
                arg = receiver;
            }

            if_chain! {
                if !all_odd;
                if let ExprKind::Lit(lit) = &arg.kind;
                if let LitKind::Int(..) | LitKind::Float(..) = &lit.kind;
                then {
                    let mut applicability = Applicability::MachineApplicable;
                    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_with_applicability(cx, operand.span, "..", &mut applicability)
                        ),
                        applicability,
                    );
                }
            }
        }
    }
}

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

#[must_use]
fn is_bit_op(op: BinOpKind) -> bool {
    use rustc_ast::ast::BinOpKind::{BitAnd, BitOr, BitXor, Shl, Shr};
    matches!(op, BitXor | BitAnd | BitOr | Shl | Shr)
}

#[must_use]
fn is_arith_op(op: BinOpKind) -> bool {
    use rustc_ast::ast::BinOpKind::{Add, Div, Mul, Rem, Sub};
    matches!(op, Add | Sub | Mul | Div | Rem)
}