Improved errors and feedback

Signed-off-by: Heinz N. Gies <[email protected]>

static/language/epilog/for.md

@@ -21,9 +21,10 @@ initial value is `[]` and the operator `+` is used - so the above example can al
 For inserting a few keys into a record we could write
 
 ```tremor
+  let base = {"snot": "badger"}
   for new_values of
     case (k,v) => {k: v}
-  into {}
+  into base
   end
 ```
 

tests/script_runtime_error.rs

@@ -162,6 +162,10 @@ test_cases!(
     subslice_no_arr,
     subslice_out_of_bounds,
     // INSERT
+    bad_fold_op_record,
+    bad_fold_op,
+    bad_fold_op_record_imut,
+    bad_fold_op_imut,
     bad_fold_type,
     bad_fold_type_imut,
     bad_merge2,

tests/script_runtime_errors/bad_fold_op/error.txt

@@ -0,0 +1,5 @@
+Error: 
+    1 | for event of
+    2 |   case (k, v) => v
+      |                  ^ Invalid fold operation / for arrays
+    3 | into [] use / end

tests/script_runtime_errors/bad_fold_op/in

@@ -0,0 +1 @@
+[1,2,3]

tests/script_runtime_errors/bad_fold_op/script.tremor

@@ -0,0 +1,3 @@
+for event of
+  case (k, v) => v
+into [] use / end

tests/script_runtime_errors/bad_fold_op_imut/error.txt

@@ -0,0 +1,5 @@
+Error: 
+    1 | (for event of
+    2 |   case (k, v) => v
+      |                  ^ Invalid fold operation / for arrays
+    3 | into [] use / end)

tests/script_runtime_errors/bad_fold_op_imut/in

@@ -0,0 +1 @@
+[1,2,3]

tests/script_runtime_errors/bad_fold_op_imut/script.tremor

@@ -0,0 +1,3 @@
+(for event of
+  case (k, v) => v
+into [] use / end)

tests/script_runtime_errors/bad_fold_op_record/error.txt

@@ -0,0 +1,5 @@
+Error: 
+    1 | for event of
+    2 |   case (k, v) => {"#{k}": v}
+      |                  ^^^^^^^^^^^ Invalid fold operation / for records
+    3 | into {} use / end

tests/script_runtime_errors/bad_fold_op_record/in

@@ -0,0 +1 @@
+{"k":"v"}

tests/script_runtime_errors/bad_fold_op_record/script.tremor

@@ -0,0 +1,3 @@
+for event of
+  case (k, v) => {"#{k}": v}
+into {} use / end

tests/script_runtime_errors/bad_fold_op_record_imut/error.txt

@@ -0,0 +1,5 @@
+Error: 
+    1 | (for event of
+    2 |   case (k, v) => {"#{k}": v}
+      |                  ^^^^^^^^^^^ Invalid fold operation / for records
+    3 | into {} use / end)

tests/script_runtime_errors/bad_fold_op_record_imut/in

@@ -0,0 +1 @@
+{"k":"v"}

tests/script_runtime_errors/bad_fold_op_record_imut/script.tremor

@@ -0,0 +1,3 @@
+(for event of
+  case (k, v) => {"#{k}": v}
+into {} use / end)

tests/script_warning.rs

@@ -66,6 +66,7 @@ test_cases!(
     match_imut_no_default,
     match_imut_multiple_default,
     // INSERT
+    for_fold_op,
     record_perf,
     array_perf,
     nanotime,

tests/script_warnings/for_fold_op/script.tremor

@@ -0,0 +1,3 @@
+for event of
+  case (k, v) => v
+into 0 use + end

tests/script_warnings/for_fold_op/warning.txt

@@ -0,0 +1,5 @@
+Warning(general): 
+    1 | for event of
+    2 |   case (k, v) => v
+    3 | into 0 use + end
+      | ^^^^^^^^^^^^^^^^ The `use <op>` syntax is experimental and may be deprecated or replaced

tremor-script/src/ast.rs

@@ -1562,9 +1562,15 @@ pub struct Merge<'script> {
 impl_expr!(Merge);
 
 /// Fold operation for for comprehensiosn
-#[derive(Clone, Debug, PartialEq, Serialize)]
+#[derive(Clone, Debug, PartialEq, Serialize, Copy)]
 pub struct ComprehensionFoldOp(pub(crate) BinOpKind);
 
+impl Default for ComprehensionFoldOp {
+    fn default() -> Self {
+        Self(BinOpKind::Add)
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Serialize)]
 /// Encapsulates a structure comprehension form
 pub struct Comprehension<'script, Ex: Expression + 'script> {
@@ -1579,7 +1585,7 @@ pub struct Comprehension<'script, Ex: Expression + 'script> {
     /// Initial value
     pub initial: ImutExpr<'script>,
     /// Fold operation
-    pub fold: ComprehensionFoldOp,
+    pub fold: Option<ComprehensionFoldOp>,
     /// Case applications against target elements
     pub cases: ComprehensionCases<'script, Ex>,
 }

tremor-script/src/ast/raw.rs

@@ -1229,7 +1229,7 @@ impl<'script> Upable<'script> for MergeRaw<'script> {
         })
     }
 }
-#[derive(Clone, Debug, PartialEq, Serialize)]
+#[derive(Clone, Debug, PartialEq, Serialize, Copy)]
 pub struct ComprehensionFoldOpRaw(pub BinOpKind);
 
 #[derive(Clone, Debug, PartialEq, Serialize)]
@@ -1241,7 +1241,7 @@ where
     pub target: ImutExprRaw<'script>,
     pub cases: ComprehensionCasesRaw<'script, Ex>,
     pub initial: ImutExprRaw<'script>,
-    pub fold: ComprehensionFoldOpRaw,
+    pub fold: Option<ComprehensionFoldOpRaw>,
     pub(crate) mid: Box<NodeMeta>,
 }
 
@@ -1256,9 +1256,17 @@ where
     fn up<'registry>(self, helper: &mut Helper<'script, 'registry>) -> Result<Self::Target> {
         // We compute the target before shadowing the key and value
 
+        if self.fold.is_some() {
+            helper.warn(
+                self.extent().expand_lines(2),
+                self.extent(),
+                &"The `use <op>` syntax is experimental and may be deprecated or replaced",
+                warning::Class::General,
+            );
+        }
         let target = self.target.up(helper)?;
         let initial = self.initial.up(helper)?;
-        let fold = ComprehensionFoldOp(self.fold.0);
+        let fold = self.fold.map(|f| ComprehensionFoldOp(f.0));
 
         // We know that each case will have a key and a value as a shadowed
         // variable so we reserve two ahead of time so we know what id's those

tremor-script/src/grammar.lalrpop

@@ -1255,16 +1255,9 @@ For: ComprehensionRaw<'input, ExprRaw<'input>> = {
         cases,
         mid: NodeMeta::new_box(start, end),
         initial: ImutExprRaw::Literal(LiteralRaw{value: Value::array(), mid: NodeMeta::new_box(start, end)}),
-        fold: ComprehensionFoldOpRaw(BinOpKind::Add)
+        fold: None
     },
-    <start:@L> "for" <target:ComplexExprImut> "of" <cases:ForCaseClauses> "into" <initial:ComplexExprImut> "end" <end:@L> => ComprehensionRaw{
-        target,
-        cases,
-        mid: NodeMeta::new_box(start, end),
-        initial,
-        fold: ComprehensionFoldOpRaw(BinOpKind::Add)
-    },
-    <start:@L> "for" <target:ComplexExprImut> "of" <cases:ForCaseClauses> "into" <initial:ComplexExprImut> "use" <fold:FoldOperator> "end" <end:@L> => ComprehensionRaw{
+    <start:@L> "for" <target:ComplexExprImut> "of" <cases:ForCaseClauses> "into" <initial:ComplexExprImut> <fold:MaybeFoldOperator> "end" <end:@L> => ComprehensionRaw{
         target,
         cases,
         mid: NodeMeta::new_box(start, end),
@@ -1273,6 +1266,11 @@ For: ComprehensionRaw<'input, ExprRaw<'input>> = {
     },
 }
 
+
+MaybeFoldOperator: Option<ComprehensionFoldOpRaw> = {
+    ("use" <FoldOperator>)? => <>,
+}
+
 FoldOperator: ComprehensionFoldOpRaw = {
     "+" => ComprehensionFoldOpRaw(BinOpKind::Add),
     "-" => ComprehensionFoldOpRaw(BinOpKind::Sub),
@@ -1302,16 +1300,10 @@ ForImut: ComprehensionRaw<'input, ImutExprRaw<'input>> = {
         cases,
         mid: NodeMeta::new_box(start, end),
         initial: ImutExprRaw::Literal(LiteralRaw{value: Value::array(), mid: NodeMeta::new_box(start, end)}),
-        fold: ComprehensionFoldOpRaw(BinOpKind::Add)
+        fold: None
     },
-    <start:@L> "for" <target:ComplexExprImut> "of" <cases:ForCaseClausesImut> "into" <initial:ComplexExprImut> "end" <end:@L> => ComprehensionRaw{
-        target,
-        cases,
-        mid: NodeMeta::new_box(start, end),
-        initial,
-        fold: ComprehensionFoldOpRaw(BinOpKind::Add)
-    },
-    <start:@L> "for" <target:ComplexExprImut> "of" <cases:ForCaseClausesImut> "into" <initial:ComplexExprImut> "use" <fold:FoldOperator> "end" <end:@L> => ComprehensionRaw{
+
+    <start:@L> "for" <target:ComplexExprImut> "of" <cases:ForCaseClausesImut> "into" <initial:ComplexExprImut> <fold:MaybeFoldOperator> "end" <end:@L> => ComprehensionRaw{
         target,
         cases,
         mid: NodeMeta::new_box(start, end),

tremor-script/src/interpreter/expr.rs

@@ -382,8 +382,9 @@ impl<'script> Expr<'script> {
         } else {
             Value::const_null()
         };
+        let fold = expr.fold.unwrap_or_default();
         // short circuite for common cases
-        if expr.fold == ComprehensionFoldOp(BinOpKind::Add) {
+        if fold == ComprehensionFoldOp(BinOpKind::Add) {
             if result.is_array() {
                 let a = result.into_array().unwrap_or_default();
                 return self
@@ -413,27 +414,35 @@ impl<'script> Expr<'script> {
                     if opts.result_needed {
                         let v = v.into_owned();
                         if let Some(lhs) = result.as_array_mut() {
-                            match expr.fold {
+                            match fold {
                                 ComprehensionFoldOp(BinOpKind::Add) => {
                                     lhs.push(v);
                                 }
-                                ComprehensionFoldOp(_) => {
-                                    return err_generic(expr, l, &"Invalid fold operation");
+                                ComprehensionFoldOp(op) => {
+                                    return err_generic(
+                                        expr,
+                                        l,
+                                        &format!("Invalid fold operation {op} for arrays"),
+                                    );
                                 }
                             }
                         } else if let Some(lhs) = result.as_object_mut() {
-                            match expr.fold {
+                            match fold {
                                 ComprehensionFoldOp(BinOpKind::Add) => {
                                     for (k, v) in v.try_into_object().add_span(expr, l)? {
                                         lhs.insert(k, v);
                                     }
                                 }
-                                ComprehensionFoldOp(_) => {
-                                    return err_generic(expr, l, &"Invalid fold operation");
+                                ComprehensionFoldOp(op) => {
+                                    return err_generic(
+                                        expr,
+                                        l,
+                                        &format!("Invalid fold operation {op} for records"),
+                                    );
                                 }
                             }
                         } else {
-                            result = stry!(exec_binary_numeric(self, e, expr.fold.0, &result, &v))
+                            result = stry!(exec_binary_numeric(self, e, fold.0, &result, &v))
                                 .into_owned();
                         }
                     }

tremor-script/src/interpreter/imut_expr.rs

@@ -409,8 +409,10 @@ impl<'script> ImutExpr<'script> {
         } else {
             Value::const_null()
         };
+        let fold = expr.fold.unwrap_or_default();
+
         // short circuite for common cases
-        if expr.fold == ComprehensionFoldOp(BinOpKind::Add) {
+        if fold == ComprehensionFoldOp(BinOpKind::Add) {
             if result.is_array() {
                 let a = result.into_array().unwrap_or_default();
                 return self
@@ -438,28 +440,36 @@ impl<'script> ImutExpr<'script> {
                     // NOTE: We are creating a new value so we have to clone;
                     let v = v.into_owned();
                     if let Some(lhs) = result.as_array_mut() {
-                        match expr.fold {
+                        match fold {
                             ComprehensionFoldOp(BinOpKind::Add) => {
                                 lhs.push(v);
                             }
-                            ComprehensionFoldOp(_) => {
-                                return err_generic(expr, l, &"Invalid fold operation")
+                            ComprehensionFoldOp(op) => {
+                                return err_generic(
+                                    expr,
+                                    l,
+                                    &format!("Invalid fold operation {op} for arrays"),
+                                );
                             }
                         }
                     } else if let Some(lhs) = result.as_object_mut() {
-                        match expr.fold {
+                        match fold {
                             ComprehensionFoldOp(BinOpKind::Add) => {
                                 for (k, v) in v.try_into_object().add_span(expr, l)? {
                                     lhs.insert(k, v);
                                 }
                             }
-                            ComprehensionFoldOp(_) => {
-                                return err_generic(expr, l, &"Invalid fold operation")
+                            ComprehensionFoldOp(op) => {
+                                return err_generic(
+                                    expr,
+                                    l,
+                                    &format!("Invalid fold operation {op} for records"),
+                                );
                             }
                         }
                     } else {
-                        result = stry!(exec_binary_numeric(self, e, expr.fold.0, &result, &v))
-                            .into_owned();
+                        result =
+                            stry!(exec_binary_numeric(self, e, fold.0, &result, &v)).into_owned();
                     }
 
                     continue 'outer;