]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 XL |
1 | use rustc_hir::{BinOpKind, Expr, ExprKind}; |
2 | use rustc_lint::{LateContext, LateLintPass}; | |
3 | use rustc_middle::ty; | |
4 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
5 | ||
cdc7bbd5 | 6 | use clippy_utils::comparisons::{normalize_comparison, Rel}; |
17df50a5 | 7 | use clippy_utils::consts::{constant, Constant}; |
cdc7bbd5 XL |
8 | use clippy_utils::diagnostics::span_lint_and_help; |
9 | use clippy_utils::source::snippet; | |
10 | use clippy_utils::ty::is_isize_or_usize; | |
11 | use clippy_utils::{clip, int_bits, unsext}; | |
12 | ||
13 | declare_clippy_lint! { | |
94222f64 XL |
14 | /// ### What it does |
15 | /// Checks for comparisons where one side of the relation is | |
cdc7bbd5 XL |
16 | /// either the minimum or maximum value for its type and warns if it involves a |
17 | /// case that is always true or always false. Only integer and boolean types are | |
18 | /// checked. | |
19 | /// | |
94222f64 XL |
20 | /// ### Why is this bad? |
21 | /// An expression like `min <= x` may misleadingly imply | |
cdc7bbd5 XL |
22 | /// that it is possible for `x` to be less than the minimum. Expressions like |
23 | /// `max < x` are probably mistakes. | |
24 | /// | |
94222f64 XL |
25 | /// ### Known problems |
26 | /// For `usize` the size of the current compile target will | |
cdc7bbd5 XL |
27 | /// be assumed (e.g., 64 bits on 64 bit systems). This means code that uses such |
28 | /// a comparison to detect target pointer width will trigger this lint. One can | |
29 | /// use `mem::sizeof` and compare its value or conditional compilation | |
30 | /// attributes | |
31 | /// like `#[cfg(target_pointer_width = "64")] ..` instead. | |
32 | /// | |
94222f64 | 33 | /// ### Example |
cdc7bbd5 XL |
34 | /// ```rust |
35 | /// let vec: Vec<isize> = Vec::new(); | |
36 | /// if vec.len() <= 0 {} | |
37 | /// if 100 > i32::MAX {} | |
38 | /// ``` | |
39 | pub ABSURD_EXTREME_COMPARISONS, | |
40 | correctness, | |
41 | "a comparison with a maximum or minimum value that is always true or false" | |
42 | } | |
43 | ||
44 | declare_lint_pass!(AbsurdExtremeComparisons => [ABSURD_EXTREME_COMPARISONS]); | |
45 | ||
46 | impl<'tcx> LateLintPass<'tcx> for AbsurdExtremeComparisons { | |
47 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | |
48 | if let ExprKind::Binary(ref cmp, lhs, rhs) = expr.kind { | |
49 | if let Some((culprit, result)) = detect_absurd_comparison(cx, cmp.node, lhs, rhs) { | |
50 | if !expr.span.from_expansion() { | |
51 | let msg = "this comparison involving the minimum or maximum element for this \ | |
52 | type contains a case that is always true or always false"; | |
53 | ||
54 | let conclusion = match result { | |
55 | AbsurdComparisonResult::AlwaysFalse => "this comparison is always false".to_owned(), | |
56 | AbsurdComparisonResult::AlwaysTrue => "this comparison is always true".to_owned(), | |
57 | AbsurdComparisonResult::InequalityImpossible => format!( | |
58 | "the case where the two sides are not equal never occurs, consider using `{} == {}` \ | |
59 | instead", | |
60 | snippet(cx, lhs.span, "lhs"), | |
61 | snippet(cx, rhs.span, "rhs") | |
62 | ), | |
63 | }; | |
64 | ||
65 | let help = format!( | |
66 | "because `{}` is the {} value for this type, {}", | |
67 | snippet(cx, culprit.expr.span, "x"), | |
68 | match culprit.which { | |
69 | ExtremeType::Minimum => "minimum", | |
70 | ExtremeType::Maximum => "maximum", | |
71 | }, | |
72 | conclusion | |
73 | ); | |
74 | ||
75 | span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help); | |
76 | } | |
77 | } | |
78 | } | |
79 | } | |
80 | } | |
81 | ||
82 | enum ExtremeType { | |
83 | Minimum, | |
84 | Maximum, | |
85 | } | |
86 | ||
87 | struct ExtremeExpr<'a> { | |
88 | which: ExtremeType, | |
89 | expr: &'a Expr<'a>, | |
90 | } | |
91 | ||
92 | enum AbsurdComparisonResult { | |
93 | AlwaysFalse, | |
94 | AlwaysTrue, | |
95 | InequalityImpossible, | |
96 | } | |
97 | ||
98 | fn is_cast_between_fixed_and_target<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { | |
99 | if let ExprKind::Cast(cast_exp, _) = expr.kind { | |
100 | let precast_ty = cx.typeck_results().expr_ty(cast_exp); | |
101 | let cast_ty = cx.typeck_results().expr_ty(expr); | |
102 | ||
103 | return is_isize_or_usize(precast_ty) != is_isize_or_usize(cast_ty); | |
104 | } | |
105 | ||
106 | false | |
107 | } | |
108 | ||
109 | fn detect_absurd_comparison<'tcx>( | |
110 | cx: &LateContext<'tcx>, | |
111 | op: BinOpKind, | |
112 | lhs: &'tcx Expr<'_>, | |
113 | rhs: &'tcx Expr<'_>, | |
114 | ) -> Option<(ExtremeExpr<'tcx>, AbsurdComparisonResult)> { | |
115 | use AbsurdComparisonResult::{AlwaysFalse, AlwaysTrue, InequalityImpossible}; | |
116 | use ExtremeType::{Maximum, Minimum}; | |
117 | // absurd comparison only makes sense on primitive types | |
118 | // primitive types don't implement comparison operators with each other | |
119 | if cx.typeck_results().expr_ty(lhs) != cx.typeck_results().expr_ty(rhs) { | |
120 | return None; | |
121 | } | |
122 | ||
123 | // comparisons between fix sized types and target sized types are considered unanalyzable | |
124 | if is_cast_between_fixed_and_target(cx, lhs) || is_cast_between_fixed_and_target(cx, rhs) { | |
125 | return None; | |
126 | } | |
127 | ||
128 | let (rel, normalized_lhs, normalized_rhs) = normalize_comparison(op, lhs, rhs)?; | |
129 | ||
130 | let lx = detect_extreme_expr(cx, normalized_lhs); | |
131 | let rx = detect_extreme_expr(cx, normalized_rhs); | |
132 | ||
133 | Some(match rel { | |
134 | Rel::Lt => { | |
135 | match (lx, rx) { | |
136 | (Some(l @ ExtremeExpr { which: Maximum, .. }), _) => (l, AlwaysFalse), // max < x | |
137 | (_, Some(r @ ExtremeExpr { which: Minimum, .. })) => (r, AlwaysFalse), // x < min | |
138 | _ => return None, | |
139 | } | |
140 | }, | |
141 | Rel::Le => { | |
142 | match (lx, rx) { | |
143 | (Some(l @ ExtremeExpr { which: Minimum, .. }), _) => (l, AlwaysTrue), // min <= x | |
144 | (Some(l @ ExtremeExpr { which: Maximum, .. }), _) => (l, InequalityImpossible), // max <= x | |
145 | (_, Some(r @ ExtremeExpr { which: Minimum, .. })) => (r, InequalityImpossible), // x <= min | |
146 | (_, Some(r @ ExtremeExpr { which: Maximum, .. })) => (r, AlwaysTrue), // x <= max | |
147 | _ => return None, | |
148 | } | |
149 | }, | |
150 | Rel::Ne | Rel::Eq => return None, | |
151 | }) | |
152 | } | |
153 | ||
154 | fn detect_extreme_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<ExtremeExpr<'tcx>> { | |
155 | let ty = cx.typeck_results().expr_ty(expr); | |
156 | ||
157 | let cv = constant(cx, cx.typeck_results(), expr)?.0; | |
158 | ||
159 | let which = match (ty.kind(), cv) { | |
160 | (&ty::Bool, Constant::Bool(false)) | (&ty::Uint(_), Constant::Int(0)) => ExtremeType::Minimum, | |
161 | (&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MIN >> (128 - int_bits(cx.tcx, ity)), ity) => { | |
162 | ExtremeType::Minimum | |
163 | }, | |
164 | ||
165 | (&ty::Bool, Constant::Bool(true)) => ExtremeType::Maximum, | |
166 | (&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MAX >> (128 - int_bits(cx.tcx, ity)), ity) => { | |
167 | ExtremeType::Maximum | |
168 | }, | |
169 | (&ty::Uint(uty), Constant::Int(i)) if clip(cx.tcx, u128::MAX, uty) == i => ExtremeType::Maximum, | |
170 | ||
171 | _ => return None, | |
172 | }; | |
173 | Some(ExtremeExpr { which, expr }) | |
174 | } |