Move Jison parsing out of AST into helpers

5 files changed, 68 insertions, 102 deletions

diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js
index efb4ece..9f25443 100644
--- a/lib/handlebars/compiler/ast.js
+++ b/lib/handlebars/compiler/ast.js
@@ -10,20 +10,12 @@ var AST = {
     this.strip = strip;
   },
 
-  MustacheStatement: function(rawParams, open, strip, locInfo) {
+  MustacheStatement: function(sexpr, escaped, strip, locInfo) {
     this.loc = locInfo;
     this.type = 'MustacheStatement';
 
-    this.sexpr = rawParams;
-
-    // Open may be a string parsed from the parser or a passed boolean flag
-    if (open != null && open.charAt) {
-      // Must use charAt to support IE pre-10
-      var escapeFlag = open.charAt(3) || open.charAt(2);
-      this.escaped = escapeFlag !== '{' && escapeFlag !== '&';
-    } else {
-      this.escaped = !!open;
-    }
+    this.sexpr = sexpr;
+    this.escaped = escaped;
 
     this.strip = strip;
   },
@@ -63,43 +55,22 @@ var AST = {
     strip.inlineStandalone = true;
   },
 
-  SubExpression: function(rawParams, hash, locInfo) {
+  SubExpression: function(path, params, hash, locInfo) {
     this.loc = locInfo;
 
     this.type = 'SubExpression';
-    this.path = rawParams[0];
-    this.params = rawParams.slice(1);
+    this.path = path;
+    this.params = params || [];
     this.hash = hash;
   },
 
-  PathExpression: function(data, parts, locInfo) {
+  PathExpression: function(data, depth, parts, original, locInfo) {
     this.loc = locInfo;
     this.type = 'PathExpression';
 
-    var original = '',
-        dig = [],
-        depth = 0,
-        depthString = '';
-
-    for(var i=0,l=parts.length; i<l; i++) {
-      var part = parts[i].part;
-      original += (parts[i].separator || '') + part;
-
-      if (part === '..' || part === '.' || part === 'this') {
-        if (dig.length > 0) {
-          throw new Exception('Invalid path: ' + original, this);
-        } else if (part === '..') {
-          depth++;
-          depthString += '../';
-        }
-      } else {
-        dig.push(part);
-      }
-    }
-
     this.data = data;
-    this.original = (data ? '@' : '') + original;
-    this.parts    = dig;
+    this.original = original;
+    this.parts    = parts;
     this.depth    = depth;
   },
 
diff --git a/lib/handlebars/compiler/helpers.js b/lib/handlebars/compiler/helpers.js
index 3d5144f..f215049 100644
--- a/lib/handlebars/compiler/helpers.js
+++ b/lib/handlebars/compiler/helpers.js
@@ -24,6 +24,43 @@ export function stripComment(comment) {
                 .replace(/-?-?~?\}\}$/, '');
 }
 
+export function preparePath(data, parts, locInfo) {
+  /*jshint -W040 */
+  locInfo = this.locInfo(locInfo);
+
+  var original = data ? '@' : '',
+      dig = [],
+      depth = 0,
+      depthString = '';
+
+  for(var i=0,l=parts.length; i<l; i++) {
+    var part = parts[i].part;
+    original += (parts[i].separator || '') + part;
+
+    if (part === '..' || part === '.' || part === 'this') {
+      if (dig.length > 0) {
+        throw new Exception('Invalid path: ' + original, {loc: locInfo});
+      } else if (part === '..') {
+        depth++;
+        depthString += '../';
+      }
+    } else {
+      dig.push(part);
+    }
+  }
+
+  return new this.PathExpression(data, depth, dig, original, locInfo);
+}
+
+export function prepareMustache(sexpr, open, strip, locInfo) {
+  /*jshint -W040 */
+  // Must use charAt to support IE pre-10
+  var escapeFlag = open.charAt(3) || open.charAt(2),
+      escaped = escapeFlag !== '{' && escapeFlag !== '&';
+
+  return new this.MustacheStatement(sexpr, escaped, strip, this.locInfo(locInfo));
+}
+
 export function prepareRawBlock(openRawBlock, content, close, locInfo) {
   /*jshint -W040 */
   if (openRawBlock.sexpr.path.original !== close) {
diff --git a/spec/ast.js b/spec/ast.js
index 5326749..3e36730 100644
--- a/spec/ast.js
+++ b/spec/ast.js
@@ -23,43 +23,14 @@ describe('ast', function() {
   }
 
   describe('MustacheStatement', function() {
-    function testEscape(open, expected) {
-      var mustache = new handlebarsEnv.AST.MustacheStatement([{}], open, false);
-      equals(mustache.escaped, expected);
-    }
-
     it('should store args', function() {
       var id = {isSimple: true},
           hash = {},
-          mustache = new handlebarsEnv.AST.MustacheStatement([id, 'param1'], '', false, LOCATION_INFO);
+          mustache = new handlebarsEnv.AST.MustacheStatement({}, true, {}, LOCATION_INFO);
       equals(mustache.type, 'MustacheStatement');
       equals(mustache.escaped, true);
       testLocationInfoStorage(mustache);
     });
-    it('should accept token for escape', function() {
-      testEscape('{{', true);
-      testEscape('{{~', true);
-      testEscape('{{#', true);
-      testEscape('{{~#', true);
-      testEscape('{{/', true);
-      testEscape('{{~/', true);
-      testEscape('{{^', true);
-      testEscape('{{~^', true);
-      testEscape('{', true);
-      testEscape('{', true);
-
-      testEscape('{{&', false);
-      testEscape('{{~&', false);
-      testEscape('{{{', false);
-      testEscape('{{~{', false);
-    });
-    it('should accept boolean for escape', function() {
-      testEscape(true, true);
-      testEscape({}, true);
-
-      testEscape(false, false);
-      testEscape(undefined, false);
-    });
   });
   describe('BlockStatement', function() {
     it('should throw on mustache mismatch', function() {
@@ -70,7 +41,7 @@ describe('ast', function() {
 
     it('stores location info', function(){
       var sexprNode = new handlebarsEnv.AST.SubExpression([{ original: 'foo'}], null);
-      var mustacheNode = new handlebarsEnv.AST.MustacheStatement(sexprNode, null, '{{', {});
+      var mustacheNode = new handlebarsEnv.AST.MustacheStatement(sexprNode, false, {});
       var block = new handlebarsEnv.AST.BlockStatement(mustacheNode,
                                                   {body: [], strip: {}}, {body: [], strip: {}},
                                                   {
@@ -82,32 +53,8 @@ describe('ast', function() {
     });
   });
   describe('PathExpression', function() {
-    it('should throw on invalid path', function() {
-      shouldThrow(function() {
-        new handlebarsEnv.AST.PathExpression(false, [
-          {part: 'foo'},
-          {part: '..'},
-          {part: 'bar'}
-        ], {start: {line: 1, column: 1}});
-      }, Handlebars.Exception, "Invalid path: foo.. - 1:1");
-      shouldThrow(function() {
-        new handlebarsEnv.AST.PathExpression(false, [
-          {part: 'foo'},
-          {part: '.'},
-          {part: 'bar'}
-        ], {start: {line: 1, column: 1}});
-      }, Handlebars.Exception, "Invalid path: foo. - 1:1");
-      shouldThrow(function() {
-        new handlebarsEnv.AST.PathExpression(false, [
-          {part: 'foo'},
-          {part: 'this'},
-          {part: 'bar'}
-        ], {start: {line: 1, column: 1}});
-      }, Handlebars.Exception, "Invalid path: foothis - 1:1");
-    });
-
     it('stores location info', function(){
-      var idNode = new handlebarsEnv.AST.PathExpression(false, [], LOCATION_INFO);
+      var idNode = new handlebarsEnv.AST.PathExpression(false, 0, [], 'foo', LOCATION_INFO);
       testLocationInfoStorage(idNode);
     });
   });
diff --git a/spec/parser.js b/spec/parser.js
index ad52734..26eb4dd 100644
--- a/spec/parser.js
+++ b/spec/parser.js
@@ -166,7 +166,6 @@ describe('parser', function() {
   it('parses inverse block with block params', function() {
     equals(ast_for("{{^foo as |bar baz|}}content{{/foo}}"), "BLOCK:\n  PATH:foo []\n  {{^}}\n    BLOCK PARAMS: [ bar baz ]\n    CONTENT[ 'content' ]\n");
   });
-
   it("raises if there's a Parse error", function() {
     shouldThrow(function() {
       ast_for("foo{{^}}bar");
@@ -186,6 +185,18 @@ describe('parser', function() {
     }, Error, /goodbyes doesn't match hellos/);
   });
 
+  it('should handle invalid paths', function() {
+    shouldThrow(function() {
+      ast_for("{{foo/../bar}}");
+    }, Error, /Invalid path: foo\/\.\. - 1:2/);
+    shouldThrow(function() {
+      ast_for("{{foo/./bar}}");
+    }, Error, /Invalid path: foo\/\. - 1:2/);
+    shouldThrow(function() {
+      ast_for("{{foo/this/bar}}");
+    }, Error, /Invalid path: foo\/this - 1:2/);
+  });
+
   it('knows how to report the correct line number in errors', function() {
     shouldThrow(function() {
       ast_for("hello\nmy\n{{foo}");
diff --git a/src/handlebars.yy b/src/handlebars.yy
index 8b63df0..7f2765c 100644
--- a/src/handlebars.yy
+++ b/src/handlebars.yy
@@ -47,7 +47,7 @@ openInverse
   ;
 
 openInverseChain
-  : OPEN_INVERSE_CHAIN sexpr CLOSE -> new yy.MustacheStatement($2, $1, yy.stripFlags($1, $3), yy.locInfo(@$))
+  : OPEN_INVERSE_CHAIN sexpr CLOSE -> yy.prepareMustache($2, $1, yy.stripFlags($1, $3), @$)
   ;
 
 inverseAndProgram
@@ -73,8 +73,8 @@ closeBlock
 mustache
   // Parsing out the '&' escape token at AST level saves ~500 bytes after min due to the removal of one parser node.
   // This also allows for handler unification as all mustache node instances can utilize the same handler
-  : OPEN sexpr CLOSE -> new yy.MustacheStatement($2, $1, yy.stripFlags($1, $3), yy.locInfo(@$))
-  | OPEN_UNESCAPED sexpr CLOSE_UNESCAPED -> new yy.MustacheStatement($2, $1, yy.stripFlags($1, $3), yy.locInfo(@$))
+  : OPEN sexpr CLOSE -> yy.prepareMustache($2, $1, yy.stripFlags($1, $3), @$)
+  | OPEN_UNESCAPED sexpr CLOSE_UNESCAPED -> yy.prepareMustache($2, $1, yy.stripFlags($1, $3), @$)
   ;
 
 partial
@@ -82,8 +82,8 @@ partial
   ;
 
 sexpr
-  : helperName param* hash? -> new yy.SubExpression([$1].concat($2), $3, yy.locInfo(@$))
-  | dataName -> new yy.SubExpression([$1], null, yy.locInfo(@$))
+  : helperName param* hash? -> new yy.SubExpression($1, $2, $3, yy.locInfo(@$))
+  | dataName -> new yy.SubExpression($1, null, null, yy.locInfo(@$))
   ;
 
 param
@@ -114,11 +114,11 @@ helperName
   ;
 
 dataName
-  : DATA pathSegments -> new yy.PathExpression(true, $2, yy.locInfo(@$))
+  : DATA pathSegments -> yy.preparePath(true, $2, @$)
   ;
 
 path
-  : pathSegments -> new yy.PathExpression(false, $1, yy.locInfo(@$))
+  : pathSegments -> yy.preparePath(false, $1, @$)
   ;
 
 pathSegments