From 7002a9ee8373230c6fb37269da717132ac79417f Mon Sep 17 00:00:00 2001 From: Krishna Veera Reddy Date: Thu, 5 Dec 2019 15:47:15 -0800 Subject: [PATCH 1/2] Fix false positive with cast_sign_loss lint `cast_sign_loss` lint incorrectly suggests that the result of `checked_abs`, `rem_euclid` and `checked_rem_euclid` cannot be casted to an unsigned integer without loss. --- clippy_lints/src/types.rs | 29 +++++++++++++++++++---------- tests/ui/cast.rs | 25 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 4ef88887642..bc1dfdfa146 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -26,8 +26,9 @@ use crate::consts::{constant, Constant}; use crate::utils::paths; use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, int_bits, last_path_segment, match_def_path, - match_path, multispan_sugg, qpath_res, same_tys, sext, snippet, snippet_opt, snippet_with_applicability, - snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext, + match_path, method_chain_args, multispan_sugg, qpath_res, same_tys, sext, snippet, snippet_opt, + snippet_with_applicability, snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, + span_lint_and_then, unsext, }; declare_clippy_lint! { @@ -1021,14 +1022,22 @@ fn check_loss_of_sign(cx: &LateContext<'_, '_>, expr: &Expr, op: &Expr, cast_fro } } - // don't lint for the result of `abs` - // `abs` is an inherent impl of `i{N}`, so a method call with ident `abs` will always - // resolve to that spesific method - if_chain! { - if let ExprKind::MethodCall(ref path, _, _) = op.kind; - if path.ident.name.as_str() == "abs"; - then { - return + // don't lint for the result of methods that always return non-negative values + if let ExprKind::MethodCall(ref path, _, _) = op.kind { + let mut method_name = path.ident.name.as_str(); + let whitelisted_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; + + if_chain! { + if method_name == "unwrap"; + if let Some(arglist) = method_chain_args(op, &["unwrap"]); + if let ExprKind::MethodCall(ref inner_path, _, _) = &arglist[0][0].kind; + then { + method_name = inner_path.ident.name.as_str(); + } + } + + if whitelisted_methods.iter().any(|&name| method_name == name) { + return; } } diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 80329a52c2d..358e36984e9 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -47,4 +47,29 @@ fn main() { (-1i32).abs() as u32; (-1i64).abs() as u64; (-1isize).abs() as usize; + (-1i8).checked_abs().unwrap() as u8; + (-1i16).checked_abs().unwrap() as u16; + (-1i32).checked_abs().unwrap() as u32; + (-1i64).checked_abs().unwrap() as u64; + (-1isize).checked_abs().unwrap() as usize; + (-1i8).rem_euclid(1i8) as u8; + (-1i16).rem_euclid(1i16) as u16; + (-1i32).rem_euclid(1i32) as u32; + (-1i64).rem_euclid(1i64) as u64; + (-1isize).rem_euclid(1isize) as usize; + (1i8).rem_euclid(-1i8) as u8; + (1i16).rem_euclid(-1i16) as u16; + (1i32).rem_euclid(-1i32) as u32; + (1i64).rem_euclid(-1i64) as u64; + (1isize).rem_euclid(-1isize) as usize; + (-1i8).checked_rem_euclid(1i8).unwrap() as u8; + (-1i16).checked_rem_euclid(1i16).unwrap() as u16; + (-1i32).checked_rem_euclid(1i32).unwrap() as u32; + (-1i64).checked_rem_euclid(1i64).unwrap() as u64; + (-1isize).checked_rem_euclid(1isize).unwrap() as usize; + (1i8).checked_rem_euclid(-1i8).unwrap() as u8; + (1i16).checked_rem_euclid(-1i16).unwrap() as u16; + (1i32).checked_rem_euclid(-1i32).unwrap() as u32; + (1i64).checked_rem_euclid(-1i64).unwrap() as u64; + (1isize).checked_rem_euclid(-1isize).unwrap() as usize; } From c0fb74baf55af92b7ef714495ee54cc04795b938 Mon Sep 17 00:00:00 2001 From: Krishna Veera Reddy Date: Thu, 5 Dec 2019 17:18:27 -0800 Subject: [PATCH 2/2] Add widening tests for `cast_sign_loss` lint --- tests/ui/cast.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 358e36984e9..7e0b211d862 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -42,34 +42,54 @@ fn main() { i32::max_value() as u32; i64::max_value() as u64; i128::max_value() as u128; + (-1i8).abs() as u8; (-1i16).abs() as u16; (-1i32).abs() as u32; (-1i64).abs() as u64; (-1isize).abs() as usize; + (-1i8).checked_abs().unwrap() as u8; (-1i16).checked_abs().unwrap() as u16; (-1i32).checked_abs().unwrap() as u32; (-1i64).checked_abs().unwrap() as u64; (-1isize).checked_abs().unwrap() as usize; + (-1i8).rem_euclid(1i8) as u8; + (-1i8).rem_euclid(1i8) as u16; (-1i16).rem_euclid(1i16) as u16; + (-1i16).rem_euclid(1i16) as u32; (-1i32).rem_euclid(1i32) as u32; + (-1i32).rem_euclid(1i32) as u64; (-1i64).rem_euclid(1i64) as u64; + (-1i64).rem_euclid(1i64) as u128; (-1isize).rem_euclid(1isize) as usize; (1i8).rem_euclid(-1i8) as u8; + (1i8).rem_euclid(-1i8) as u16; (1i16).rem_euclid(-1i16) as u16; + (1i16).rem_euclid(-1i16) as u32; (1i32).rem_euclid(-1i32) as u32; + (1i32).rem_euclid(-1i32) as u64; (1i64).rem_euclid(-1i64) as u64; + (1i64).rem_euclid(-1i64) as u128; (1isize).rem_euclid(-1isize) as usize; + (-1i8).checked_rem_euclid(1i8).unwrap() as u8; + (-1i8).checked_rem_euclid(1i8).unwrap() as u16; (-1i16).checked_rem_euclid(1i16).unwrap() as u16; + (-1i16).checked_rem_euclid(1i16).unwrap() as u32; (-1i32).checked_rem_euclid(1i32).unwrap() as u32; + (-1i32).checked_rem_euclid(1i32).unwrap() as u64; (-1i64).checked_rem_euclid(1i64).unwrap() as u64; + (-1i64).checked_rem_euclid(1i64).unwrap() as u128; (-1isize).checked_rem_euclid(1isize).unwrap() as usize; (1i8).checked_rem_euclid(-1i8).unwrap() as u8; + (1i8).checked_rem_euclid(-1i8).unwrap() as u16; (1i16).checked_rem_euclid(-1i16).unwrap() as u16; + (1i16).checked_rem_euclid(-1i16).unwrap() as u32; (1i32).checked_rem_euclid(-1i32).unwrap() as u32; + (1i32).checked_rem_euclid(-1i32).unwrap() as u64; (1i64).checked_rem_euclid(-1i64).unwrap() as u64; + (1i64).checked_rem_euclid(-1i64).unwrap() as u128; (1isize).checked_rem_euclid(-1isize).unwrap() as usize; }