Port random integer & decimal to engine-p + related refactoring (#3393)

* Implement minmax for Range; Simplify range command

* Port random integer to enginep; New FromValue impl

Now, FromValue is implemented for Tagged<Range> to allow extracting args
into this type.

* Make sure range value extraction fails properly

The range endpoint extraction methods now return error instead of
silently clipping the value. This now makes `random integer ..-4` fail
properly since -4 can't be cast as u64.

* Port random decimal to enginep & Refactor

This added a way to interpret Range limits as f64 and a Primitive helper
to get its value as f64.

A side effect of this commit is that it is now possible to specify the
command bounds as true decimals. E.g., `random decimal 0.0..3.14` does
not clip 3.14 to 3.

crates/nu-command/src/commands/random/decimal.rs

@@ -1,17 +1,15 @@
 use crate::prelude::*;
-use nu_engine::deserializer::NumericRange;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
-use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, UntaggedValue};
+use nu_protocol::{Range, Signature, SyntaxShape, UntaggedValue};
 use nu_source::Tagged;
 use rand::prelude::{thread_rng, Rng};
 use std::cmp::Ordering;
 
 pub struct SubCommand;
 
-#[derive(Deserialize)]
 pub struct DecimalArgs {
-    range: Option<Tagged<NumericRange>>,
+    range: Option<Tagged<Range>>,
 }
 
 impl WholeStreamCommand for SubCommand {
@@ -27,7 +25,7 @@ impl WholeStreamCommand for SubCommand {
         "Generate a random decimal within a range [min..max]"
     }
 
-    fn run_with_actions(&self, args: CommandArgs) -> Result<ActionStream, ShellError> {
+    fn run(&self, args: CommandArgs) -> Result<OutputStream, ShellError> {
         decimal(args)
     }
 
@@ -49,19 +47,22 @@ impl WholeStreamCommand for SubCommand {
                 result: None,
             },
             Example {
-                description: "Generate a random decimal between 1 and 10",
-                example: "random decimal 1..10",
+                description: "Generate a random decimal between 1.0 and 1.1",
+                example: "random decimal 1.0..1.1",
                 result: None,
             },
         ]
     }
 }
 
-pub fn decimal(args: CommandArgs) -> Result<ActionStream, ShellError> {
-    let (DecimalArgs { range }, _) = args.process()?;
+pub fn decimal(args: CommandArgs) -> Result<OutputStream, ShellError> {
+    let args = args.evaluate_once()?;
+    let cmd_args = DecimalArgs {
+        range: args.opt(0)?,
+    };
 
-    let (min, max) = if let Some(range) = &range {
-        (range.item.min() as f64, range.item.max() as f64)
+    let (min, max) = if let Some(range) = &cmd_args.range {
+        (range.item.min_f64()?, range.item.max_f64()?)
     } else {
         (0.0, 1.0)
     };
@@ -70,21 +71,23 @@ pub fn decimal(args: CommandArgs) -> Result<ActionStream, ShellError> {
         Some(Ordering::Greater) => Err(ShellError::labeled_error(
             format!("Invalid range {}..{}", min, max),
             "expected a valid range",
-            range
+            cmd_args
+                .range
                 .expect("Unexpected ordering error in random decimal")
                 .span(),
         )),
-        Some(Ordering::Equal) => {
-            let untagged_result = UntaggedValue::decimal_from_float(min, Span::new(64, 64));
-            Ok(ActionStream::one(ReturnSuccess::value(untagged_result)))
-        }
+        Some(Ordering::Equal) => Ok(OutputStream::one(UntaggedValue::decimal_from_float(
+            min,
+            Span::new(64, 64),
+        ))),
         _ => {
             let mut thread_rng = thread_rng();
             let result: f64 = thread_rng.gen_range(min, max);
 
-            let untagged_result = UntaggedValue::decimal_from_float(result, Span::new(64, 64));
-
-            Ok(ActionStream::one(ReturnSuccess::value(untagged_result)))
+            Ok(OutputStream::one(UntaggedValue::decimal_from_float(
+                result,
+                Span::new(64, 64),
+            )))
         }
     }
 }

crates/nu-command/src/commands/random/integer.rs

@@ -1,17 +1,15 @@
 use crate::prelude::*;
-use nu_engine::deserializer::NumericRange;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
-use nu_protocol::{ReturnSuccess, Signature, SyntaxShape, UntaggedValue};
+use nu_protocol::{Range, Signature, SyntaxShape, UntaggedValue};
 use nu_source::Tagged;
 use rand::prelude::{thread_rng, Rng};
 use std::cmp::Ordering;
 
 pub struct SubCommand;
 
-#[derive(Deserialize)]
 pub struct IntegerArgs {
-    range: Option<Tagged<NumericRange>>,
+    range: Option<Tagged<Range>>,
 }
 
 impl WholeStreamCommand for SubCommand {
@@ -27,7 +25,7 @@ impl WholeStreamCommand for SubCommand {
         "Generate a random integer [min..max]"
     }
 
-    fn run_with_actions(&self, args: CommandArgs) -> Result<ActionStream, ShellError> {
+    fn run(&self, args: CommandArgs) -> Result<OutputStream, ShellError> {
         integer(args)
     }
 
@@ -57,11 +55,14 @@ impl WholeStreamCommand for SubCommand {
     }
 }
 
-pub fn integer(args: CommandArgs) -> Result<ActionStream, ShellError> {
-    let (IntegerArgs { range }, _) = args.process()?;
+pub fn integer(args: CommandArgs) -> Result<OutputStream, ShellError> {
+    let args = args.evaluate_once()?;
+    let cmd_args = IntegerArgs {
+        range: args.opt(0)?,
+    };
 
-    let (min, max) = if let Some(range) = &range {
-        (range.item.min(), range.item.max())
+    let (min, max) = if let Some(range) = &cmd_args.range {
+        (range.min_u64()?, range.max_u64()?)
     } else {
         (0, u64::MAX)
     };
@@ -70,23 +71,23 @@ pub fn integer(args: CommandArgs) -> Result<ActionStream, ShellError> {
         Ordering::Greater => Err(ShellError::labeled_error(
             format!("Invalid range {}..{}", min, max),
             "expected a valid range",
-            range
+            cmd_args
+                .range
                 .expect("Unexpected ordering error in random integer")
                 .span(),
         )),
-        Ordering::Equal => {
-            let untagged_result = UntaggedValue::int(min).into_value(Tag::unknown());
-            Ok(ActionStream::one(ReturnSuccess::value(untagged_result)))
-        }
+        Ordering::Equal => Ok(OutputStream::one(
+            UntaggedValue::int(min).into_value(Tag::unknown()),
+        )),
         _ => {
             let mut thread_rng = thread_rng();
             // add 1 to max, because gen_range is right-exclusive
             let max = max.saturating_add(1);
             let result: u64 = thread_rng.gen_range(min, max);
 
-            let untagged_result = UntaggedValue::int(result).into_value(Tag::unknown());
-
-            Ok(ActionStream::one(ReturnSuccess::value(untagged_result)))
+            Ok(OutputStream::one(
+                UntaggedValue::int(result).into_value(Tag::unknown()),
+            ))
         }
     }
 }

crates/nu-command/src/commands/range.rs

@@ -1,7 +1,7 @@
 use crate::prelude::*;
 use nu_engine::WholeStreamCommand;
 use nu_errors::ShellError;
-use nu_protocol::{RangeInclusion, Signature, SyntaxShape, Value};
+use nu_protocol::{Signature, SyntaxShape, Value};
 
 struct RangeArgs {
     range: nu_protocol::Range,
@@ -37,30 +37,8 @@ fn range(args: CommandArgs) -> Result<OutputStream, ShellError> {
         range: args.req(0)?,
     };
 
-    let (from, left_inclusive) = cmd_args.range.from;
-    let (to, right_inclusive) = cmd_args.range.to;
-    let from_span = from.span;
-    let to_span = to.span;
-
-    let from = from
-        .map(|from| from.as_usize(from_span))
-        .item
-        .unwrap_or(0)
-        .saturating_add(if left_inclusive == RangeInclusion::Inclusive {
-            0
-        } else {
-            1
-        });
-
-    let to = to
-        .map(|to| to.as_usize(to_span))
-        .item
-        .unwrap_or(usize::MAX)
-        .saturating_sub(if right_inclusive == RangeInclusion::Inclusive {
-            0
-        } else {
-            1
-        });
+    let from = cmd_args.range.min_usize()?;
+    let to = cmd_args.range.max_usize()?;
 
     if from > to {
         Ok(OutputStream::one(Value::nothing()))

crates/nu-engine/src/from_value.rs

@@ -267,6 +267,22 @@ impl FromValue for Range {
     }
 }
 
+impl FromValue for Tagged<Range> {
+    fn from_value(v: &Value) -> Result<Self, ShellError> {
+        let tag = v.tag.clone();
+        match v.value {
+            UntaggedValue::Primitive(Primitive::Range(ref range)) => {
+                Ok((*range.clone()).tagged(tag))
+            }
+            _ => Err(ShellError::labeled_error(
+                "Can't convert to range",
+                "can't convert to range",
+                tag.span,
+            )),
+        }
+    }
+}
+
 impl FromValue for Vec<u8> {
     fn from_value(v: &Value) -> Result<Self, ShellError> {
         match v {

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

@@ -129,6 +129,30 @@ impl Primitive {
         }
     }
 
+    /// Converts a primitive value to a f64, if possible. Uses a span to build an error if the conversion isn't possible.
+    pub fn as_f64(&self, span: Span) -> Result<f64, ShellError> {
+        match self {
+            Primitive::Int(int) => int.to_f64().ok_or_else(|| {
+                ShellError::range_error(
+                    ExpectedRange::F64,
+                    &format!("{}", int).spanned(span),
+                    "converting an integer into a 64-bit floating point",
+                )
+            }),
+            Primitive::Decimal(decimal) => decimal.to_f64().ok_or_else(|| {
+                ShellError::range_error(
+                    ExpectedRange::F64,
+                    &format!("{}", decimal).spanned(span),
+                    "converting a decimal into a 64-bit floating point",
+                )
+            }),
+            other => Err(ShellError::type_error(
+                "number",
+                other.type_name().spanned(span),
+            )),
+        }
+    }
+
     /// Converts a primitive value to a i64, if possible. Uses a span to build an error if the conversion isn't possible.
     pub fn as_i64(&self, span: Span) -> Result<i64, ShellError> {
         match self {

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

@@ -1,5 +1,6 @@
 use crate::value::Primitive;
 use derive_new::new;
+use nu_errors::ShellError;
 use nu_source::{DbgDocBldr, DebugDocBuilder, Spanned};
 use serde::{Deserialize, Serialize};
 
@@ -35,3 +36,89 @@ pub struct Range {
     pub from: (Spanned<Primitive>, RangeInclusion),
     pub to: (Spanned<Primitive>, RangeInclusion),
 }
+
+impl Range {
+    pub fn min_u64(&self) -> Result<u64, ShellError> {
+        let (from, range_incl) = &self.from;
+
+        let minval = if let Primitive::Nothing = from.item {
+            u64::MIN
+        } else {
+            from.item.as_u64(from.span)?
+        };
+
+        match range_incl {
+            RangeInclusion::Inclusive => Ok(minval),
+            RangeInclusion::Exclusive => Ok(minval.saturating_add(1)),
+        }
+    }
+
+    pub fn max_u64(&self) -> Result<u64, ShellError> {
+        let (to, range_incl) = &self.to;
+
+        let maxval = if let Primitive::Nothing = to.item {
+            u64::MAX
+        } else {
+            to.item.as_u64(to.span)?
+        };
+
+        match range_incl {
+            RangeInclusion::Inclusive => Ok(maxval),
+            RangeInclusion::Exclusive => Ok(maxval.saturating_sub(1)),
+        }
+    }
+
+    pub fn min_usize(&self) -> Result<usize, ShellError> {
+        let (from, range_incl) = &self.from;
+
+        let minval = if let Primitive::Nothing = from.item {
+            usize::MIN
+        } else {
+            from.item.as_usize(from.span)?
+        };
+
+        match range_incl {
+            RangeInclusion::Inclusive => Ok(minval),
+            RangeInclusion::Exclusive => Ok(minval.saturating_add(1)),
+        }
+    }
+
+    pub fn max_usize(&self) -> Result<usize, ShellError> {
+        let (to, range_incl) = &self.to;
+
+        let maxval = if let Primitive::Nothing = to.item {
+            usize::MAX
+        } else {
+            to.item.as_usize(to.span)?
+        };
+
+        match range_incl {
+            RangeInclusion::Inclusive => Ok(maxval),
+            RangeInclusion::Exclusive => Ok(maxval.saturating_sub(1)),
+        }
+    }
+
+    pub fn min_f64(&self) -> Result<f64, ShellError> {
+        let from = &self.from.0;
+
+        if let Primitive::Nothing = from.item {
+            Ok(f64::MIN)
+        } else {
+            Ok(from.item.as_f64(from.span)?)
+        }
+
+        // How would inclusive vs. exclusive range work here?
+    }
+
+    pub fn max_f64(&self) -> Result<f64, ShellError> {
+        let to = &self.to.0;
+
+        if let Primitive::Nothing = to.item {
+            Ok(f64::MAX)
+        } else {
+            Ok(to.item.as_f64(to.span)?)
+        }
+
+        // How would inclusive vs. exclusive range work here?
+    }
+}