From 9fd6923821924b61f4613d3f65193dc67f9e83d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 7 May 2021 22:58:12 +0300 Subject: [PATCH] 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 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. --- .../nu-command/src/commands/random/decimal.rs | 41 +++++---- .../nu-command/src/commands/random/integer.rs | 35 ++++---- crates/nu-command/src/commands/range.rs | 28 +----- crates/nu-engine/src/from_value.rs | 16 ++++ crates/nu-protocol/src/value/primitive.rs | 24 +++++ crates/nu-protocol/src/value/range.rs | 87 +++++++++++++++++++ 6 files changed, 170 insertions(+), 61 deletions(-) diff --git a/crates/nu-command/src/commands/random/decimal.rs b/crates/nu-command/src/commands/random/decimal.rs index 18951180e9..eb5c262b43 100644 --- a/crates/nu-command/src/commands/random/decimal.rs +++ b/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>, + range: Option>, } 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 { + fn run(&self, args: CommandArgs) -> Result { 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 { - let (DecimalArgs { range }, _) = args.process()?; +pub fn decimal(args: CommandArgs) -> Result { + 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 { 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), + ))) } } } diff --git a/crates/nu-command/src/commands/random/integer.rs b/crates/nu-command/src/commands/random/integer.rs index 94184d2bae..a4ef074c72 100644 --- a/crates/nu-command/src/commands/random/integer.rs +++ b/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>, + range: Option>, } 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 { + fn run(&self, args: CommandArgs) -> Result { integer(args) } @@ -57,11 +55,14 @@ impl WholeStreamCommand for SubCommand { } } -pub fn integer(args: CommandArgs) -> Result { - let (IntegerArgs { range }, _) = args.process()?; +pub fn integer(args: CommandArgs) -> Result { + 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 { 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()), + )) } } } diff --git a/crates/nu-command/src/commands/range.rs b/crates/nu-command/src/commands/range.rs index aae93d0ec9..016ffbfc63 100644 --- a/crates/nu-command/src/commands/range.rs +++ b/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 { 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())) diff --git a/crates/nu-engine/src/from_value.rs b/crates/nu-engine/src/from_value.rs index ea5dda67bf..e69b5994b0 100644 --- a/crates/nu-engine/src/from_value.rs +++ b/crates/nu-engine/src/from_value.rs @@ -267,6 +267,22 @@ impl FromValue for Range { } } +impl FromValue for Tagged { + fn from_value(v: &Value) -> Result { + 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 { fn from_value(v: &Value) -> Result { match v { diff --git a/crates/nu-protocol/src/value/primitive.rs b/crates/nu-protocol/src/value/primitive.rs index 12832d16c4..4f890651c2 100644 --- a/crates/nu-protocol/src/value/primitive.rs +++ b/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 { + 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 { match self { diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 935391d9b8..c7152fc8e4 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/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, RangeInclusion), pub to: (Spanned, RangeInclusion), } + +impl Range { + pub fn min_u64(&self) -> Result { + 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 { + 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 { + 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 { + 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 { + 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 { + 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? + } +}