Remove panics from random integer and make the constraint more idiomatic (#2578)

* Remove panics from random integer and make the constraint more idiomatic

* Add open intervals to NumericRange

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

@@ -1,16 +1,17 @@
 use crate::commands::WholeStreamCommand;
+use crate::deserializer::NumericRange;
 use crate::prelude::*;
 use nu_errors::ShellError;
 use nu_protocol::{ReturnSuccess, 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 {
-    min: Option<Tagged<u64>>,
-    max: Option<Tagged<u64>>,
+    range: Option<Tagged<NumericRange>>,
 }
 
 #[async_trait]
@@ -20,13 +21,11 @@ impl WholeStreamCommand for SubCommand {
     }
 
     fn signature(&self) -> Signature {
-        Signature::build("random integer")
-            .named("min", SyntaxShape::Int, "Minimum value", Some('m'))
-            .named("max", SyntaxShape::Int, "Maximum value", Some('x'))
+        Signature::build("random integer").optional("range", SyntaxShape::Range, "Range of values")
     }
 
     fn usage(&self) -> &str {
-        "Generate a random integer [--min <m>] [--max <x>]"
+        "Generate a random integer [min..max]"
     }
 
     async fn run(
@@ -46,17 +45,17 @@ impl WholeStreamCommand for SubCommand {
             },
             Example {
                 description: "Generate a random integer less than or equal to 500",
-                example: "random integer --max 500",
+                example: "random integer ..500",
                 result: None,
             },
             Example {
                 description: "Generate a random integer greater than or equal to 100000",
-                example: "random integer --min 100000",
+                example: "random integer 100000..",
                 result: None,
             },
             Example {
                 description: "Generate a random integer between 1 and 10",
-                example: "random integer --min 1 --max 10",
+                example: "random integer 1..10",
                 result: None,
             },
         ]
@@ -67,26 +66,37 @@ pub async fn integer(
     args: CommandArgs,
     registry: &CommandRegistry,
 ) -> Result<OutputStream, ShellError> {
-    let (IntegerArgs { min, max }, _) = args.process(&registry).await?;
+    let (IntegerArgs { range }, _) = args.process(&registry).await?;
 
-    let min = if let Some(min_tagged) = min {
-        *min_tagged
+    let (min, max) = if let Some(range) = &range {
+        (range.item.min(), range.item.max())
     } else {
-        0
+        (0, u64::MAX)
     };
 
-    let max = if let Some(max_tagged) = max {
-        *max_tagged
-    } else {
-        u64::MAX
-    };
+    match min.cmp(&max) {
+        Ordering::Greater => Err(ShellError::labeled_error(
+            format!("Invalid range {}..{}", min, max),
+            "expected a valid range",
+            range
+                .expect("Unexpected ordering error in random integer")
+                .span(),
+        )),
+        Ordering::Equal => {
+            let untagged_result = UntaggedValue::int(min).into_value(Tag::unknown());
+            Ok(OutputStream::one(ReturnSuccess::value(untagged_result)))
+        }
+        _ => {
+            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 mut thread_rng = thread_rng();
-    let result: u64 = thread_rng.gen_range(min, max);
+            let untagged_result = UntaggedValue::int(result).into_value(Tag::unknown());
 
-    let untagged_result = UntaggedValue::int(result).into_value(Tag::unknown());
-
-    Ok(OutputStream::one(ReturnSuccess::value(untagged_result)))
+            Ok(OutputStream::one(ReturnSuccess::value(untagged_result)))
+        }
+    }
 }
 
 #[cfg(test)]

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

@@ -3,7 +3,7 @@ use crate::context::CommandRegistry;
 use crate::deserializer::NumericRange;
 use crate::prelude::*;
 use nu_errors::ShellError;
-use nu_protocol::{ReturnSuccess, Signature, SyntaxShape};
+use nu_protocol::{RangeInclusion, ReturnSuccess, Signature, SyntaxShape};
 use nu_source::Tagged;
 
 #[derive(Deserialize)]
@@ -44,11 +44,23 @@ async fn range(args: CommandArgs, registry: &CommandRegistry) -> Result<OutputSt
     let registry = registry.clone();
     let (RangeArgs { area }, input) = args.process(&registry).await?;
     let range = area.item;
-    let (from, _) = range.from;
-    let (to, _) = range.to;
-
-    let from = *from as usize;
-    let to = *to as usize;
+    let (from, left_inclusive) = range.from;
+    let (to, right_inclusive) = range.to;
+    let from = from.map(|from| *from as usize).unwrap_or(0).saturating_add(
+        if left_inclusive == RangeInclusion::Inclusive {
+            0
+        } else {
+            1
+        },
+    );
+    let to = to
+        .map(|to| *to as usize)
+        .unwrap_or(usize::MAX)
+        .saturating_sub(if right_inclusive == RangeInclusion::Inclusive {
+            0
+        } else {
+            1
+        });
 
     Ok(input
         .skip(from)

crates/nu-cli/src/deserializer.rs

@@ -12,8 +12,31 @@ use std::path::PathBuf;
 
 #[derive(Copy, Clone, Deserialize, Serialize)]
 pub struct NumericRange {
-    pub from: (Spanned<u64>, RangeInclusion),
-    pub to: (Spanned<u64>, RangeInclusion),
+    pub from: (Option<Spanned<u64>>, RangeInclusion),
+    pub to: (Option<Spanned<u64>>, RangeInclusion),
+}
+
+impl NumericRange {
+    pub fn min(self) -> u64 {
+        match self.from.1 {
+            RangeInclusion::Inclusive => self.from.0.map(|from| *from).unwrap_or(0),
+            RangeInclusion::Exclusive => {
+                self.from.0.map(|from| *from).unwrap_or(0).saturating_add(1)
+            }
+        }
+    }
+
+    pub fn max(self) -> u64 {
+        match self.to.1 {
+            RangeInclusion::Inclusive => self.to.0.map(|to| *to).unwrap_or(u64::MAX),
+            RangeInclusion::Exclusive => self
+                .to
+                .0
+                .map(|to| *to)
+                .unwrap_or(u64::MAX)
+                .saturating_sub(1),
+        }
+    }
 }
 
 #[derive(Debug)]
@@ -443,12 +466,21 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut ConfigDeserializer<'de> {
                 let left_span = left.span;
                 let right_span = right.span;
 
-                let left = left.as_u64(left_span)?;
-                let right = right.as_u64(right_span)?;
+                let left = match left.item {
+                    Primitive::Nothing => None,
+                    _ => Some(left.as_u64(left_span)?),
+                };
+                let right = match right.item {
+                    Primitive::Nothing => None,
+                    _ => Some(right.as_u64(right_span)?),
+                };
 
                 let numeric_range = NumericRange {
-                    from: (left.spanned(left_span), left_inclusion),
-                    to: (right.spanned(right_span), right_inclusion),
+                    from: (left.map(|left| left.spanned(left_span)), left_inclusion),
+                    to: (
+                        right.map(|right| right.spanned(right_span)),
+                        right_inclusion,
+                    ),
                 };
 
                 visit::<Tagged<NumericRange>, _>(numeric_range.tagged(tag), name, fields, visitor)

crates/nu-cli/tests/commands/random/integer.rs

@@ -5,9 +5,33 @@ fn generates_an_integer() {
     let actual = nu!(
         cwd: ".", pipeline(
         r#"
-        random integer --min 42 --max 43
+        random integer 42..43
         "#
     ));
 
     assert!(actual.out.contains("42") || actual.out.contains("43"));
 }
+
+#[test]
+fn generates_55() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        random integer 55..55
+        "#
+    ));
+
+    assert!(actual.out.contains("55"));
+}
+
+#[test]
+fn generates_0() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        random integer ..<1
+        "#
+    ));
+
+    assert!(actual.out.contains("0"));
+}

docs/commands/random.md

@@ -106,16 +106,16 @@ true
 ```
 
 ```shell
-> random integer --min 5000
+> random integer 5000..
 8700890823
 ```
 
 ```shell
-> random integer --max 100
+> random integer ..100
 73
 ```
 
 ```shell
-> random integer --min 100000 --max 200000
+> random integer 100000..200000
 173400
 ```