]>
Commit | Line | Data |
---|---|---|
f20569fa XL |
1 | //! lint on manually implemented checked conversions that could be transformed into `try_from` |
2 | ||
cdc7bbd5 XL |
3 | use clippy_utils::diagnostics::span_lint_and_sugg; |
4 | use clippy_utils::source::snippet_with_applicability; | |
923072b8 | 5 | use clippy_utils::{in_constant, meets_msrv, msrvs, SpanlessEq}; |
f20569fa XL |
6 | use if_chain::if_chain; |
7 | use rustc_ast::ast::LitKind; | |
8 | use rustc_errors::Applicability; | |
9 | use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, QPath, TyKind}; | |
10 | use rustc_lint::{LateContext, LateLintPass, LintContext}; | |
11 | use rustc_middle::lint::in_external_macro; | |
12 | use rustc_semver::RustcVersion; | |
13 | use rustc_session::{declare_tool_lint, impl_lint_pass}; | |
14 | ||
f20569fa | 15 | declare_clippy_lint! { |
94222f64 XL |
16 | /// ### What it does |
17 | /// Checks for explicit bounds checking when casting. | |
f20569fa | 18 | /// |
94222f64 XL |
19 | /// ### Why is this bad? |
20 | /// Reduces the readability of statements & is error prone. | |
f20569fa | 21 | /// |
94222f64 | 22 | /// ### Example |
f20569fa XL |
23 | /// ```rust |
24 | /// # let foo: u32 = 5; | |
923072b8 | 25 | /// foo <= i32::MAX as u32; |
f20569fa XL |
26 | /// ``` |
27 | /// | |
923072b8 | 28 | /// Use instead: |
f20569fa | 29 | /// ```rust |
f20569fa | 30 | /// # let foo = 1; |
923072b8 FG |
31 | /// # #[allow(unused)] |
32 | /// i32::try_from(foo).is_ok(); | |
f20569fa | 33 | /// ``` |
a2a8927a | 34 | #[clippy::version = "1.37.0"] |
f20569fa XL |
35 | pub CHECKED_CONVERSIONS, |
36 | pedantic, | |
37 | "`try_from` could replace manual bounds checking when casting" | |
38 | } | |
39 | ||
40 | pub struct CheckedConversions { | |
41 | msrv: Option<RustcVersion>, | |
42 | } | |
43 | ||
44 | impl CheckedConversions { | |
45 | #[must_use] | |
46 | pub fn new(msrv: Option<RustcVersion>) -> Self { | |
47 | Self { msrv } | |
48 | } | |
49 | } | |
50 | ||
51 | impl_lint_pass!(CheckedConversions => [CHECKED_CONVERSIONS]); | |
52 | ||
53 | impl<'tcx> LateLintPass<'tcx> for CheckedConversions { | |
54 | fn check_expr(&mut self, cx: &LateContext<'_>, item: &Expr<'_>) { | |
923072b8 | 55 | if !meets_msrv(self.msrv, msrvs::TRY_FROM) { |
f20569fa XL |
56 | return; |
57 | } | |
58 | ||
59 | let result = if_chain! { | |
923072b8 | 60 | if !in_constant(cx, item.hir_id); |
f20569fa | 61 | if !in_external_macro(cx.sess(), item.span); |
cdc7bbd5 | 62 | if let ExprKind::Binary(op, left, right) = &item.kind; |
f20569fa XL |
63 | |
64 | then { | |
65 | match op.node { | |
66 | BinOpKind::Ge | BinOpKind::Le => single_check(item), | |
67 | BinOpKind::And => double_check(cx, left, right), | |
68 | _ => None, | |
69 | } | |
70 | } else { | |
71 | None | |
72 | } | |
73 | }; | |
74 | ||
75 | if let Some(cv) = result { | |
76 | if let Some(to_type) = cv.to_type { | |
77 | let mut applicability = Applicability::MachineApplicable; | |
78 | let snippet = snippet_with_applicability(cx, cv.expr_to_cast.span, "_", &mut applicability); | |
79 | span_lint_and_sugg( | |
80 | cx, | |
81 | CHECKED_CONVERSIONS, | |
82 | item.span, | |
83 | "checked cast can be simplified", | |
84 | "try", | |
85 | format!("{}::try_from({}).is_ok()", to_type, snippet), | |
86 | applicability, | |
87 | ); | |
88 | } | |
89 | } | |
90 | } | |
91 | ||
92 | extract_msrv_attr!(LateContext); | |
93 | } | |
94 | ||
95 | /// Searches for a single check from unsigned to _ is done | |
96 | /// todo: check for case signed -> larger unsigned == only x >= 0 | |
97 | fn single_check<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> { | |
98 | check_upper_bound(expr).filter(|cv| cv.cvt == ConversionType::FromUnsigned) | |
99 | } | |
100 | ||
101 | /// Searches for a combination of upper & lower bound checks | |
102 | fn double_check<'a>(cx: &LateContext<'_>, left: &'a Expr<'_>, right: &'a Expr<'_>) -> Option<Conversion<'a>> { | |
103 | let upper_lower = |l, r| { | |
104 | let upper = check_upper_bound(l); | |
105 | let lower = check_lower_bound(r); | |
106 | ||
107 | upper.zip(lower).and_then(|(l, r)| l.combine(r, cx)) | |
108 | }; | |
109 | ||
110 | upper_lower(left, right).or_else(|| upper_lower(right, left)) | |
111 | } | |
112 | ||
113 | /// Contains the result of a tried conversion check | |
114 | #[derive(Clone, Debug)] | |
115 | struct Conversion<'a> { | |
116 | cvt: ConversionType, | |
117 | expr_to_cast: &'a Expr<'a>, | |
118 | to_type: Option<&'a str>, | |
119 | } | |
120 | ||
121 | /// The kind of conversion that is checked | |
923072b8 | 122 | #[derive(Copy, Clone, Debug, PartialEq, Eq)] |
f20569fa XL |
123 | enum ConversionType { |
124 | SignedToUnsigned, | |
125 | SignedToSigned, | |
126 | FromUnsigned, | |
127 | } | |
128 | ||
129 | impl<'a> Conversion<'a> { | |
130 | /// Combine multiple conversions if the are compatible | |
131 | pub fn combine(self, other: Self, cx: &LateContext<'_>) -> Option<Conversion<'a>> { | |
132 | if self.is_compatible(&other, cx) { | |
133 | // Prefer a Conversion that contains a type-constraint | |
134 | Some(if self.to_type.is_some() { self } else { other }) | |
135 | } else { | |
136 | None | |
137 | } | |
138 | } | |
139 | ||
140 | /// Checks if two conversions are compatible | |
141 | /// same type of conversion, same 'castee' and same 'to type' | |
142 | pub fn is_compatible(&self, other: &Self, cx: &LateContext<'_>) -> bool { | |
143 | (self.cvt == other.cvt) | |
144 | && (SpanlessEq::new(cx).eq_expr(self.expr_to_cast, other.expr_to_cast)) | |
145 | && (self.has_compatible_to_type(other)) | |
146 | } | |
147 | ||
148 | /// Checks if the to-type is the same (if there is a type constraint) | |
149 | fn has_compatible_to_type(&self, other: &Self) -> bool { | |
150 | match (self.to_type, other.to_type) { | |
151 | (Some(l), Some(r)) => l == r, | |
152 | _ => true, | |
153 | } | |
154 | } | |
155 | ||
156 | /// Try to construct a new conversion if the conversion type is valid | |
157 | fn try_new(expr_to_cast: &'a Expr<'_>, from_type: &str, to_type: &'a str) -> Option<Conversion<'a>> { | |
158 | ConversionType::try_new(from_type, to_type).map(|cvt| Conversion { | |
159 | cvt, | |
160 | expr_to_cast, | |
161 | to_type: Some(to_type), | |
162 | }) | |
163 | } | |
164 | ||
165 | /// Construct a new conversion without type constraint | |
166 | fn new_any(expr_to_cast: &'a Expr<'_>) -> Conversion<'a> { | |
167 | Conversion { | |
168 | cvt: ConversionType::SignedToUnsigned, | |
169 | expr_to_cast, | |
170 | to_type: None, | |
171 | } | |
172 | } | |
173 | } | |
174 | ||
175 | impl ConversionType { | |
176 | /// Creates a conversion type if the type is allowed & conversion is valid | |
177 | #[must_use] | |
178 | fn try_new(from: &str, to: &str) -> Option<Self> { | |
179 | if UINTS.contains(&from) { | |
180 | Some(Self::FromUnsigned) | |
181 | } else if SINTS.contains(&from) { | |
182 | if UINTS.contains(&to) { | |
183 | Some(Self::SignedToUnsigned) | |
184 | } else if SINTS.contains(&to) { | |
185 | Some(Self::SignedToSigned) | |
186 | } else { | |
187 | None | |
188 | } | |
189 | } else { | |
190 | None | |
191 | } | |
192 | } | |
193 | } | |
194 | ||
195 | /// Check for `expr <= (to_type::MAX as from_type)` | |
196 | fn check_upper_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> { | |
197 | if_chain! { | |
cdc7bbd5 | 198 | if let ExprKind::Binary(ref op, left, right) = &expr.kind; |
f20569fa XL |
199 | if let Some((candidate, check)) = normalize_le_ge(op, left, right); |
200 | if let Some((from, to)) = get_types_from_cast(check, INTS, "max_value", "MAX"); | |
201 | ||
202 | then { | |
203 | Conversion::try_new(candidate, from, to) | |
204 | } else { | |
205 | None | |
206 | } | |
207 | } | |
208 | } | |
209 | ||
210 | /// Check for `expr >= 0|(to_type::MIN as from_type)` | |
211 | fn check_lower_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> { | |
212 | fn check_function<'a>(candidate: &'a Expr<'a>, check: &'a Expr<'a>) -> Option<Conversion<'a>> { | |
213 | (check_lower_bound_zero(candidate, check)).or_else(|| (check_lower_bound_min(candidate, check))) | |
214 | } | |
215 | ||
216 | // First of we need a binary containing the expression & the cast | |
cdc7bbd5 | 217 | if let ExprKind::Binary(ref op, left, right) = &expr.kind { |
f20569fa XL |
218 | normalize_le_ge(op, right, left).and_then(|(l, r)| check_function(l, r)) |
219 | } else { | |
220 | None | |
221 | } | |
222 | } | |
223 | ||
224 | /// Check for `expr >= 0` | |
225 | fn check_lower_bound_zero<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option<Conversion<'a>> { | |
226 | if_chain! { | |
227 | if let ExprKind::Lit(ref lit) = &check.kind; | |
228 | if let LitKind::Int(0, _) = &lit.node; | |
229 | ||
230 | then { | |
231 | Some(Conversion::new_any(candidate)) | |
232 | } else { | |
233 | None | |
234 | } | |
235 | } | |
236 | } | |
237 | ||
238 | /// Check for `expr >= (to_type::MIN as from_type)` | |
239 | fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option<Conversion<'a>> { | |
240 | if let Some((from, to)) = get_types_from_cast(check, SINTS, "min_value", "MIN") { | |
241 | Conversion::try_new(candidate, from, to) | |
242 | } else { | |
243 | None | |
244 | } | |
245 | } | |
246 | ||
247 | /// Tries to extract the from- and to-type from a cast expression | |
248 | fn get_types_from_cast<'a>( | |
249 | expr: &'a Expr<'_>, | |
250 | types: &'a [&str], | |
251 | func: &'a str, | |
252 | assoc_const: &'a str, | |
253 | ) -> Option<(&'a str, &'a str)> { | |
254 | // `to_type::max_value() as from_type` | |
255 | // or `to_type::MAX as from_type` | |
256 | let call_from_cast: Option<(&Expr<'_>, &str)> = if_chain! { | |
257 | // to_type::max_value(), from_type | |
cdc7bbd5 | 258 | if let ExprKind::Cast(limit, from_type) = &expr.kind; |
f20569fa XL |
259 | if let TyKind::Path(ref from_type_path) = &from_type.kind; |
260 | if let Some(from_sym) = int_ty_to_sym(from_type_path); | |
261 | ||
262 | then { | |
263 | Some((limit, from_sym)) | |
264 | } else { | |
265 | None | |
266 | } | |
267 | }; | |
268 | ||
269 | // `from_type::from(to_type::max_value())` | |
270 | let limit_from: Option<(&Expr<'_>, &str)> = call_from_cast.or_else(|| { | |
271 | if_chain! { | |
272 | // `from_type::from, to_type::max_value()` | |
cdc7bbd5 | 273 | if let ExprKind::Call(from_func, args) = &expr.kind; |
f20569fa XL |
274 | // `to_type::max_value()` |
275 | if args.len() == 1; | |
276 | if let limit = &args[0]; | |
277 | // `from_type::from` | |
278 | if let ExprKind::Path(ref path) = &from_func.kind; | |
279 | if let Some(from_sym) = get_implementing_type(path, INTS, "from"); | |
280 | ||
281 | then { | |
282 | Some((limit, from_sym)) | |
283 | } else { | |
284 | None | |
285 | } | |
286 | } | |
287 | }); | |
288 | ||
289 | if let Some((limit, from_type)) = limit_from { | |
290 | match limit.kind { | |
291 | // `from_type::from(_)` | |
292 | ExprKind::Call(path, _) => { | |
293 | if let ExprKind::Path(ref path) = path.kind { | |
294 | // `to_type` | |
295 | if let Some(to_type) = get_implementing_type(path, types, func) { | |
296 | return Some((from_type, to_type)); | |
297 | } | |
298 | } | |
299 | }, | |
300 | // `to_type::MAX` | |
301 | ExprKind::Path(ref path) => { | |
302 | if let Some(to_type) = get_implementing_type(path, types, assoc_const) { | |
303 | return Some((from_type, to_type)); | |
304 | } | |
305 | }, | |
306 | _ => {}, | |
307 | } | |
308 | }; | |
309 | None | |
310 | } | |
311 | ||
312 | /// Gets the type which implements the called function | |
313 | fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function: &str) -> Option<&'a str> { | |
314 | if_chain! { | |
cdc7bbd5 | 315 | if let QPath::TypeRelative(ty, path) = &path; |
f20569fa | 316 | if path.ident.name.as_str() == function; |
cdc7bbd5 | 317 | if let TyKind::Path(QPath::Resolved(None, tp)) = &ty.kind; |
923072b8 | 318 | if let [int] = tp.segments; |
f20569fa | 319 | then { |
a2a8927a XL |
320 | let name = int.ident.name.as_str(); |
321 | candidates.iter().find(|c| &name == *c).copied() | |
f20569fa XL |
322 | } else { |
323 | None | |
324 | } | |
325 | } | |
326 | } | |
327 | ||
328 | /// Gets the type as a string, if it is a supported integer | |
329 | fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> { | |
330 | if_chain! { | |
cdc7bbd5 | 331 | if let QPath::Resolved(_, path) = *path; |
923072b8 | 332 | if let [ty] = path.segments; |
f20569fa | 333 | then { |
a2a8927a XL |
334 | let name = ty.ident.name.as_str(); |
335 | INTS.iter().find(|c| &name == *c).copied() | |
f20569fa XL |
336 | } else { |
337 | None | |
338 | } | |
339 | } | |
340 | } | |
341 | ||
342 | /// Will return the expressions as if they were expr1 <= expr2 | |
343 | fn normalize_le_ge<'a>(op: &BinOp, left: &'a Expr<'a>, right: &'a Expr<'a>) -> Option<(&'a Expr<'a>, &'a Expr<'a>)> { | |
344 | match op.node { | |
345 | BinOpKind::Le => Some((left, right)), | |
346 | BinOpKind::Ge => Some((right, left)), | |
347 | _ => None, | |
348 | } | |
349 | } | |
350 | ||
351 | // Constants | |
352 | const UINTS: &[&str] = &["u8", "u16", "u32", "u64", "usize"]; | |
353 | const SINTS: &[&str] = &["i8", "i16", "i32", "i64", "isize"]; | |
354 | const INTS: &[&str] = &["u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize"]; |