Fix cast_sign_loss false positive

This checks if the value is a non-negative constant before linting about
losing the sign.

Because the `constant` function doesn't handle const functions, we check if
the value is from a call to a `max_value` function directly. A utility method
called `get_def_path` was added to make checking for the function paths
easier.

Fixes #2728

Author: Michael Wright

clippy_lints/src/types.rs

@@ -4,8 +4,8 @@ use crate::consts::{constant, Constant};
 use crate::reexport::*;
 use crate::utils::paths;
 use crate::utils::{
-    clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro, int_bits, last_path_segment,
-    match_def_path, match_path, multispan_sugg, opt_def_id, same_tys, sext, snippet, snippet_opt,
+    clip, comparisons, differing_macro_contexts, get_def_path, higher, in_constant, in_macro, int_bits,
+    last_path_segment, match_def_path, match_path, multispan_sugg, opt_def_id, same_tys, sext, snippet, snippet_opt,
     snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext,
     AbsolutePathBuffer,
 };
@@ -1001,6 +1001,48 @@ enum ArchSuffix {
     None,
 }
 
+fn check_loss_of_sign(cx: &LateContext<'_, '_>, expr: &Expr, op: &Expr, cast_from: Ty<'_>, cast_to: Ty<'_>) {
+    if !cast_from.is_signed() || cast_to.is_signed() {
+        return;
+    }
+
+    // don't lint for positive constants
+    let const_val = constant(cx, &cx.tables, op);
+    if_chain! {
+        if let Some((const_val, _)) = const_val;
+        if let Constant::Int(n) = const_val;
+        if let ty::Int(ity) = cast_from.sty;
+        if sext(cx.tcx, n, ity) >= 0;
+        then {
+            return
+        }
+    }
+
+    // don't lint for max_value const fns
+    if_chain! {
+        if let ExprKind::Call(callee, args) = &op.node;
+        if args.is_empty();
+        if let ExprKind::Path(qpath) = &callee.node;
+        let def = cx.tables.qpath_def(qpath, callee.hir_id);
+        if let Some(def_id) = def.opt_def_id();
+        let def_path = get_def_path(cx.tcx, def_id);
+        if let &["core", "num", impl_ty, "max_value"] = &def_path[..];
+        then {
+           if let "<impl i8>" | "<impl i16>" | "<impl i32>" |
+                  "<impl i64>" | "<impl i128>" = impl_ty {
+               return;
+           }
+        }
+    }
+
+    span_lint(
+        cx,
+        CAST_SIGN_LOSS,
+        expr.span,
+        &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to),
+    );
+}
+
 fn check_truncation_and_wrapping(cx: &LateContext<'_, '_>, expr: &Expr, cast_from: Ty<'_>, cast_to: Ty<'_>) {
     let arch_64_suffix = " on targets with 64-bit wide pointers";
     let arch_32_suffix = " on targets with 32-bit wide pointers";
@@ -1176,14 +1218,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass {
                         }
                     },
                     (true, true) => {
-                        if cast_from.is_signed() && !cast_to.is_signed() {
-                            span_lint(
-                                cx,
-                                CAST_SIGN_LOSS,
-                                expr.span,
-                                &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to),
-                            );
-                        }
+                        check_loss_of_sign(cx, expr, ex, cast_from, cast_to);
                         check_truncation_and_wrapping(cx, expr, cast_from, cast_to);
                         check_lossless(cx, expr, ex, cast_from, cast_to);
                     },

clippy_lints/src/utils/mod.rs

@@ -130,6 +130,12 @@ pub fn match_def_path(tcx: TyCtxt<'_, '_, '_>, def_id: DefId, path: &[&str]) ->
     apb.names.len() == path.len() && apb.names.into_iter().zip(path.iter()).all(|(a, &b)| *a == *b)
 }
 
+pub fn get_def_path(tcx: TyCtxt<'_, '_, '_>, def_id: DefId) -> Vec<&'static str> {
+    let mut apb = AbsolutePathBuffer { names: vec![] };
+    tcx.push_item_path(&mut apb, def_id, false);
+    apb.names.iter().map(|n| n.get()).collect()
+}
+
 /// Check if type is struct, enum or union type with given def path.
 pub fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool {
     match ty.sty {

tests/ui/cast.rs

@@ -34,7 +34,15 @@ fn main() {
     (1u8 + 1u8) as u16;
     // Test clippy::cast_sign_loss
     1i32 as u32;
+    -1i32 as u32;
     1isize as usize;
+    -1isize as usize;
+    0i8 as u8;
+    i8::max_value() as u8;
+    i16::max_value() as u16;
+    i32::max_value() as u32;
+    i64::max_value() as u64;
+    i128::max_value() as u128;
     // Extra checks for *size
     // Test cast_unnecessary
     1i32 as i32;

tests/ui/cast.stderr

@@ -70,12 +70,6 @@ error: casting i32 to i8 may truncate the value
 LL |     1i32 as i8;
    |     ^^^^^^^^^^
 
-error: casting i32 to u8 may lose the sign of the value
-  --> $DIR/cast.rs:22:5
-   |
-LL |     1i32 as u8;
-   |     ^^^^^^^^^^
-
 error: casting i32 to u8 may truncate the value
   --> $DIR/cast.rs:22:5
    |
@@ -147,36 +141,36 @@ LL |     (1u8 + 1u8) as u16;
    |     ^^^^^^^^^^^^^^^^^^ help: try: `u16::from(1u8 + 1u8)`
 
 error: casting i32 to u32 may lose the sign of the value
-  --> $DIR/cast.rs:36:5
+  --> $DIR/cast.rs:37:5
    |
-LL |     1i32 as u32;
-   |     ^^^^^^^^^^^
+LL |     -1i32 as u32;
+   |     ^^^^^^^^^^^^
 
 error: casting isize to usize may lose the sign of the value
-  --> $DIR/cast.rs:37:5
+  --> $DIR/cast.rs:39:5
    |
-LL |     1isize as usize;
-   |     ^^^^^^^^^^^^^^^
+LL |     -1isize as usize;
+   |     ^^^^^^^^^^^^^^^^
 
 error: casting to the same type is unnecessary (`i32` -> `i32`)
-  --> $DIR/cast.rs:40:5
+  --> $DIR/cast.rs:48:5
    |
 LL |     1i32 as i32;
    |     ^^^^^^^^^^^
    |
    = note: `-D clippy::unnecessary-cast` implied by `-D warnings`
 
 error: casting to the same type is unnecessary (`f32` -> `f32`)
-  --> $DIR/cast.rs:41:5
+  --> $DIR/cast.rs:49:5
    |
 LL |     1f32 as f32;
    |     ^^^^^^^^^^^
 
 error: casting to the same type is unnecessary (`bool` -> `bool`)
-  --> $DIR/cast.rs:42:5
+  --> $DIR/cast.rs:50:5
    |
 LL |     false as bool;
    |     ^^^^^^^^^^^^^
 
-error: aborting due to 28 previous errors
+error: aborting due to 27 previous errors
 

tests/ui/cast_size.stderr

@@ -38,14 +38,6 @@ error: casting isize to i32 may truncate the value on targets with 64-bit wide p
 LL |     1isize as i32;
    |     ^^^^^^^^^^^^^
 
-error: casting isize to u32 may lose the sign of the value
-  --> $DIR/cast_size.rs:17:5
-   |
-LL |     1isize as u32;
-   |     ^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::cast-sign-loss` implied by `-D warnings`
-
 error: casting isize to u32 may truncate the value on targets with 64-bit wide pointers
   --> $DIR/cast_size.rs:17:5
    |
@@ -78,12 +70,6 @@ error: casting i64 to isize may truncate the value on targets with 32-bit wide p
 LL |     1i64 as isize;
    |     ^^^^^^^^^^^^^
 
-error: casting i64 to usize may lose the sign of the value
-  --> $DIR/cast_size.rs:22:5
-   |
-LL |     1i64 as usize;
-   |     ^^^^^^^^^^^^^
-
 error: casting i64 to usize may truncate the value on targets with 32-bit wide pointers
   --> $DIR/cast_size.rs:22:5
    |
@@ -114,11 +100,5 @@ error: casting u32 to isize may wrap around the value on targets with 32-bit wid
 LL |     1u32 as isize;
    |     ^^^^^^^^^^^^^
 
-error: casting i32 to usize may lose the sign of the value
-  --> $DIR/cast_size.rs:28:5
-   |
-LL |     1i32 as usize;
-   |     ^^^^^^^^^^^^^
-
-error: aborting due to 19 previous errors
+error: aborting due to 16 previous errors