Marko v3: Improved for loop parsing

taglibs/core/core-transformer.js

@@ -10,7 +10,19 @@ var coreAttrHandlers = [
                 return false;
             }
 
-            var loopNode = createLoopNode(forArgument, null, this.builder);
+            var loopNode;
+
+            try {
+                loopNode = createLoopNode(forArgument, null, this.builder);
+            } catch(e) {
+                if (e.code === 'INVALID_FOR') {
+                    this.addError(e.message);
+                    return;
+                } else {
+                    throw e;
+                }
+            }
+
 
             //Surround the existing node with the newly created loop node
             // NOTE: The loop node will be one of the following:

taglibs/core/for-tag.js

@@ -9,12 +9,15 @@ module.exports = function codeGenerator(elNode, codegen) {
 
     var builder = codegen.builder;
 
-    var loopNode = createLoopNode(argument, elNode.body, builder);
+    try {
-
+        var loopNode = createLoopNode(argument, elNode.body, builder);
-    if (loopNode.error) {
+        return loopNode;
-        codegen.addError(loopNode.error);
+    } catch(e) {
-        return elNode;
+        if (e.code === 'INVALID_FOR') {
+            codegen.addError(e.message);
+        } else {
+            throw e;
+        }
     }
 
-    return loopNode;
 };

taglibs/core/util/createLoopNode.js

@@ -3,10 +3,6 @@ var parseFor = require('./parseFor');
 function createLoopNode(str, body, builder) {
     var forDef = parseFor(str);
 
-    if (forDef.error) {
-        return forDef;
-    }
-
     forDef.body = body;
 
     if (forDef.loopType === 'ForEach') {

taglibs/core/util/parseFor.js

@@ -1,8 +1,6 @@
 'use strict';
-var Expression = require('../../../compiler/ast/Expression');
-var Literal = require('../../../compiler/ast/Literal');
-var Identifier = require('../../../compiler/ast/Identifier');
 var removeComments = require('../../../compiler/util/removeComments');
+var compiler = require('../../../compiler');
 
 var integerRegExp = /^-?\d+$/;
 var numberRegExp = /^-?(?:\d+|\d+\.\d*|\d*\.\d+|\d+\.\d+)$/;
@@ -62,18 +60,39 @@ var tokenizer = require('../../../compiler/util/tokenizer').create([
     }
 ]);
 
+function throwError(message) {
+    var error = new Error(message);
+    error.code = 'INVALID_FOR';
+    throw error;
+}
 
-function createNumberExpression(str) {
+function buildIdentifier(name, errorMessage) {
+    try {
+        return compiler.builder.identifier(name);
+    } catch(e) {
+        throwError(errorMessage + ': ' + e.message);
+    }
+}
+
+function parseExpression(str, errorMessage) {
+    try {
+        return compiler.builder.parseExpression(str);
+    } catch(e) {
+        throwError(errorMessage + ': ' + e.message);
+    }
+}
+
+function createNumberExpression(str, errorMessage) {
     if (str == null) {
         return null;
     }
 
     if (integerRegExp.test(str)) {
-        return new Literal({value: parseInt(str, 10)});
+        return compiler.builder.literal(parseInt(str, 10));
     } else if (numberRegExp.test(str)) {
-        return new Literal({value: parseFloat(str)});
+        return compiler.builder.literal(parseFloat(str));
     } else {
-        return new Expression({value: str});
+        return parseExpression(str, errorMessage);
     }
 }
 
@@ -143,6 +162,12 @@ module.exports = function(str) {
             case 'iterator':
                 iteratorExpression = part;
                 break;
+            case 'pipe':
+                if (part.length !== 0) {
+                    throwError('Unexpected input: ' + part);
+                    return;
+                }
+                break;
         }
     }
 
@@ -190,9 +215,7 @@ module.exports = function(str) {
                         forTest = str.substring(prevToken.end, token.start);
                         forUpdate = str.substring(token.end);
                     } else {
-                        return {
+                        throwError('Invalid native for loop. Expected format: <init>; <test>; <update>');
-                            error: 'Invalid native for loop. Expected format: <init>; <test>; <update>'
-                        };
                     }
 
                     prevToken = token;
@@ -229,42 +252,46 @@ module.exports = function(str) {
     finishPrevPart(str.length);
 
     if (loopType === 'ForEach') {
-        var nameValue = varName.split(',');
+        var nameValue = varName.split(/\s*,\s*/);
         if (nameValue.length === 2) {
-            nameVarName = new Identifier({name: nameValue[0]});
+            nameVarName = buildIdentifier(nameValue[0], 'Invalid name variable');
-            valueVarName = new Identifier({name: nameValue[1]});
+            valueVarName = buildIdentifier(nameValue[1], 'Invalid value variable');
+            varName = null;
             loopType = 'ForEachProp';
         }
     }
 
     if (inExpression) {
-        inExpression = new Expression({value: inExpression});
+        inExpression = parseExpression(inExpression, 'Invalid "in" expression');
     }
 
     if (separatorExpression) {
-        separatorExpression = new Expression({value: separatorExpression});
+        separatorExpression = parseExpression(separatorExpression, 'Invalid "separator" expression');
     }
 
     if (iteratorExpression) {
-        iteratorExpression = new Expression({value: iteratorExpression});
+        iteratorExpression = parseExpression(iteratorExpression, 'Invalid "iterator" expression');
     }
 
     if (fromExpression) {
-        fromExpression = createNumberExpression(fromExpression);
+        fromExpression = createNumberExpression(fromExpression, 'Invalid "from" expression');
     }
 
     if (toExpression) {
-        toExpression = createNumberExpression(toExpression);
+        toExpression = createNumberExpression(toExpression, 'Invalid "to" expression');
     }
 
     if (stepExpression) {
-        stepExpression = createNumberExpression(stepExpression);
+        stepExpression = createNumberExpression(stepExpression, 'Invalid "step" expression');
+    }
+
+    if (varName != null) {
+        varName = buildIdentifier(varName, 'Invalid variable name');
     }
 
-    varName = new Identifier({name: varName});
 
     if (statusVarName) {
-        statusVarName = new Identifier({name: statusVarName});
+        statusVarName = buildIdentifier(statusVarName, 'Invalid status-var option');
     }
 
     // No more tokens... now we need to sort out what happened
@@ -294,9 +321,7 @@ module.exports = function(str) {
         };
     } else if (loopType === 'For') {
         if (forTest == null) {
-            return {
+            throwError('Invalid native for loop. Expected format: <init>; <test>; <update>');
-                error: 'Invalid native for loop. Expected format: <init>; <test>; <update>'
-            };
         }
         return {
             'loopType': loopType,
@@ -305,8 +330,6 @@ module.exports = function(str) {
             'update': forUpdate
         };
     } else {
-        return {
+        throwError('Invalid for loop');
-            'error': 'Invalid for loop'
-        };
     }
 };

test/fixtures/parseFor/autotest/forEach-comment/expected.json

@@ -5,7 +5,7 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "colors"
+    "name": "colors"
   }
 }

test/fixtures/parseFor/autotest/forEach-crazy/expected.json

@@ -5,19 +5,54 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "BinaryExpression",
-    "value": "(a in test | 0)"
+    "left": {
+      "type": "BinaryExpression",
+      "left": {
+        "type": "Identifier",
+        "name": "a"
+      },
+      "operator": "in",
+      "right": {
+        "type": "Identifier",
+        "name": "test"
+      }
+    },
+    "operator": "|",
+    "right": {
+      "type": "Literal",
+      "value": 0
+    }
   },
   "separator": {
-    "type": "Expression",
+    "type": "ConditionalExpression",
-    "value": "(iterator==1 ? 'foo' : 'bar')"
+    "test": {
+      "type": "BinaryExpression",
+      "left": {
+        "type": "Identifier",
+        "name": "iterator"
+      },
+      "operator": "==",
+      "right": {
+        "type": "Literal",
+        "value": 1
+      }
+    },
+    "consequent": {
+      "type": "Literal",
+      "value": "foo"
+    },
+    "alternate": {
+      "type": "Literal",
+      "value": "bar"
+    }
   },
   "statusVarName": {
     "type": "Identifier",
     "name": "loop"
   },
   "iterator": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "my Iterator"
+    "name": "myIterator"
   }
 }

test/fixtures/parseFor/autotest/forEach-crazy/input.txt

@@ -1 +1 @@
-color in (a in test | 0) | status-var=loop separator=(iterator==1 ? 'foo' : 'bar') iterator=my Iterator
+color in (a in test | 0) | status-var=loop separator=(iterator==1 ? 'foo' : 'bar') iterator=myIterator

test/fixtures/parseFor/autotest/forEach-invalid-option/expected.json

@@ -0,0 +1,3 @@
+{
+  "error": "Unexpected input: foo"
+}

test/fixtures/parseFor/autotest/forEach-invalid-option/input.txt

@@ -0,0 +1 @@
+color in colors | foo

test/fixtures/parseFor/autotest/forEach-invalid-option2/expected.json

@@ -0,0 +1,3 @@
+{
+  "error": "Invalid status-var option: Invalid JavaScript identifier: test foo"
+}

test/fixtures/parseFor/autotest/forEach-invalid-option2/input.txt

@@ -0,0 +1 @@
+color in colors | status-var=test foo

test/fixtures/parseFor/autotest/forEach-iterator/expected.json

@@ -5,11 +5,11 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "colors"
+    "name": "colors"
   },
   "iterator": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "myIterator"
+    "name": "myIterator"
   }
 }

test/fixtures/parseFor/autotest/forEach-nesting/expected.json

@@ -5,7 +5,40 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "ArrayExpression",
-    "value": "[(({a} in test | 0))]"
+    "elements": [
+      {
+        "type": "BinaryExpression",
+        "left": {
+          "type": "BinaryExpression",
+          "left": {
+            "type": "ObjectExpression",
+            "properties": [
+              {
+                "type": "Property",
+                "key": {
+                  "type": "Identifier",
+                  "name": "a"
+                },
+                "value": {
+                  "type": "Identifier",
+                  "name": "a"
+                }
+              }
+            ]
+          },
+          "operator": "in",
+          "right": {
+            "type": "Identifier",
+            "name": "test"
+          }
+        },
+        "operator": "|",
+        "right": {
+          "type": "Literal",
+          "value": 0
+        }
+      }
+    ]
   }
 }

test/fixtures/parseFor/autotest/forEach-simple/expected.json

@@ -5,7 +5,7 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "colors"
+    "name": "colors"
   }
 }

test/fixtures/parseFor/autotest/forEach-status-var-separator-iterator/expected.json

@@ -5,19 +5,19 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "colors"
+    "name": "colors"
   },
   "separator": {
-    "type": "Expression",
+    "type": "Literal",
-    "value": "','"
+    "value": ","
   },
   "statusVarName": {
     "type": "Identifier",
     "name": "loop"
   },
   "iterator": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "myIterator"
+    "name": "myIterator"
   }
 }

test/fixtures/parseFor/autotest/forEach-status-var-separator/expected.json

@@ -5,12 +5,12 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "colors"
+    "name": "colors"
   },
   "separator": {
-    "type": "Expression",
+    "type": "Literal",
-    "value": "','"
+    "value": ","
   },
   "statusVarName": {
     "type": "Identifier",

test/fixtures/parseFor/autotest/forEach-status-var/expected.json

@@ -5,8 +5,8 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Identifier",
-    "value": "colors"
+    "name": "colors"
   },
   "statusVarName": {
     "type": "Identifier",

test/fixtures/parseFor/autotest/forEach-strings/expected.json

@@ -5,7 +5,7 @@
     "name": "color"
   },
   "in": {
-    "type": "Expression",
+    "type": "Literal",
-    "value": "' | separator=\",\";'"
+    "value": " | separator=\",\";"
   }
 }

test/fixtures/parseFor/autotest/forEachProp/expected.json

@@ -0,0 +1,38 @@
+{
+  "loopType": "ForEachProp",
+  "nameVarName": {
+    "type": "Identifier",
+    "name": "name"
+  },
+  "valueVarName": {
+    "type": "Identifier",
+    "name": "value"
+  },
+  "in": {
+    "type": "ObjectExpression",
+    "properties": [
+      {
+        "type": "Property",
+        "key": {
+          "type": "Literal",
+          "value": "foo"
+        },
+        "value": {
+          "type": "Literal",
+          "value": "low"
+        }
+      },
+      {
+        "type": "Property",
+        "key": {
+          "type": "Literal",
+          "value": "bar"
+        },
+        "value": {
+          "type": "Literal",
+          "value": "high"
+        }
+      }
+    ]
+  }
+}

test/fixtures/parseFor/autotest/forEachProp/input.txt

@@ -0,0 +1 @@
+name, value in {'foo': 'low', 'bar': 'high'}

test/fixtures/parseFor/autotest/forRange-to-expr/expected.json

@@ -9,7 +9,15 @@
     "value": 0
   },
   "to": {
-    "type": "Expression",
+    "type": "MemberExpression",
-    "value": "'abc'.length"
+    "object": {
+      "type": "Literal",
+      "value": "abc"
+    },
+    "property": {
+      "type": "Identifier",
+      "name": "length"
+    },
+    "computed": false
   }
 }

test/fixtures/render/autotest-pending/error-for/template.marko

@@ -1,17 +0,0 @@
-<for>
-    Missing each attribute
-</for>
-
-<for each="item">
-</for>
-
-<for each="item in items" invalid="true">
-    Invalid attribute
-</for>
-
-<for each="item in items" separator="${;">
-    Invalid separator
-</for>
-
-<div for="item in items; invalid=true">
-</div>

test/fixtures/render/autotest-pending/error-for/test.js

@@ -1,22 +0,0 @@
-var expect = require('chai').expect;
-
-exports.templateData = {};
-
-exports.options = {
-    handleCompileError: function(e) {
-        expect(e.toString()).to.contain('template.marko:8:0');
-        expect(e.toString()).to.contain('does not support attribute "invalid"');
-
-        expect(e.toString()).to.contain('template.marko:12:0');
-        expect(e.toString()).to.contain('Invalid attribute value of "${;"');
-
-        expect(e.toString()).to.contain('template.marko:1:0');
-        expect(e.toString()).to.contain('"each" attribute is required');
-
-        expect(e.toString()).to.contain('template.marko:5:0');
-        expect(e.toString()).to.contain('Invalid each attribute of "item"');
-
-        expect(e.toString()).to.contain('template.marko:16:0');
-        expect(e.toString()).to.contain('Invalid for attribute of "item in items; invalid=true"');
-    }
-};

test/fixtures/render/autotest/for-tag-invalid-option/expected.html

@@ -0,0 +1 @@
+<div>red</div><div>green</div><div>blue</div>

test/fixtures/render/autotest/for-tag-invalid-option/template.marko

@@ -0,0 +1,5 @@
+<for(item in ['red', 'green', 'blue'] | foo)>
+    <div>
+        ${item}
+    </div>
+</for>

test/fixtures/render/autotest/for-tag-invalid-option/test.js

@@ -0,0 +1,7 @@
+var expect = require('chai').expect;
+
+exports.templateData = {};
+
+exports.checkError = function(err) {
+    expect(err.toString()).to.contain('template.marko:1:0] Unexpected input: foo');
+};

test/parseFor-test.js

@@ -13,10 +13,21 @@ describe('parseFor' , function() {
     autotest.scanDir(
         autoTestDir,
         function run(dir) {
-            var inputPath = path.join(dir, 'input.txt');
+            let inputPath = path.join(dir, 'input.txt');
-            var input = fs.readFileSync(inputPath, {encoding: 'utf8'});
+            let input = fs.readFileSync(inputPath, {encoding: 'utf8'});
-            var parsed = parseFor(input);
+            try {
-            return parsed;
+                let parsed = parseFor(input);
+                return parsed;
+            } catch(e) {
+                if (e.code === 'INVALID_FOR') {
+                    return {
+                        error: e.message
+                    };
+                } else {
+                    throw e;
+                }
+
+            }
         },
         {
             deepEqual: true,