diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index b3da15c87a..0be42ff4f8 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -85,13 +85,19 @@ pub fn eval_expression( val: *f, span: expr.span, }), - Expr::Range(from, to, operator) => { - // TODO: Embed the min/max into Range and set max to be the true max + Expr::Range(from, next, to, operator) => { let from = if let Some(f) = from { eval_expression(context, f)? } else { - Value::Int { - val: 0i64, + Value::Nothing { + span: Span::unknown(), + } + }; + + let next = if let Some(s) = next { + eval_expression(context, s)? + } else { + Value::Nothing { span: Span::unknown(), } }; @@ -99,31 +105,13 @@ pub fn eval_expression( let to = if let Some(t) = to { eval_expression(context, t)? } else { - Value::Int { - val: 100i64, + Value::Nothing { span: Span::unknown(), } }; - let range = match (&from, &to) { - (&Value::Int { .. }, &Value::Int { .. }) => Range { - from: from.clone(), - to: to.clone(), - inclusion: operator.inclusion, - }, - (lhs, rhs) => { - return Err(ShellError::OperatorMismatch { - op_span: operator.span, - lhs_ty: lhs.get_type(), - lhs_span: lhs.span(), - rhs_ty: rhs.get_type(), - rhs_span: rhs.span(), - }) - } - }; - Ok(Value::Range { - val: Box::new(range), + val: Box::new(Range::new(expr.span, from, next, to, operator)?), span: expr.span, }) } @@ -159,7 +147,6 @@ pub fn eval_expression( x => Err(ShellError::UnsupportedOperator(x, op_span)), } } - Expr::Subexpression(block_id) => { let engine_state = context.engine_state.borrow(); let block = engine_state.get_block(*block_id); diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 17bc3178c9..f32de5eca9 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -85,15 +85,19 @@ pub fn flatten_expression( } output } - Expr::Range(from, to, op) => { + Expr::Range(from, next, to, op) => { let mut output = vec![]; if let Some(f) = from { output.extend(flatten_expression(working_set, f)); } + if let Some(s) = next { + output.extend(vec![(op.next_op_span, FlatShape::Operator)]); + output.extend(flatten_expression(working_set, s)); + } + output.extend(vec![(op.span, FlatShape::Operator)]); if let Some(t) = to { output.extend(flatten_expression(working_set, t)); } - output.extend(vec![(op.span, FlatShape::Operator)]); output } Expr::Bool(_) => { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index cb68e00ec2..6199e87ee1 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -709,8 +709,8 @@ pub fn parse_range( working_set: &mut StateWorkingSet, span: Span, ) -> (Expression, Option) { - // Range follows the following syntax: [][][] - // where is ".." + // Range follows the following syntax: [][][] + // where is ".." // and is ".." or "..<" // and one of the or bounds must be present (just '..' is not allowed since it // looks like parent directory) @@ -725,42 +725,28 @@ pub fn parse_range( // First, figure out what exact operators are used and determine their positions let dotdot_pos: Vec<_> = token.match_indices("..").map(|(pos, _)| pos).collect(); - let (step_op_pos, range_op_pos) = + let (next_op_pos, range_op_pos) = match dotdot_pos.len() { 1 => (None, dotdot_pos[0]), 2 => (Some(dotdot_pos[0]), dotdot_pos[1]), _ => return ( garbage(span), Some(ParseError::Expected( - "one range operator ('..' or '..<') and optionally one step operator ('..')" + "one range operator ('..' or '..<') and optionally one next operator ('..')" .into(), span, )), ), }; - let _step_op_span = step_op_pos.map(|pos| { - Span::new( - span.start + pos, - span.start + pos + "..".len(), // Only ".." is allowed for step operator - ) - }); - - let (range_op, range_op_str, range_op_span) = if let Some(pos) = token.find("..<") { + let (inclusion, range_op_str, range_op_span) = if let Some(pos) = token.find("..<") { if pos == range_op_pos { let op_str = "..<"; let op_span = Span::new( span.start + range_op_pos, span.start + range_op_pos + op_str.len(), ); - ( - RangeOperator { - inclusion: RangeInclusion::RightExclusive, - span: op_span, - }, - "..<", - op_span, - ) + (RangeInclusion::RightExclusive, "..<", op_span) } else { return ( garbage(span), @@ -776,21 +762,14 @@ pub fn parse_range( span.start + range_op_pos, span.start + range_op_pos + op_str.len(), ); - ( - RangeOperator { - inclusion: RangeInclusion::Inclusive, - span: op_span, - }, - "..", - op_span, - ) + (RangeInclusion::Inclusive, "..", op_span) }; - // Now, based on the operator positions, figure out where the bounds & step are located and + // Now, based on the operator positions, figure out where the bounds & next are located and // parse them - // TODO: Actually parse the step number + // TODO: Actually parse the next number let from = if token.starts_with("..") { - // token starts with either step operator, or range operator -- we don't care which one + // token starts with either next operator, or range operator -- we don't care which one None } else { let from_span = Span::new(span.start, span.start + dotdot_pos[0]); @@ -830,9 +809,32 @@ pub fn parse_range( ); } + let (next, next_op_span) = if let Some(pos) = next_op_pos { + let next_op_span = Span::new(span.start + pos, span.start + pos + "..".len()); + let next_span = Span::new(next_op_span.end, range_op_span.start); + + match parse_value(working_set, next_span, &SyntaxShape::Number) { + (expression, None) => (Some(Box::new(expression)), next_op_span), + _ => { + return ( + garbage(span), + Some(ParseError::Expected("number".into(), span)), + ) + } + } + } else { + (None, Span::unknown()) + }; + + let range_op = RangeOperator { + inclusion, + span: range_op_span, + next_op_span, + }; + ( Expression { - expr: Expr::Range(from, to, range_op), + expr: Expr::Range(from, next, to, range_op), span, ty: Type::Range, }, diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index ef52f1fbcf..2015f356c0 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -164,6 +164,7 @@ mod range { Expression { expr: Expr::Range( Some(_), + None, Some(_), RangeOperator { inclusion: RangeInclusion::Inclusive, @@ -195,6 +196,7 @@ mod range { Expression { expr: Expr::Range( Some(_), + None, Some(_), RangeOperator { inclusion: RangeInclusion::RightExclusive, @@ -209,6 +211,38 @@ mod range { } } + #[test] + fn parse_reverse_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse(&mut working_set, None, b"10..0", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + None, + Some(_), + RangeOperator { + inclusion: RangeInclusion::Inclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + #[test] fn parse_subexpression_range() { let engine_state = EngineState::new(); @@ -226,6 +260,7 @@ mod range { Expression { expr: Expr::Range( Some(_), + None, Some(_), RangeOperator { inclusion: RangeInclusion::RightExclusive, @@ -257,6 +292,7 @@ mod range { Expression { expr: Expr::Range( Some(_), + None, Some(_), RangeOperator { inclusion: RangeInclusion::Inclusive, @@ -288,6 +324,7 @@ mod range { Expression { expr: Expr::Range( Some(_), + None, Some(_), RangeOperator { inclusion: RangeInclusion::RightExclusive, @@ -320,6 +357,7 @@ mod range { expr: Expr::Range( Some(_), None, + None, RangeOperator { inclusion: RangeInclusion::Inclusive, .. @@ -350,6 +388,7 @@ mod range { Expression { expr: Expr::Range( Some(_), + None, Some(_), RangeOperator { inclusion: RangeInclusion::Inclusive, diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index f26673f2a1..62d7a25c8f 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -7,8 +7,9 @@ pub enum Expr { Int(i64), Float(f64), Range( - Option>, - Option>, + Option>, // from + Option>, // next value after "from" + Option>, // to RangeOperator, ), Var(VarId), diff --git a/crates/nu-protocol/src/ast/operator.rs b/crates/nu-protocol/src/ast/operator.rs index c7c82eba41..edd4f56fc2 100644 --- a/crates/nu-protocol/src/ast/operator.rs +++ b/crates/nu-protocol/src/ast/operator.rs @@ -59,6 +59,7 @@ pub enum RangeInclusion { pub struct RangeOperator { pub inclusion: RangeInclusion, pub span: Span, + pub next_op_span: Span, } impl Display for RangeOperator { diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 094ca2d590..5d81e03f91 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -8,7 +8,7 @@ pub use stream::*; use std::fmt::Debug; -use crate::ast::{PathMember, RangeInclusion}; +use crate::ast::PathMember; use crate::{span, BlockId, Span, Type}; use crate::ShellError; @@ -131,20 +131,10 @@ impl Value { Value::Int { val, .. } => val.to_string(), Value::Float { val, .. } => val.to_string(), Value::Range { val, .. } => { - let vals: Vec = match (&val.from, &val.to) { - (Value::Int { val: from, .. }, Value::Int { val: to, .. }) => { - match val.inclusion { - RangeInclusion::Inclusive => (*from..=*to).collect(), - RangeInclusion::RightExclusive => (*from..*to).collect(), - } - } - _ => Vec::new(), - }; - format!( "range: [{}]", - vals.iter() - .map(|x| x.to_string()) + val.into_iter() + .map(|x| x.into_string()) .collect::>() .join(", ") ) diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 54d836de16..ff138e4af6 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -1,12 +1,106 @@ -use crate::{ast::RangeInclusion, *}; +use crate::{ + ast::{RangeInclusion, RangeOperator}, + *, +}; #[derive(Debug, Clone, PartialEq)] pub struct Range { pub from: Value, + pub incr: Value, pub to: Value, pub inclusion: RangeInclusion, } +impl Range { + pub fn new( + expr_span: Span, + from: Value, + next: Value, + to: Value, + operator: &RangeOperator, + ) -> Result { + // Select from & to values if they're not specified + // TODO: Replace the placeholder values with proper min/max based on data type + let from = if let Value::Nothing { .. } = from { + Value::Int { + val: 0i64, + span: Span::unknown(), + } + } else { + from + }; + + let to = if let Value::Nothing { .. } = to { + if let Ok(Value::Bool { val: true, .. }) = next.lt(expr_span, &from) { + Value::Int { + val: -100i64, + span: Span::unknown(), + } + } else { + Value::Int { + val: 100i64, + span: Span::unknown(), + } + } + } else { + to + }; + + // Check if the range counts up or down + let moves_up = matches!(from.lte(expr_span, &to), Ok(Value::Bool { val: true, .. })); + + // Convert the next value into the inctement + let incr = if let Value::Nothing { .. } = next { + if moves_up { + Value::Int { + val: 1i64, + span: Span::unknown(), + } + } else { + Value::Int { + val: -1i64, + span: Span::unknown(), + } + } + } else { + next.sub(operator.next_op_span, &from)? + }; + + let zero = Value::Int { + val: 0i64, + span: Span::unknown(), + }; + + // Increment must be non-zero, otherwise we iterate forever + if matches!(incr.eq(expr_span, &zero), Ok(Value::Bool { val: true, .. })) { + return Err(ShellError::CannotCreateRange(expr_span)); + } + + // If to > from, then incr > 0, otherwise we iterate forever + if let (Value::Bool { val: true, .. }, Value::Bool { val: false, .. }) = ( + to.gt(operator.span, &from)?, + incr.gt(operator.next_op_span, &zero)?, + ) { + return Err(ShellError::CannotCreateRange(expr_span)); + } + + // If to < from, then incr < 0, otherwise we iterate forever + if let (Value::Bool { val: true, .. }, Value::Bool { val: false, .. }) = ( + to.lt(operator.span, &from)?, + incr.lt(operator.next_op_span, &zero)?, + ) { + return Err(ShellError::CannotCreateRange(expr_span)); + } + + Ok(Range { + from, + incr, + to, + inclusion: operator.inclusion, + }) + } +} + impl IntoIterator for Range { type Item = Value; @@ -25,8 +119,7 @@ pub struct RangeIterator { span: Span, is_end_inclusive: bool, moves_up: bool, - one: Value, - negative_one: Value, + incr: Value, done: bool, } @@ -52,8 +145,7 @@ impl RangeIterator { span, is_end_inclusive: matches!(range.inclusion, RangeInclusion::Inclusive), done: false, - one: Value::Int { val: 1, span }, - negative_one: Value::Int { val: -1, span }, + incr: range.incr, } } } @@ -70,10 +162,10 @@ impl Iterator for RangeIterator { Ordering::Less } else { match (&self.curr, &self.end) { - (Value::Int { val: x, .. }, Value::Int { val: y, .. }) => x.cmp(y), - // (Value::Float { val: x, .. }, Value::Float { val: y, .. }) => x.cmp(y), - // (Value::Float { val: x, .. }, Value::Int { val: y, .. }) => x.cmp(y), - // (Value::Int { val: x, .. }, Value::Float { val: y, .. }) => x.cmp(y), + (Value::Int { val: curr, .. }, Value::Int { val: end, .. }) => curr.cmp(end), + // (Value::Float { val: curr, .. }, Value::Float { val: end, .. }) => curr.cmp(end), + // (Value::Float { val: curr, .. }, Value::Int { val: end, .. }) => curr.cmp(end), + // (Value::Int { val: curr, .. }, Value::Float { val: end, .. }) => curr.cmp(end), _ => { self.done = true; return Some(Value::Error { @@ -83,10 +175,15 @@ impl Iterator for RangeIterator { } }; - if self.moves_up - && (ordering == Ordering::Less || self.is_end_inclusive && ordering == Ordering::Equal) + let desired_ordering = if self.moves_up { + Ordering::Less + } else { + Ordering::Greater + }; + + if (ordering == desired_ordering) || (self.is_end_inclusive && ordering == Ordering::Equal) { - let next_value = self.curr.add(self.span, &self.one); + let next_value = self.curr.add(self.span, &self.incr); let mut next = match next_value { Ok(result) => result, @@ -98,22 +195,6 @@ impl Iterator for RangeIterator { }; std::mem::swap(&mut self.curr, &mut next); - Some(next) - } else if !self.moves_up - && (ordering == Ordering::Greater - || self.is_end_inclusive && ordering == Ordering::Equal) - { - let next_value = self.curr.add(self.span, &self.negative_one); - - let mut next = match next_value { - Ok(result) => result, - Err(error) => { - self.done = true; - return Some(Value::Error { error }); - } - }; - std::mem::swap(&mut self.curr, &mut next); - Some(next) } else { None