Update operators and other usages

crates/nu-protocol/src/ast/unit.rs

@@ -1,24 +1,9 @@
-use crate::{ShellError, Span, Value};
+use crate::{Filesize, FilesizeUnit, IntoValue, ShellError, Span, Value};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub enum Unit {
-    // Filesize units: metric
+    Filesize(FilesizeUnit),
-    Byte,
-    Kilobyte,
-    Megabyte,
-    Gigabyte,
-    Terabyte,
-    Petabyte,
-    Exabyte,
-
-    // Filesize units: ISO/IEC 80000
-    Kibibyte,
-    Mebibyte,
-    Gibibyte,
-    Tebibyte,
-    Pebibyte,
-    Exbibyte,
 
     // Duration units
     Nanosecond,
@@ -34,33 +19,19 @@ pub enum Unit {
 impl Unit {
     pub fn build_value(self, size: i64, span: Span) -> Result<Value, ShellError> {
         match self {
-            Unit::Byte => Ok(Value::filesize(size, span)),
+            Unit::Filesize(unit) => {
-            Unit::Kilobyte => Ok(Value::filesize(size * 1000, span)),
+                if let Some(filesize) = Filesize::from_unit(size, unit) {
-            Unit::Megabyte => Ok(Value::filesize(size * 1000 * 1000, span)),
+                    Ok(filesize.into_value(span))
-            Unit::Gigabyte => Ok(Value::filesize(size * 1000 * 1000 * 1000, span)),
+                } else {
-            Unit::Terabyte => Ok(Value::filesize(size * 1000 * 1000 * 1000 * 1000, span)),
+                    Err(ShellError::GenericError {
-            Unit::Petabyte => Ok(Value::filesize(
+                        error: "filesize too large".into(),
-                size * 1000 * 1000 * 1000 * 1000 * 1000,
+                        msg: "filesize too large".into(),
-                span,
+                        span: Some(span),
-            )),
+                        help: None,
-            Unit::Exabyte => Ok(Value::filesize(
+                        inner: vec![],
-                size * 1000 * 1000 * 1000 * 1000 * 1000 * 1000,
+                    })
-                span,
+                }
-            )),
+            }
-
-            Unit::Kibibyte => Ok(Value::filesize(size * 1024, span)),
-            Unit::Mebibyte => Ok(Value::filesize(size * 1024 * 1024, span)),
-            Unit::Gibibyte => Ok(Value::filesize(size * 1024 * 1024 * 1024, span)),
-            Unit::Tebibyte => Ok(Value::filesize(size * 1024 * 1024 * 1024 * 1024, span)),
-            Unit::Pebibyte => Ok(Value::filesize(
-                size * 1024 * 1024 * 1024 * 1024 * 1024,
-                span,
-            )),
-            Unit::Exbibyte => Ok(Value::filesize(
-                size * 1024 * 1024 * 1024 * 1024 * 1024 * 1024,
-                span,
-            )),
-
             Unit::Nanosecond => Ok(Value::duration(size, span)),
             Unit::Microsecond => Ok(Value::duration(size * 1000, span)),
             Unit::Millisecond => Ok(Value::duration(size * 1000 * 1000, span)),

crates/nu-protocol/src/value/filesize.rs

@@ -5,9 +5,10 @@ use num_format::ToFormattedString;
 use serde::{Deserialize, Serialize};
 use std::{
     fmt,
-    iter::{Product, Sum},
+    iter::Sum,
-    ops::{Add, Div, Mul, Neg, Rem, Sub},
+    ops::{Add, Mul, Neg, Sub},
 };
+use thiserror::Error;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
 #[repr(transparent)]
@@ -25,6 +26,18 @@ impl Filesize {
         self.0
     }
 
+    pub const fn is_positive(self) -> bool {
+        self.0.is_positive()
+    }
+
+    pub const fn is_negative(self) -> bool {
+        self.0.is_negative()
+    }
+
+    pub const fn signum(self) -> Self {
+        Self(self.0.signum())
+    }
+
     pub const fn from_unit(value: i64, unit: FilesizeUnit) -> Option<Self> {
         if let Some(bytes) = value.checked_mul(unit.as_bytes() as i64) {
             Some(Self(bytes))
@@ -62,6 +75,39 @@ impl TryFrom<Filesize> for u64 {
     }
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Error)]
+pub struct TryFromFloatError(());
+
+impl fmt::Display for TryFromFloatError {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(fmt, "out of range float type conversion attempted")
+    }
+}
+
+impl TryFrom<f64> for Filesize {
+    type Error = TryFromFloatError;
+
+    fn try_from(value: f64) -> Result<Self, Self::Error> {
+        if i64::MIN as f64 <= value && value <= i64::MAX as f64 {
+            Ok(Self(value as i64))
+        } else {
+            Err(TryFromFloatError(()))
+        }
+    }
+}
+
+impl TryFrom<f32> for Filesize {
+    type Error = TryFromFloatError;
+
+    fn try_from(value: f32) -> Result<Self, Self::Error> {
+        if i64::MIN as f32 <= value && value <= i64::MAX as f32 {
+            Ok(Self(value as i64))
+        } else {
+            Err(TryFromFloatError(()))
+        }
+    }
+}
+
 macro_rules! impl_from {
     ($($ty:ty),* $(,)?) => {
         $(
@@ -118,27 +164,45 @@ impl Sub for Filesize {
     }
 }
 
-impl Mul for Filesize {
+impl Mul<i64> for Filesize {
     type Output = Option<Self>;
 
-    fn mul(self, rhs: Self) -> Self::Output {
+    fn mul(self, rhs: i64) -> Self::Output {
-        self.0.checked_mul(rhs.0).map(Self)
+        self.0.checked_mul(rhs).map(Self)
     }
 }
 
-impl Div for Filesize {
+impl Mul<Filesize> for i64 {
+    type Output = Option<Filesize>;
+
+    fn mul(self, rhs: Filesize) -> Self::Output {
+        self.checked_mul(rhs.0).map(Filesize::new)
+    }
+}
+
+impl Mul<f64> for Filesize {
     type Output = Option<Self>;
 
-    fn div(self, rhs: Self) -> Self::Output {
+    fn mul(self, rhs: f64) -> Self::Output {
-        self.0.checked_div(rhs.0).map(Self)
+        let bytes = ((self.0 as f64) * rhs).round();
+        if i64::MIN as f64 <= bytes && bytes <= i64::MAX as f64 {
+            Some(Self(bytes as i64))
+        } else {
+            None
+        }
     }
 }
 
-impl Rem for Filesize {
+impl Mul<Filesize> for f64 {
-    type Output = Option<Self>;
+    type Output = Option<Filesize>;
 
-    fn rem(self, rhs: Self) -> Self::Output {
+    fn mul(self, rhs: Filesize) -> Self::Output {
-        self.0.checked_rem(rhs.0).map(Self)
+        let bytes = (self * (rhs.0 as f64)).round();
+        if i64::MIN as f64 <= bytes && bytes <= i64::MAX as f64 {
+            Some(Filesize(bytes as i64))
+        } else {
+            None
+        }
     }
 }
 
@@ -160,23 +224,13 @@ impl Sum<Filesize> for Option<Filesize> {
     }
 }
 
-impl Product<Filesize> for Option<Filesize> {
-    fn product<I: Iterator<Item = Filesize>>(iter: I) -> Self {
-        let mut product = Filesize::ZERO;
-        for filesize in iter {
-            product = (product * filesize)?;
-        }
-        Some(product)
-    }
-}
-
 impl fmt::Display for Filesize {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        format_filesize(self.0, "auto", Some(false)).fmt(f)
+        format_filesize(*self, "auto", Some(false)).fmt(f)
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
 pub enum FilesizeUnit {
     B,
     KB,
@@ -236,37 +290,15 @@ impl FilesizeUnit {
 
     pub const fn is_decimal(&self) -> bool {
         match self {
-            FilesizeUnit::B
+            Self::B | Self::KB | Self::MB | Self::GB | Self::TB | Self::PB | Self::EB => true,
-            | FilesizeUnit::KB
+            Self::KiB | Self::MiB | Self::GiB | Self::TiB | Self::PiB | Self::EiB => false,
-            | FilesizeUnit::MB
-            | FilesizeUnit::GB
-            | FilesizeUnit::TB
-            | FilesizeUnit::PB
-            | FilesizeUnit::EB => true,
-            FilesizeUnit::KiB
-            | FilesizeUnit::MiB
-            | FilesizeUnit::GiB
-            | FilesizeUnit::TiB
-            | FilesizeUnit::PiB
-            | FilesizeUnit::EiB => false,
         }
     }
 
     pub const fn is_binary(&self) -> bool {
         match self {
-            FilesizeUnit::KB
+            Self::KB | Self::MB | Self::GB | Self::TB | Self::PB | Self::EB => false,
-            | FilesizeUnit::MB
+            Self::B | Self::KiB | Self::MiB | Self::GiB | Self::TiB | Self::PiB | Self::EiB => true,
-            | FilesizeUnit::GB
-            | FilesizeUnit::TB
-            | FilesizeUnit::PB
-            | FilesizeUnit::EB => false,
-            FilesizeUnit::B
-            | FilesizeUnit::KiB
-            | FilesizeUnit::MiB
-            | FilesizeUnit::GiB
-            | FilesizeUnit::TiB
-            | FilesizeUnit::PiB
-            | FilesizeUnit::EiB => true,
         }
     }
 }
@@ -277,11 +309,11 @@ impl fmt::Display for FilesizeUnit {
     }
 }
 
-pub fn format_filesize_from_conf(num_bytes: i64, config: &Config) -> String {
+pub fn format_filesize_from_conf(filesize: Filesize, config: &Config) -> String {
     // We need to take into account config.filesize_metric so, if someone asks for KB
     // and filesize_metric is false, return KiB
     format_filesize(
-        num_bytes,
+        filesize,
         &config.filesize.format,
         Some(config.filesize.metric),
     )
@@ -290,7 +322,7 @@ pub fn format_filesize_from_conf(num_bytes: i64, config: &Config) -> String {
 // filesize_metric is explicit when printed a value according to user config;
 // other places (such as `format filesize`) don't.
 pub fn format_filesize(
-    num_bytes: i64,
+    filesize: Filesize,
     format_value: &str,
     filesize_metric: Option<bool>,
 ) -> String {
@@ -299,7 +331,7 @@ pub fn format_filesize(
     // When format_value is "auto" or an invalid value, the returned ByteUnit doesn't matter
     // and is always B.
     let filesize_unit = get_filesize_format(format_value, filesize_metric);
-    let byte = byte_unit::Byte::from_u64(num_bytes.unsigned_abs());
+    let byte = byte_unit::Byte::from_u64(filesize.0.unsigned_abs());
     let adj_byte = if let Some(unit) = filesize_unit {
         byte.get_adjusted_unit(unit)
     } else {
@@ -317,7 +349,7 @@ pub fn format_filesize(
             let locale = get_system_locale();
             let locale_byte = adj_byte.get_value() as u64;
             let locale_byte_string = locale_byte.to_formatted_string(&locale);
-            let locale_signed_byte_string = if num_bytes.is_negative() {
+            let locale_signed_byte_string = if filesize.is_negative() {
                 format!("-{locale_byte_string}")
             } else {
                 locale_byte_string
@@ -330,7 +362,7 @@ pub fn format_filesize(
             }
         }
         _ => {
-            if num_bytes.is_negative() {
+            if filesize.is_negative() {
                 format!("-{:.1}", adj_byte)
             } else {
                 format!("{:.1}", adj_byte)
@@ -390,6 +422,9 @@ mod tests {
         #[case] filesize_format: String,
         #[case] exp: &str,
     ) {
-        assert_eq!(exp, format_filesize(val, &filesize_format, filesize_metric));
+        assert_eq!(
+            exp,
+            format_filesize(Filesize::new(val), &filesize_format, filesize_metric)
+        );
     }
 }

crates/nu-protocol/src/value/from_value.rs

@@ -252,9 +252,7 @@ impl FromValue for i64 {
     fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
             Value::Int { val, .. } => Ok(val),
-            Value::Filesize { val, .. } => Ok(val),
             Value::Duration { val, .. } => Ok(val),
-
             v => Err(ShellError::CantConvert {
                 to_type: Self::expected_type().to_string(),
                 from_type: v.get_type().to_string(),
@@ -308,9 +306,7 @@ macro_rules! impl_from_value_for_uint {
                 let span = v.span();
                 const MAX: i64 = $max;
                 match v {
-                    Value::Int { val, .. }
+                    Value::Int { val, .. } | Value::Duration { val, .. } => {
-                    | Value::Filesize { val, .. }
-                    | Value::Duration { val, .. } => {
                         match val {
                             i64::MIN..=-1 => Err(ShellError::NeedsPositiveValue { span }),
                             0..=MAX => Ok(val as $type),

crates/nu-protocol/src/value/mod.rs

@@ -83,7 +83,7 @@ pub enum Value {
         internal_span: Span,
     },
     Filesize {
-        val: i64,
+        val: Filesize,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
         #[serde(rename = "span")]
@@ -303,7 +303,7 @@ impl Value {
     /// Returns the inner `i64` filesize value or an error if this `Value` is not a filesize
     pub fn as_filesize(&self) -> Result<Filesize, ShellError> {
         if let Value::Filesize { val, .. } = self {
-            Ok(Filesize::new(*val))
+            Ok(*val)
         } else {
             self.cant_convert_to("filesize")
         }
@@ -1819,9 +1819,9 @@ impl Value {
         }
     }
 
-    pub fn filesize(val: i64, span: Span) -> Value {
+    pub fn filesize(val: impl Into<Filesize>, span: Span) -> Value {
         Value::Filesize {
-            val,
+            val: val.into(),
             internal_span: span,
         }
     }
@@ -1938,7 +1938,7 @@ impl Value {
 
     /// Note: Only use this for test data, *not* live data, as it will point into unknown source
     /// when used in errors.
-    pub fn test_filesize(val: i64) -> Value {
+    pub fn test_filesize(val: impl Into<Filesize>) -> Value {
         Value::filesize(val, Span::test_data())
     }
 
@@ -2478,7 +2478,7 @@ impl Value {
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                if let Some(val) = lhs.checked_add(*rhs) {
+                if let Some(val) = *lhs + *rhs {
                     Ok(Value::filesize(val, span))
                 } else {
                     Err(ShellError::OperatorOverflow {
@@ -2599,7 +2599,7 @@ impl Value {
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                if let Some(val) = lhs.checked_sub(*rhs) {
+                if let Some(val) = *lhs - *rhs {
                     Ok(Value::filesize(val, span))
                 } else {
                     Err(ShellError::OperatorOverflow {
@@ -2647,16 +2647,48 @@ impl Value {
                 Ok(Value::float(lhs * rhs, span))
             }
             (Value::Int { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                Ok(Value::filesize(*lhs * *rhs, span))
+                if let Some(val) = *lhs * *rhs {
+                    Ok(Value::filesize(val, span))
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "multiply operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
             }
             (Value::Filesize { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                Ok(Value::filesize(*lhs * *rhs, span))
+                if let Some(val) = *lhs * *rhs {
+                    Ok(Value::filesize(val, span))
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "multiply operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
             }
             (Value::Float { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                Ok(Value::filesize((*lhs * *rhs as f64) as i64, span))
+                if let Some(val) = *lhs * *rhs {
+                    Ok(Value::filesize(val, span))
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "multiply operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
             }
             (Value::Filesize { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                Ok(Value::filesize((*lhs as f64 * *rhs) as i64, span))
+                if let Some(val) = *lhs * *rhs {
+                    Ok(Value::filesize(val, span))
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "multiply operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
             }
             (Value::Int { val: lhs, .. }, Value::Duration { val: rhs, .. }) => {
                 Ok(Value::duration(*lhs * *rhs, span))
@@ -2714,14 +2746,14 @@ impl Value {
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                if *rhs == 0 {
+                if *rhs == Filesize::ZERO {
                     Err(ShellError::DivisionByZero { span: op })
                 } else {
-                    Ok(Value::float(*lhs as f64 / *rhs as f64, span))
+                    Ok(Value::float(lhs.get() as f64 / rhs.get() as f64, span))
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                if let Some(val) = lhs.checked_div(*rhs) {
+                if let Some(val) = lhs.get().checked_div(*rhs) {
                     Ok(Value::filesize(val, span))
                 } else if *rhs == 0 {
                     Err(ShellError::DivisionByZero { span: op })
@@ -2735,9 +2767,8 @@ impl Value {
             }
             (Value::Filesize { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
                 if *rhs != 0.0 {
-                    let val = *lhs as f64 / rhs;
+                    if let Ok(val) = Filesize::try_from(lhs.get() as f64 / rhs) {
-                    if i64::MIN as f64 <= val && val <= i64::MAX as f64 {
+                        Ok(Value::filesize(val, span))
-                        Ok(Value::filesize(val as i64, span))
                     } else {
                         Err(ShellError::OperatorOverflow {
                             msg: "division operation overflowed".into(),
@@ -2799,163 +2830,6 @@ impl Value {
         }
     }
 
-    pub fn modulo(&self, op: Span, rhs: &Value, span: Span) -> Result<Value, ShellError> {
-        // Based off the unstable `div_floor` function in the std library.
-        fn checked_mod_i64(dividend: i64, divisor: i64) -> Option<i64> {
-            let remainder = dividend.checked_rem(divisor)?;
-            if (remainder > 0 && divisor < 0) || (remainder < 0 && divisor > 0) {
-                // Note that `remainder + divisor` cannot overflow, because `remainder` and
-                // `divisor` have opposite signs.
-                Some(remainder + divisor)
-            } else {
-                Some(remainder)
-            }
-        }
-
-        fn checked_mod_f64(dividend: f64, divisor: f64) -> Option<f64> {
-            if divisor == 0.0 {
-                None
-            } else {
-                let remainder = dividend % divisor;
-                if (remainder > 0.0 && divisor < 0.0) || (remainder < 0.0 && divisor > 0.0) {
-                    Some(remainder + divisor)
-                } else {
-                    Some(remainder)
-                }
-            }
-        }
-
-        match (self, rhs) {
-            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
-                    Ok(Value::int(val, span))
-                } else if *rhs == 0 {
-                    Err(ShellError::DivisionByZero { span: op })
-                } else {
-                    Err(ShellError::OperatorOverflow {
-                        msg: "modulo operation overflowed".into(),
-                        span,
-                        help: None,
-                    })
-                }
-            }
-            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_f64(*lhs as f64, *rhs) {
-                    Ok(Value::float(val, span))
-                } else {
-                    Err(ShellError::DivisionByZero { span: op })
-                }
-            }
-            (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_f64(*lhs, *rhs as f64) {
-                    Ok(Value::float(val, span))
-                } else {
-                    Err(ShellError::DivisionByZero { span: op })
-                }
-            }
-            (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_f64(*lhs, *rhs) {
-                    Ok(Value::float(val, span))
-                } else {
-                    Err(ShellError::DivisionByZero { span: op })
-                }
-            }
-            (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
-                    Ok(Value::filesize(val, span))
-                } else if *rhs == 0 {
-                    Err(ShellError::DivisionByZero { span: op })
-                } else {
-                    Err(ShellError::OperatorOverflow {
-                        msg: "modulo operation overflowed".into(),
-                        span,
-                        help: None,
-                    })
-                }
-            }
-            (Value::Filesize { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
-                    Ok(Value::filesize(val, span))
-                } else if *rhs == 0 {
-                    Err(ShellError::DivisionByZero { span: op })
-                } else {
-                    Err(ShellError::OperatorOverflow {
-                        msg: "modulo operation overflowed".into(),
-                        span,
-                        help: None,
-                    })
-                }
-            }
-            (Value::Filesize { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_f64(*lhs as f64, *rhs) {
-                    if i64::MIN as f64 <= val && val <= i64::MAX as f64 {
-                        Ok(Value::filesize(val as i64, span))
-                    } else {
-                        Err(ShellError::OperatorOverflow {
-                            msg: "modulo operation overflowed".into(),
-                            span,
-                            help: None,
-                        })
-                    }
-                } else {
-                    Err(ShellError::DivisionByZero { span: op })
-                }
-            }
-            (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
-                    Ok(Value::duration(val, span))
-                } else if *rhs == 0 {
-                    Err(ShellError::DivisionByZero { span: op })
-                } else {
-                    Err(ShellError::OperatorOverflow {
-                        msg: "division operation overflowed".into(),
-                        span,
-                        help: None,
-                    })
-                }
-            }
-            (Value::Duration { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
-                    Ok(Value::duration(val, span))
-                } else if *rhs == 0 {
-                    Err(ShellError::DivisionByZero { span: op })
-                } else {
-                    Err(ShellError::OperatorOverflow {
-                        msg: "division operation overflowed".into(),
-                        span,
-                        help: None,
-                    })
-                }
-            }
-            (Value::Duration { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                if let Some(val) = checked_mod_f64(*lhs as f64, *rhs) {
-                    if i64::MIN as f64 <= val && val <= i64::MAX as f64 {
-                        Ok(Value::duration(val as i64, span))
-                    } else {
-                        Err(ShellError::OperatorOverflow {
-                            msg: "division operation overflowed".into(),
-                            span,
-                            help: None,
-                        })
-                    }
-                } else {
-                    Err(ShellError::DivisionByZero { span: op })
-                }
-            }
-            (Value::Custom { val: lhs, .. }, rhs) => {
-                lhs.operation(span, Operator::Math(Math::Modulo), op, rhs)
-            }
-
-            _ => Err(ShellError::OperatorMismatch {
-                op_span: op,
-                lhs_ty: self.get_type().to_string(),
-                lhs_span: self.span(),
-                rhs_ty: rhs.get_type().to_string(),
-                rhs_span: rhs.span(),
-            }),
-        }
-    }
-
     pub fn floor_div(&self, op: Span, rhs: &Value, span: Span) -> Result<Value, ShellError> {
         // Taken from the unstable `div_floor` function in the std library.
         fn checked_div_floor_i64(dividend: i64, divisor: i64) -> Option<i64> {
@@ -3017,9 +2891,9 @@ impl Value {
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
-                if let Some(val) = checked_div_floor_i64(*lhs, *rhs) {
+                if let Some(val) = checked_div_floor_i64(lhs.get(), rhs.get()) {
                     Ok(Value::int(val, span))
-                } else if *rhs == 0 {
+                } else if *rhs == Filesize::ZERO {
                     Err(ShellError::DivisionByZero { span: op })
                 } else {
                     Err(ShellError::OperatorOverflow {
@@ -3030,7 +2904,7 @@ impl Value {
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
-                if let Some(val) = checked_div_floor_i64(*lhs, *rhs) {
+                if let Some(val) = checked_div_floor_i64(lhs.get(), *rhs) {
                     Ok(Value::filesize(val, span))
                 } else if *rhs == 0 {
                     Err(ShellError::DivisionByZero { span: op })
@@ -3043,9 +2917,9 @@ impl Value {
                 }
             }
             (Value::Filesize { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
-                if let Some(val) = checked_div_floor_f64(*lhs as f64, *rhs) {
+                if let Some(val) = checked_div_floor_f64(lhs.get() as f64, *rhs) {
-                    if i64::MIN as f64 <= val && val <= i64::MAX as f64 {
+                    if let Ok(val) = Filesize::try_from(val) {
-                        Ok(Value::filesize(val as i64, span))
+                        Ok(Value::filesize(val, span))
                     } else {
                         Err(ShellError::OperatorOverflow {
                             msg: "division operation overflowed".into(),
@@ -3111,6 +2985,163 @@ impl Value {
         }
     }
 
+    pub fn modulo(&self, op: Span, rhs: &Value, span: Span) -> Result<Value, ShellError> {
+        // Based off the unstable `div_floor` function in the std library.
+        fn checked_mod_i64(dividend: i64, divisor: i64) -> Option<i64> {
+            let remainder = dividend.checked_rem(divisor)?;
+            if (remainder > 0 && divisor < 0) || (remainder < 0 && divisor > 0) {
+                // Note that `remainder + divisor` cannot overflow, because `remainder` and
+                // `divisor` have opposite signs.
+                Some(remainder + divisor)
+            } else {
+                Some(remainder)
+            }
+        }
+
+        fn checked_mod_f64(dividend: f64, divisor: f64) -> Option<f64> {
+            if divisor == 0.0 {
+                None
+            } else {
+                let remainder = dividend % divisor;
+                if (remainder > 0.0 && divisor < 0.0) || (remainder < 0.0 && divisor > 0.0) {
+                    Some(remainder + divisor)
+                } else {
+                    Some(remainder)
+                }
+            }
+        }
+
+        match (self, rhs) {
+            (Value::Int { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
+                    Ok(Value::int(val, span))
+                } else if *rhs == 0 {
+                    Err(ShellError::DivisionByZero { span: op })
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "modulo operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
+            }
+            (Value::Int { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_f64(*lhs as f64, *rhs) {
+                    Ok(Value::float(val, span))
+                } else {
+                    Err(ShellError::DivisionByZero { span: op })
+                }
+            }
+            (Value::Float { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_f64(*lhs, *rhs as f64) {
+                    Ok(Value::float(val, span))
+                } else {
+                    Err(ShellError::DivisionByZero { span: op })
+                }
+            }
+            (Value::Float { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_f64(*lhs, *rhs) {
+                    Ok(Value::float(val, span))
+                } else {
+                    Err(ShellError::DivisionByZero { span: op })
+                }
+            }
+            (Value::Filesize { val: lhs, .. }, Value::Filesize { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_i64(lhs.get(), rhs.get()) {
+                    Ok(Value::filesize(val, span))
+                } else if *rhs == Filesize::ZERO {
+                    Err(ShellError::DivisionByZero { span: op })
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "modulo operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
+            }
+            (Value::Filesize { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_i64(lhs.get(), *rhs) {
+                    Ok(Value::filesize(val, span))
+                } else if *rhs == 0 {
+                    Err(ShellError::DivisionByZero { span: op })
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "modulo operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
+            }
+            (Value::Filesize { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_f64(lhs.get() as f64, *rhs) {
+                    if let Ok(val) = Filesize::try_from(val) {
+                        Ok(Value::filesize(val, span))
+                    } else {
+                        Err(ShellError::OperatorOverflow {
+                            msg: "modulo operation overflowed".into(),
+                            span,
+                            help: None,
+                        })
+                    }
+                } else {
+                    Err(ShellError::DivisionByZero { span: op })
+                }
+            }
+            (Value::Duration { val: lhs, .. }, Value::Duration { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
+                    Ok(Value::duration(val, span))
+                } else if *rhs == 0 {
+                    Err(ShellError::DivisionByZero { span: op })
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "division operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
+            }
+            (Value::Duration { val: lhs, .. }, Value::Int { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_i64(*lhs, *rhs) {
+                    Ok(Value::duration(val, span))
+                } else if *rhs == 0 {
+                    Err(ShellError::DivisionByZero { span: op })
+                } else {
+                    Err(ShellError::OperatorOverflow {
+                        msg: "division operation overflowed".into(),
+                        span,
+                        help: None,
+                    })
+                }
+            }
+            (Value::Duration { val: lhs, .. }, Value::Float { val: rhs, .. }) => {
+                if let Some(val) = checked_mod_f64(*lhs as f64, *rhs) {
+                    if i64::MIN as f64 <= val && val <= i64::MAX as f64 {
+                        Ok(Value::duration(val as i64, span))
+                    } else {
+                        Err(ShellError::OperatorOverflow {
+                            msg: "division operation overflowed".into(),
+                            span,
+                            help: None,
+                        })
+                    }
+                } else {
+                    Err(ShellError::DivisionByZero { span: op })
+                }
+            }
+            (Value::Custom { val: lhs, .. }, rhs) => {
+                lhs.operation(span, Operator::Math(Math::Modulo), op, rhs)
+            }
+
+            _ => Err(ShellError::OperatorMismatch {
+                op_span: op,
+                lhs_ty: self.get_type().to_string(),
+                lhs_span: self.span(),
+                rhs_ty: rhs.get_type().to_string(),
+                rhs_span: rhs.span(),
+            }),
+        }
+    }
+
     pub fn lt(&self, op: Span, rhs: &Value, span: Span) -> Result<Value, ShellError> {
         if let (Value::Custom { val: lhs, .. }, rhs) = (self, rhs) {
             return lhs.operation(