RUST-2127 lint for sign loss when casting (#1481)

Author: abr-egn

src/action/gridfs/download.rs

@@ -66,12 +66,14 @@ impl GridFsBucket {
         } else {
             (-1, -revision - 1)
         };
+        // unwrap safety: `skip` is always >= 0
+        let skip: u64 = skip.try_into().unwrap();
 
         match self
             .files()
             .find_one(doc! { "filename": filename })
             .sort(doc! { "uploadDate": sort })
-            .skip(skip as u64)
+            .skip(skip)
             .await?
         {
             Some(fcd) => Ok(fcd),

src/bson_util.rs

@@ -62,14 +62,39 @@ pub(crate) fn get_int_raw(val: RawBsonRef<'_>) -> Option<i64> {
     }
 }
 
+#[allow(private_bounds)]
+pub(crate) fn round_clamp<T: RoundClampTarget>(input: f64) -> T {
+    T::round_clamp(input)
+}
+
+trait RoundClampTarget {
+    fn round_clamp(input: f64) -> Self;
+}
+
+impl RoundClampTarget for u64 {
+    #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
+    fn round_clamp(input: f64) -> Self {
+        input as u64
+    }
+}
+
+impl RoundClampTarget for u32 {
+    #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
+    fn round_clamp(input: f64) -> Self {
+        input as u32
+    }
+}
+
 /// Coerce numeric types into an `u64` if it would be lossless to do so. If this Bson is not numeric
 /// or the conversion would be lossy (e.g. 1.5 -> 1), this returns `None`.
 #[allow(clippy::cast_possible_truncation)]
 pub(crate) fn get_u64(val: &Bson) -> Option<u64> {
     match *val {
         Bson::Int32(i) => u64::try_from(i).ok(),
         Bson::Int64(i) => u64::try_from(i).ok(),
-        Bson::Double(f) if (f - (f as u64 as f64)).abs() <= f64::EPSILON => Some(f as u64),
+        Bson::Double(f) if (f - (round_clamp::<u64>(f) as f64)).abs() <= f64::EPSILON => {
+            Some(round_clamp(f))
+        }
         _ => None,
     }
 }
@@ -291,6 +316,31 @@ impl RawDocumentCollection for RawArrayBuf {
     }
 }
 
+pub(crate) mod option_u64_as_i64 {
+    use serde::{Deserialize, Serialize};
+
+    pub(crate) fn serialize<S: serde::Serializer>(
+        value: &Option<u64>,
+        s: S,
+    ) -> std::result::Result<S::Ok, S::Error> {
+        let conv: Option<i64> = value
+            .as_ref()
+            .map(|&u| u.try_into())
+            .transpose()
+            .map_err(serde::ser::Error::custom)?;
+        conv.serialize(s)
+    }
+
+    pub(crate) fn deserialize<'de, D: serde::Deserializer<'de>>(
+        d: D,
+    ) -> std::result::Result<Option<u64>, D::Error> {
+        let conv = Option::<i64>::deserialize(d)?;
+        conv.map(|i| i.try_into())
+            .transpose()
+            .map_err(serde::de::Error::custom)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use crate::bson_util::num_decimal_digits;

src/client/options.rs

@@ -1952,7 +1952,10 @@ impl ConnectionString {
                         // -1 maxStaleness means no maxStaleness, which is the default
                         return Ok(());
                     }
-                    Ordering::Greater => Duration::from_secs(max_staleness_seconds as u64),
+                    Ordering::Greater => {
+                        // unwrap safety: `max_staleness_seconds` will always be >= 0
+                        Duration::from_secs(max_staleness_seconds.try_into().unwrap())
+                    }
                 };
 
                 parts.max_staleness = Some(max_staleness);

src/cmap/conn/stream_description.rs

@@ -57,7 +57,7 @@ impl StreamDescription {
             logical_session_timeout: reply
                 .command_response
                 .logical_session_timeout_minutes
-                .map(|mins| Duration::from_secs(mins as u64 * 60)),
+                .map(|mins| Duration::from_secs(mins * 60)),
             max_bson_object_size: reply.command_response.max_bson_object_size,
             // The defaulting to 100,000 is here because mongocryptd doesn't include this field in
             // hello replies; this should never happen when talking to a real server.

src/cmap/options.rs

@@ -116,7 +116,8 @@ impl<'de> Deserialize<'de> for BackgroundThreadInterval {
             Ordering::Less => BackgroundThreadInterval::Never,
             Ordering::Equal => return Err(D::Error::custom("zero is not allowed")),
             Ordering::Greater => {
-                BackgroundThreadInterval::Every(Duration::from_millis(millis as u64))
+                // unwrap safety: millis is validated to be in the u64 range
+                BackgroundThreadInterval::Every(Duration::from_millis(millis.try_into().unwrap()))
             }
         })
     }

src/hello.rs

@@ -157,7 +157,8 @@ pub(crate) struct HelloCommandResponse {
     pub is_replica_set: Option<bool>,
 
     /// The time in minutes that a session remains active after its most recent use.
-    pub logical_session_timeout_minutes: Option<i64>,
+    #[serde(default, with = "crate::bson_util::option_u64_as_i64")]
+    pub logical_session_timeout_minutes: Option<u64>,
 
     /// Optime and date information for the server's most recent write operation.
     pub last_write: Option<LastWrite>,

src/index/options.rs

@@ -168,7 +168,9 @@ impl<'de> Deserialize<'de> for IndexVersion {
             0 => Ok(IndexVersion::V0),
             1 => Ok(IndexVersion::V1),
             2 => Ok(IndexVersion::V2),
-            i => Ok(IndexVersion::Custom(i as u32)),
+            i => Ok(IndexVersion::Custom(
+                i.try_into().map_err(serde::de::Error::custom)?,
+            )),
         }
     }
 }
@@ -213,7 +215,9 @@ impl<'de> Deserialize<'de> for TextIndexVersion {
             1 => Ok(TextIndexVersion::V1),
             2 => Ok(TextIndexVersion::V2),
             3 => Ok(TextIndexVersion::V3),
-            i => Ok(TextIndexVersion::Custom(i as u32)),
+            i => Ok(TextIndexVersion::Custom(
+                i.try_into().map_err(serde::de::Error::custom)?,
+            )),
         }
     }
 }
@@ -253,7 +257,9 @@ impl<'de> Deserialize<'de> for Sphere2DIndexVersion {
         match i32::deserialize(deserializer)? {
             2 => Ok(Sphere2DIndexVersion::V2),
             3 => Ok(Sphere2DIndexVersion::V3),
-            i => Ok(Sphere2DIndexVersion::Custom(i as u32)),
+            i => Ok(Sphere2DIndexVersion::Custom(
+                i.try_into().map_err(serde::de::Error::custom)?,
+            )),
         }
     }
 }

src/lib.rs

@@ -1,8 +1,11 @@
 #![doc = include_str!("../README.md")]
-#![warn(missing_docs)]
-#![warn(rustdoc::missing_crate_level_docs)]
-#![warn(clippy::cast_possible_truncation)]
-#![warn(clippy::cast_possible_wrap)]
+#![warn(
+    missing_docs,
+    rustdoc::missing_crate_level_docs,
+    clippy::cast_possible_truncation,
+    clippy::cast_possible_wrap,
+    clippy::cast_sign_loss
+)]
 #![allow(
     clippy::unreadable_literal,
     clippy::cognitive_complexity,

src/sdam/description/server.rs

@@ -409,7 +409,7 @@ impl ServerDescription {
             Ok(Some(ref reply)) => Ok(reply
                 .command_response
                 .logical_session_timeout_minutes
-                .map(|timeout| Duration::from_secs(timeout as u64 * 60))),
+                .map(|timeout| Duration::from_secs(timeout * 60))),
             Err(ref e) => Err(e.clone()),
         }
     }

src/sdam/description/topology/server_selection/test/in_window.rs

@@ -1,6 +1,9 @@
 use std::{collections::HashMap, sync::Arc, time::Duration};
 
-use crate::bson::{doc, Document};
+use crate::{
+    bson::{doc, Document},
+    bson_util::round_clamp,
+};
 use approx::abs_diff_eq;
 use serde::Deserialize;
 
@@ -188,14 +191,14 @@ async fn load_balancing_test() {
             assert!(
                 share_of_selections <= max_share,
                 "expected no more than {}% of selections, instead got {}%",
-                (max_share * 100.0) as u32,
-                (share_of_selections * 100.0) as u32
+                round_clamp::<u32>(max_share * 100.0),
+                round_clamp::<u32>(share_of_selections * 100.0)
             );
             assert!(
                 share_of_selections >= min_share,
                 "expected at least {}% of selections, instead got {}%",
-                (min_share * 100.0) as u32,
-                (share_of_selections * 100.0) as u32
+                round_clamp::<u32>(min_share * 100.0),
+                round_clamp::<u32>(share_of_selections * 100.0)
             );
         }
     }