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