Improve range parsing and handling (#2506)

* Improve range parsing and handling

* linting

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

@@ -86,8 +86,13 @@ struct RangeIterator {
 
 impl RangeIterator {
     pub fn new(range: Range, tag: Tag) -> RangeIterator {
+        let start = match range.from.0.item {
+            Primitive::Nothing => Primitive::Int(0.into()),
+            x => x,
+        };
+
         RangeIterator {
-            curr: range.from.0.item,
+            curr: start,
             end: range.to.0.item,
             tag,
             is_end_inclusive: matches!(range.to.1, RangeInclusion::Inclusive),

crates/nu-cli/src/completion/engine.rs

@@ -60,8 +60,12 @@ impl<'s> Flatten<'s> {
             }
             Expression::Range(range) => {
                 let mut result = Vec::new();
-                result.append(&mut self.expression(&range.left));
-                result.append(&mut self.expression(&range.right));
+                if let Some(left) = &range.left {
+                    result.append(&mut self.expression(left));
+                }
+                if let Some(right) = &range.right {
+                    result.append(&mut self.expression(right));
+                }
                 result
             }
 

crates/nu-cli/src/evaluate/evaluator.rs

@@ -61,11 +61,18 @@ pub(crate) async fn evaluate_baseline_expr(
             }
         }
         Expression::Range(range) => {
-            let left = &range.left;
-            let right = &range.right;
+            let left = if let Some(left) = &range.left {
+                evaluate_baseline_expr(&left, registry, it, vars, env).await?
+            } else {
+                Value::nothing()
+            };
+
+            let right = if let Some(right) = &range.right {
+                evaluate_baseline_expr(&right, registry, it, vars, env).await?
+            } else {
+                Value::nothing()
+            };
 
-            let left = evaluate_baseline_expr(&left, registry, it, vars, env).await?;
-            let right = evaluate_baseline_expr(&right, registry, it, vars, env).await?;
             let left_span = left.tag.span;
             let right_span = right.tag.span;
 

crates/nu-parser/src/parse.rs

@@ -236,39 +236,72 @@ fn trim_quotes(input: &str) -> String {
 }
 
 /// Parse a numeric range
-fn parse_range(lite_arg: &Spanned<String>) -> (SpannedExpression, Option<ParseError>) {
+fn parse_range(
+    lite_arg: &Spanned<String>,
+    registry: &dyn SignatureRegistry,
+) -> (SpannedExpression, Option<ParseError>) {
+    let lite_arg_span_start = lite_arg.span.start();
+    let lite_arg_len = lite_arg.item.len();
+    let dotdot_pos = lite_arg.item.find("..");
     let numbers: Vec<_> = lite_arg.item.split("..").collect();
 
     if numbers.len() != 2 {
-        (
+        return (
             garbage(lite_arg.span),
             Some(ParseError::mismatch("range", lite_arg.clone())),
-        )
-    } else if let Ok(lhs) = numbers[0].parse::<i64>() {
-        if let Ok(rhs) = numbers[1].parse::<i64>() {
-            (
-                SpannedExpression::new(
-                    Expression::range(
-                        SpannedExpression::new(Expression::integer(lhs), lite_arg.span),
-                        lite_arg.span,
-                        SpannedExpression::new(Expression::integer(rhs), lite_arg.span),
-                    ),
-                    lite_arg.span,
-                ),
-                None,
-            )
-        } else {
-            (
-                garbage(lite_arg.span),
-                Some(ParseError::mismatch("range", lite_arg.clone())),
-            )
-        }
-    } else {
-        (
-            garbage(lite_arg.span),
-            Some(ParseError::mismatch("range", lite_arg.clone())),
-        )
+        );
     }
+
+    let dotdot_pos = dotdot_pos.expect("Internal error: range .. can't be found but should be");
+
+    let lhs = numbers[0].to_string().spanned(Span::new(
+        lite_arg_span_start,
+        lite_arg_span_start + dotdot_pos,
+    ));
+    let rhs = numbers[1].to_string().spanned(Span::new(
+        lite_arg_span_start + dotdot_pos + 2,
+        lite_arg_span_start + lite_arg_len,
+    ));
+
+    let left_hand_open = dotdot_pos == 0;
+    let right_hand_open = dotdot_pos == lite_arg_len - 2;
+
+    let left = if left_hand_open {
+        None
+    } else if let (left, None) = parse_arg(SyntaxShape::Number, registry, &lhs) {
+        Some(left)
+    } else {
+        return (
+            garbage(lite_arg.span),
+            Some(ParseError::mismatch("range", lhs)),
+        );
+    };
+
+    let right = if right_hand_open {
+        None
+    } else if let (right, None) = parse_arg(SyntaxShape::Number, registry, &rhs) {
+        Some(right)
+    } else {
+        return (
+            garbage(lite_arg.span),
+            Some(ParseError::mismatch("range", rhs)),
+        );
+    };
+
+    (
+        SpannedExpression::new(
+            Expression::range(
+                left,
+                Span::new(
+                    lite_arg_span_start + dotdot_pos,
+                    lite_arg_span_start + dotdot_pos + 2,
+                ),
+                right,
+            ),
+            lite_arg.span,
+        ),
+        None,
+    )
 }
 
 /// Parse any allowed operator, including word-based operators
@@ -685,7 +718,8 @@ fn parse_arg(
     registry: &dyn SignatureRegistry,
     lite_arg: &Spanned<String>,
 ) -> (SpannedExpression, Option<ParseError>) {
-    if lite_arg.item.starts_with('$') {
+    // If this is a full column path (and not a range), just parse it here
+    if lite_arg.item.starts_with('$') && !lite_arg.item.contains("..") {
         return parse_full_column_path(&lite_arg, registry);
     }
 
@@ -745,7 +779,7 @@ fn parse_arg(
             )
         }
 
-        SyntaxShape::Range => parse_range(&lite_arg),
+        SyntaxShape::Range => parse_range(&lite_arg, registry),
         SyntaxShape::Operator => parse_operator(&lite_arg),
         SyntaxShape::Unit => parse_unit(&lite_arg),
         SyntaxShape::Path => {

crates/nu-parser/src/shapes.rs

@@ -65,9 +65,13 @@ pub fn expression_to_flat_shape(e: &SpannedExpression) -> Vec<Spanned<FlatShape>
         }
         Expression::Range(range) => {
             let mut output = vec![];
-            output.append(&mut expression_to_flat_shape(&range.left));
+            if let Some(left) = &range.left {
+                output.append(&mut expression_to_flat_shape(left));
+            }
             output.push(FlatShape::DotDot.spanned(range.dotdot));
-            output.append(&mut expression_to_flat_shape(&range.right));
+            if let Some(right) = &range.right {
+                output.append(&mut expression_to_flat_shape(right));
+            }
             output
         }
         Expression::Boolean(_) => vec![FlatShape::Keyword.spanned(e.span)],

crates/nu-protocol/src/hir.rs

@@ -600,6 +600,21 @@ impl SpannedExpression {
             Expression::Binary(binary) => {
                 binary.left.has_shallow_it_usage() || binary.right.has_shallow_it_usage()
             }
+            Expression::Range(range) => {
+                let left = if let Some(left) = &range.left {
+                    left.has_shallow_it_usage()
+                } else {
+                    false
+                };
+
+                let right = if let Some(right) = &range.right {
+                    right.has_shallow_it_usage()
+                } else {
+                    false
+                };
+
+                left || right
+            }
             Expression::Variable(Variable::It(_)) => true,
             Expression::Path(path) => path.head.has_shallow_it_usage(),
             Expression::List(list) => {
@@ -890,20 +905,27 @@ impl ShellTypeName for Synthetic {
 
 #[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash, Deserialize, Serialize)]
 pub struct Range {
-    pub left: SpannedExpression,
+    pub left: Option<SpannedExpression>,
     pub dotdot: Span,
-    pub right: SpannedExpression,
+    pub right: Option<SpannedExpression>,
 }
 
 impl PrettyDebugWithSource for Range {
     fn pretty_debug(&self, source: &str) -> DebugDocBuilder {
         b::delimit(
             "<",
-            self.left.pretty_debug(source)
-                + b::space()
+            (if let Some(left) = &self.left {
+                left.pretty_debug(source)
+            } else {
+                DebugDocBuilder::blank()
+            }) + b::space()
                 + b::keyword(self.dotdot.slice(source))
                 + b::space()
-                + self.right.pretty_debug(source),
+                + (if let Some(right) = &self.right {
+                    right.pretty_debug(source)
+                } else {
+                    DebugDocBuilder::blank()
+                }),
             ">",
         )
         .group()
@@ -1083,7 +1105,11 @@ impl Expression {
         Expression::Literal(Literal::Operator(operator))
     }
 
-    pub fn range(left: SpannedExpression, dotdot: Span, right: SpannedExpression) -> Expression {
+    pub fn range(
+        left: Option<SpannedExpression>,
+        dotdot: Span,
+        right: Option<SpannedExpression>,
+    ) -> Expression {
         Expression::Range(Box::new(Range {
             left,
             dotdot,

tests/shell/pipeline/commands/internal.rs

@@ -454,6 +454,54 @@ fn list_with_commas() {
     assert_eq!(actual.out, "6");
 }
 
+#[test]
+fn range_with_left_var() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+        echo [[size]; [3]] | echo $it.size..10 | math sum
+        "#
+    );
+
+    assert_eq!(actual.out, "52");
+}
+
+#[test]
+fn range_with_right_var() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+        echo [[size]; [30]] | echo 4..$it.size | math sum
+        "#
+    );
+
+    assert_eq!(actual.out, "459");
+}
+
+#[test]
+fn range_with_open_left() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+        echo ..30 | math sum
+        "#
+    );
+
+    assert_eq!(actual.out, "465");
+}
+
+#[test]
+fn range_with_open_right() {
+    let actual = nu!(
+        cwd: ".",
+        r#"
+        echo 5.. | first 10 | math sum
+        "#
+    );
+
+    assert_eq!(actual.out, "95");
+}
+
 #[test]
 fn it_expansion_of_tables() {
     let actual = nu!(