Rework strip flags to make clearer at in AST level

Rather than keeping state in the AST, which requires some gymnastics, we create a separate visitor flow which does the top down iteration necessary to calculate all of the state needed for proper whitespace control evaluation.

7 files changed, 268 insertions, 213 deletions

diff --git a/docs/compiler-api.md b/docs/compiler-api.md
index 7e89b01..89dcc4c 100644
--- a/docs/compiler-api.md
+++ b/docs/compiler-api.md
@@ -45,28 +45,31 @@ interface Program <: Node {
     body: [ Statement ];
     
     blockParams: [ string ];
-    strip: StripFlags | null;
 }
 ```
 
 ### Statements
 
 ```java
-interface Statement <: Node {
-    strip: StripFlags | null;
-}
+interface Statement <: Node { }
 
 interface MustacheStatement <: Statement {
     type: "MustacheStatement";
     sexpr: Subexpression;
     escaped: boolean;
+
+    strip: StripFlags | null;
 }
 
 interface BlockStatement <: Statement {
     type: "BlockStatement";
     sexpr: Subexpression;
-    program: Program;
+    program: Program | null;
     inverse: Program | null;
+
+    openStrip: StripFlags | null;
+    inverseStrip: StripFlags | null;
+    closeStrip: StripFlags | null;
 }
 
 interface PartialStatement <: Statement {
@@ -74,6 +77,7 @@ interface PartialStatement <: Statement {
     sexpr: Subexpression;
     
     indent: string;
+    strip: StripFlags | null;
 }
 
 interface ContentStatement <: Statement {
@@ -85,6 +89,8 @@ interface ContentStatement <: Statement {
 interface CommentStatement <: Statement {
     type: "CommentStatement";
     value: string;
+
+    strip: StripFlags | null;
 }
 ```
 
@@ -167,15 +173,13 @@ interface HashPair <: Node {
 }
 
 interface StripFlags {
-    left: boolean;
-    right: boolean;
+    open: boolean;
+    close: boolean;
 }
 ```
 
 `StripFlags` are used to signify whitespace control character that may have been entered on a given statement.
 
-TODO : Document what the flags mean or drop this from things like Program.
-
 ## AST Visitor
 
 `Handlebars.Visitor` is available as a base class for general interaction with AST structures. This will by default traverse the entire tree and individual methods may be overridden to provide specific responses to particular nodes.
diff --git a/lib/handlebars/compiler/ast.js b/lib/handlebars/compiler/ast.js
index 9f25443..72a56aa 100644
--- a/lib/handlebars/compiler/ast.js
+++ b/lib/handlebars/compiler/ast.js
@@ -20,14 +20,17 @@ var AST = {
     this.strip = strip;
   },
 
-  BlockStatement: function(sexpr, program, inverse, strip, locInfo) {
+  BlockStatement: function(sexpr, program, inverse, openStrip, inverseStrip, closeStrip, locInfo) {
     this.loc = locInfo;
 
     this.type = 'BlockStatement';
     this.sexpr = sexpr;
     this.program  = program;
     this.inverse  = inverse;
-    this.strip = strip;
+
+    this.openStrip = openStrip;
+    this.inverseStrip = inverseStrip;
+    this.closeStrip = closeStrip;
   },
 
   PartialStatement: function(sexpr, strip, locInfo) {
@@ -37,7 +40,6 @@ var AST = {
     this.indent = '';
 
     this.strip = strip;
-    this.strip.inlineStandalone = true;
   },
 
   ContentStatement: function(string, locInfo) {
@@ -52,7 +54,6 @@ var AST = {
     this.value = comment;
 
     this.strip = strip;
-    strip.inlineStandalone = true;
   },
 
   SubExpression: function(path, params, hash, locInfo) {
diff --git a/lib/handlebars/compiler/base.js b/lib/handlebars/compiler/base.js
index 786c37e..ff237ec 100644
--- a/lib/handlebars/compiler/base.js
+++ b/lib/handlebars/compiler/base.js
@@ -1,5 +1,6 @@
 import parser from "./parser";
 import AST from "./ast";
+import WhitespaceControl from "./whitespace-control";
 module Helpers from "./helpers";
 import { extend } from "../utils";
 
@@ -19,5 +20,6 @@ export function parse(input, options) {
     return new yy.SourceLocation(options && options.srcName, locInfo);
   };
 
-  return parser.parse(input);
+  var strip = new WhitespaceControl();
+  return strip.accept(parser.parse(input));
 }
diff --git a/lib/handlebars/compiler/helpers.js b/lib/handlebars/compiler/helpers.js
index f215049..1daddf6 100644
--- a/lib/handlebars/compiler/helpers.js
+++ b/lib/handlebars/compiler/helpers.js
@@ -14,8 +14,8 @@ export function SourceLocation(source, locInfo) {
 
 export function stripFlags(open, close) {
   return {
-    left: open.charAt(2) === '~',
-    right: close.charAt(close.length-3) === '~'
+    open: open.charAt(2) === '~',
+    close: close.charAt(close.length-3) === '~'
   };
 }
 
@@ -69,9 +69,13 @@ export function prepareRawBlock(openRawBlock, content, close, locInfo) {
     throw new Exception(openRawBlock.sexpr.path.original + " doesn't match " + close, errorNode);
   }
 
+  locInfo = this.locInfo(locInfo);
   var program = new this.Program([content], null, {}, locInfo);
 
-  return new this.BlockStatement(openRawBlock.sexpr, program, undefined, undefined, locInfo);
+  return new this.BlockStatement(
+      openRawBlock.sexpr, program, undefined,
+      {}, {}, {},
+      locInfo);
 }
 
 export function prepareBlock(openBlock, program, inverseAndProgram, close, inverted, locInfo) {
@@ -85,192 +89,26 @@ export function prepareBlock(openBlock, program, inverseAndProgram, close, inver
 
   program.blockParams = openBlock.blockParams;
 
-  // Safely handle a chained inverse that does not have a non-conditional inverse
-  // (i.e. both inverseAndProgram AND close are undefined)
-  if (!close) {
-    close = {strip: {}};
-  }
-
-  // Find the inverse program that is involed with whitespace stripping.
-  var inverse = inverseAndProgram && inverseAndProgram.program,
-      firstInverse = inverse,
-      lastInverse = inverse;
-  if (inverse && inverse.inverse) {
-    firstInverse = inverse.body[0].program;
+  var inverse,
+      inverseStrip;
 
-    // Walk the inverse chain to find the last inverse that is actually in the chain.
-    while (lastInverse.inverse) {
-      lastInverse = lastInverse.body[lastInverse.body.length-1].program;
+  if (inverseAndProgram) {
+    if (inverseAndProgram.chain) {
+      inverseAndProgram.program.body[0].closeStrip = close.strip || close.openStrip;
     }
-  }
 
-  var strip = {
-    left: openBlock.strip.left,
-    right: close.strip.right,
-
-    // Determine the standalone candiacy. Basically flag our content as being possibly standalone
-    // so our parent can determine if we actually are standalone
-    openStandalone: isNextWhitespace(program.body),
-    closeStandalone: isPrevWhitespace((firstInverse || program).body)
-  };
-
-  if (openBlock.strip.right) {
-    omitRight(program.body, null, true);
-  }
-
-  if (inverse) {
-    var inverseStrip = inverseAndProgram.strip;
-
-    if (inverseStrip.left) {
-      omitLeft(program.body, null, true);
-    }
-
-    if (inverseStrip.right) {
-      omitRight(firstInverse.body, null, true);
-    }
-    if (close.strip.left) {
-      omitLeft(lastInverse.body, null, true);
-    }
-
-    // Find standalone else statments
-    if (isPrevWhitespace(program.body)
-        && isNextWhitespace(firstInverse.body)) {
-
-      omitLeft(program.body);
-      omitRight(firstInverse.body);
-    }
-  } else {
-    if (close.strip.left) {
-      omitLeft(program.body, null, true);
-    }
+    inverseStrip = inverseAndProgram.strip;
+    inverse = inverseAndProgram.program;
   }
 
   if (inverted) {
-    return new this.BlockStatement(openBlock.sexpr, inverse, program, strip, locInfo);
-  } else {
-    return new this.BlockStatement(openBlock.sexpr, program, inverse, strip, locInfo);
-  }
-}
-
-
-export function prepareProgram(body, isRoot) {
-  for (var i = 0, l = body.length; i < l; i++) {
-    var current = body[i],
-        strip = current.strip;
-
-    if (!strip) {
-      continue;
-    }
-
-    var _isPrevWhitespace = isPrevWhitespace(body, i, isRoot, current.type === 'partial'),
-        _isNextWhitespace = isNextWhitespace(body, i, isRoot),
-
-        openStandalone = strip.openStandalone && _isPrevWhitespace,
-        closeStandalone = strip.closeStandalone && _isNextWhitespace,
-        inlineStandalone = strip.inlineStandalone && _isPrevWhitespace && _isNextWhitespace;
-
-    if (strip.right) {
-      omitRight(body, i, true);
-    }
-    if (strip.left) {
-      omitLeft(body, i, true);
-    }
-
-    if (inlineStandalone) {
-      omitRight(body, i);
-
-      if (omitLeft(body, i)) {
-        // If we are on a standalone node, save the indent info for partials
-        if (current.type === 'PartialStatement') {
-          // Pull out the whitespace from the final line
-          current.indent = (/([ \t]+$)/).exec(body[i-1].original)[1];
-        }
-      }
-    }
-    if (openStandalone) {
-      omitRight((current.program || current.inverse).body);
-
-      // Strip out the previous content node if it's whitespace only
-      omitLeft(body, i);
-    }
-    if (closeStandalone) {
-      // Always strip the next node
-      omitRight(body, i);
-
-      omitLeft((current.inverse || current.program).body);
-    }
-  }
-
-  return body;
-}
-
-function isPrevWhitespace(body, i, isRoot) {
-  if (i === undefined) {
-    i = body.length;
-  }
-
-  // Nodes that end with newlines are considered whitespace (but are special
-  // cased for strip operations)
-  var prev = body[i-1],
-      sibling = body[i-2];
-  if (!prev) {
-    return isRoot;
-  }
-
-  if (prev.type === 'ContentStatement') {
-    return (sibling || !isRoot ? (/\r?\n\s*?$/) : (/(^|\r?\n)\s*?$/)).test(prev.original);
-  }
-}
-function isNextWhitespace(body, i, isRoot) {
-  if (i === undefined) {
-    i = -1;
-  }
-
-  var next = body[i+1],
-      sibling = body[i+2];
-  if (!next) {
-    return isRoot;
-  }
-
-  if (next.type === 'ContentStatement') {
-    return (sibling || !isRoot ? (/^\s*?\r?\n/) : (/^\s*?(\r?\n|$)/)).test(next.original);
-  }
-}
-
-// Marks the node to the right of the position as omitted.
-// I.e. {{foo}}' ' will mark the ' ' node as omitted.
-//
-// If i is undefined, then the first child will be marked as such.
-//
-// If mulitple is truthy then all whitespace will be stripped out until non-whitespace
-// content is met.
-function omitRight(body, i, multiple) {
-  var current = body[i == null ? 0 : i + 1];
-  if (!current || current.type !== 'ContentStatement' || (!multiple && current.rightStripped)) {
-    return;
-  }
-
-  var original = current.value;
-  current.value = current.value.replace(multiple ? (/^\s+/) : (/^[ \t]*\r?\n?/), '');
-  current.rightStripped = current.value !== original;
-}
-
-// Marks the node to the left of the position as omitted.
-// I.e. ' '{{foo}} will mark the ' ' node as omitted.
-//
-// If i is undefined then the last child will be marked as such.
-//
-// If mulitple is truthy then all whitespace will be stripped out until non-whitespace
-// content is met.
-function omitLeft(body, i, multiple) {
-  var current = body[i == null ? body.length - 1 : i - 1];
-  if (!current || current.type !== 'ContentStatement' || (!multiple && current.leftStripped)) {
-    return;
+    inverted = inverse;
+    inverse = program;
+    program = inverted;
   }
 
-  // We omit the last node if it's whitespace only and not preceeded by a non-content node.
-  var original = current.value;
-  current.value = current.value.replace(multiple ? (/\s+$/) : (/[ \t]+$/), '');
-  current.leftStripped = current.value !== original;
-  return current.leftStripped;
+  return new this.BlockStatement(
+      openBlock.sexpr, program, inverse,
+      openBlock.strip, inverseStrip, close && (close.strip || close.openStrip),
+      this.locInfo(locInfo));
 }
diff --git a/lib/handlebars/compiler/whitespace-control.js b/lib/handlebars/compiler/whitespace-control.js
new file mode 100644
index 0000000..e10eb15
--- /dev/null
+++ b/lib/handlebars/compiler/whitespace-control.js
@@ -0,0 +1,210 @@
+import Visitor from "./visitor";
+
+function WhitespaceControl() {
+}
+WhitespaceControl.prototype = new Visitor();
+
+WhitespaceControl.prototype.Program = function(program) {
+  var isRoot = !this.isRootSeen;
+  this.isRootSeen = true;
+
+  var body = program.body;
+  for (var i = 0, l = body.length; i < l; i++) {
+    var current = body[i],
+        strip = this.accept(current);
+
+    if (!strip) {
+      continue;
+    }
+
+    var _isPrevWhitespace = isPrevWhitespace(body, i, isRoot),
+        _isNextWhitespace = isNextWhitespace(body, i, isRoot),
+
+        openStandalone = strip.openStandalone && _isPrevWhitespace,
+        closeStandalone = strip.closeStandalone && _isNextWhitespace,
+        inlineStandalone = strip.inlineStandalone && _isPrevWhitespace && _isNextWhitespace;
+
+    if (strip.close) {
+      omitRight(body, i, true);
+    }
+    if (strip.open) {
+      omitLeft(body, i, true);
+    }
+
+    if (inlineStandalone) {
+      omitRight(body, i);
+
+      if (omitLeft(body, i)) {
+        // If we are on a standalone node, save the indent info for partials
+        if (current.type === 'PartialStatement') {
+          // Pull out the whitespace from the final line
+          current.indent = (/([ \t]+$)/).exec(body[i-1].original)[1];
+        }
+      }
+    }
+    if (openStandalone) {
+      omitRight((current.program || current.inverse).body);
+
+      // Strip out the previous content node if it's whitespace only
+      omitLeft(body, i);
+    }
+    if (closeStandalone) {
+      // Always strip the next node
+      omitRight(body, i);
+
+      omitLeft((current.inverse || current.program).body);
+    }
+  }
+
+  return program;
+};
+WhitespaceControl.prototype.BlockStatement = function(block) {
+  this.accept(block.program);
+  this.accept(block.inverse);
+
+  // Find the inverse program that is involed with whitespace stripping.
+  var program = block.program || block.inverse,
+      inverse = block.program && block.inverse,
+      firstInverse = inverse,
+      lastInverse = inverse;
+
+  if (inverse && inverse.chained) {
+    firstInverse = inverse.body[0].program;
+
+    // Walk the inverse chain to find the last inverse that is actually in the chain.
+    while (lastInverse.chained) {
+      lastInverse = lastInverse.body[lastInverse.body.length-1].program;
+    }
+  }
+
+  var strip = {
+    open: block.openStrip.open,
+    close: block.closeStrip.close,
+
+    // Determine the standalone candiacy. Basically flag our content as being possibly standalone
+    // so our parent can determine if we actually are standalone
+    openStandalone: isNextWhitespace(program.body),
+    closeStandalone: isPrevWhitespace((firstInverse || program).body)
+  };
+
+  if (block.openStrip.close) {
+    omitRight(program.body, null, true);
+  }
+
+  if (inverse) {
+    var inverseStrip = block.inverseStrip;
+
+    if (inverseStrip.open) {
+      omitLeft(program.body, null, true);
+    }
+
+    if (inverseStrip.close) {
+      omitRight(firstInverse.body, null, true);
+    }
+    if (block.closeStrip.open) {
+      omitLeft(lastInverse.body, null, true);
+    }
+
+    // Find standalone else statments
+    if (isPrevWhitespace(program.body)
+        && isNextWhitespace(firstInverse.body)) {
+
+      omitLeft(program.body);
+      omitRight(firstInverse.body);
+    }
+  } else {
+    if (block.closeStrip.open) {
+      omitLeft(program.body, null, true);
+    }
+  }
+
+  return strip;
+};
+
+WhitespaceControl.prototype.MustacheStatement = function(mustache) {
+  return mustache.strip;
+};
+
+WhitespaceControl.prototype.PartialStatement = 
+    WhitespaceControl.prototype.CommentStatement = function(node) {
+  var strip = node.strip || {};
+  return {
+    inlineStandalone: true,
+    open: strip.open,
+    close: strip.close
+  };
+};
+
+
+function isPrevWhitespace(body, i, isRoot) {
+  if (i === undefined) {
+    i = body.length;
+  }
+
+  // Nodes that end with newlines are considered whitespace (but are special
+  // cased for strip operations)
+  var prev = body[i-1],
+      sibling = body[i-2];
+  if (!prev) {
+    return isRoot;
+  }
+
+  if (prev.type === 'ContentStatement') {
+    return (sibling || !isRoot ? (/\r?\n\s*?$/) : (/(^|\r?\n)\s*?$/)).test(prev.original);
+  }
+}
+function isNextWhitespace(body, i, isRoot) {
+  if (i === undefined) {
+    i = -1;
+  }
+
+  var next = body[i+1],
+      sibling = body[i+2];
+  if (!next) {
+    return isRoot;
+  }
+
+  if (next.type === 'ContentStatement') {
+    return (sibling || !isRoot ? (/^\s*?\r?\n/) : (/^\s*?(\r?\n|$)/)).test(next.original);
+  }
+}
+
+// Marks the node to the right of the position as omitted.
+// I.e. {{foo}}' ' will mark the ' ' node as omitted.
+//
+// If i is undefined, then the first child will be marked as such.
+//
+// If mulitple is truthy then all whitespace will be stripped out until non-whitespace
+// content is met.
+function omitRight(body, i, multiple) {
+  var current = body[i == null ? 0 : i + 1];
+  if (!current || current.type !== 'ContentStatement' || (!multiple && current.rightStripped)) {
+    return;
+  }
+
+  var original = current.value;
+  current.value = current.value.replace(multiple ? (/^\s+/) : (/^[ \t]*\r?\n?/), '');
+  current.rightStripped = current.value !== original;
+}
+
+// Marks the node to the left of the position as omitted.
+// I.e. ' '{{foo}} will mark the ' ' node as omitted.
+//
+// If i is undefined then the last child will be marked as such.
+//
+// If mulitple is truthy then all whitespace will be stripped out until non-whitespace
+// content is met.
+function omitLeft(body, i, multiple) {
+  var current = body[i == null ? body.length - 1 : i - 1];
+  if (!current || current.type !== 'ContentStatement' || (!multiple && current.leftStripped)) {
+    return;
+  }
+
+  // We omit the last node if it's whitespace only and not preceeded by a non-content node.
+  var original = current.value;
+  current.value = current.value.replace(multiple ? (/\s+$/) : (/[ \t]+$/), '');
+  current.leftStripped = current.value !== original;
+  return current.leftStripped;
+}
+
+export default WhitespaceControl;
diff --git a/spec/ast.js b/spec/ast.js
index 3e36730..d464cf1 100644
--- a/spec/ast.js
+++ b/spec/ast.js
@@ -42,13 +42,14 @@ describe('ast', function() {
     it('stores location info', function(){
       var sexprNode = new handlebarsEnv.AST.SubExpression([{ original: 'foo'}], null);
       var mustacheNode = new handlebarsEnv.AST.MustacheStatement(sexprNode, false, {});
-      var block = new handlebarsEnv.AST.BlockStatement(mustacheNode,
-                                                  {body: [], strip: {}}, {body: [], strip: {}},
-                                                  {
-                                                    strip: {},
-                                                    path: {original: 'foo'}
-                                                  },
-                                                  LOCATION_INFO);
+      var block = new handlebarsEnv.AST.BlockStatement(
+            mustacheNode,
+            {body: []},
+            {body: []},
+            {},
+            {},
+            {},
+            LOCATION_INFO);
       testLocationInfoStorage(block);
     });
   });
diff --git a/src/handlebars.yy b/src/handlebars.yy
index 7f2765c..0b2062e 100644
--- a/src/handlebars.yy
+++ b/src/handlebars.yy
@@ -5,11 +5,11 @@
 %%
 
 root
-  : program EOF { yy.prepareProgram($1.body, true); return $1; }
+  : program EOF { return $1; }
   ;
 
 program
-  : statement* -> new yy.Program(yy.prepareProgram($1), null, {}, yy.locInfo(@$))
+  : statement* -> new yy.Program($1, null, {}, yy.locInfo(@$))
   ;
 
 statement
@@ -26,7 +26,7 @@ content
   ;
 
 rawBlock
-  : openRawBlock content END_RAW_BLOCK -> yy.prepareRawBlock($1, $2, $3, yy.locInfo(@$))
+  : openRawBlock content END_RAW_BLOCK -> yy.prepareRawBlock($1, $2, $3, @$)
   ;
 
 openRawBlock
@@ -34,8 +34,8 @@ openRawBlock
   ;
 
 block
-  : openBlock program inverseChain? closeBlock -> yy.prepareBlock($1, $2, $3, $4, false, yy.locInfo(@$))
-  | openInverse program inverseAndProgram? closeBlock -> yy.prepareBlock($1, $2, $3, $4, true, yy.locInfo(@$))
+  : openBlock program inverseChain? closeBlock -> yy.prepareBlock($1, $2, $3, $4, false, @$)
+  | openInverse program inverseAndProgram? closeBlock -> yy.prepareBlock($1, $2, $3, $4, true, @$)
   ;
 
 openBlock
@@ -56,10 +56,9 @@ inverseAndProgram
 
 inverseChain
   : openInverseChain program inverseChain? {
-    var inverse = yy.prepareBlock($1, $2, $3, $3, false, yy.locInfo(@$)),
-        program = new yy.Program(yy.prepareProgram([inverse]), null, {}, yy.locInfo(@$));
-
-    program.inverse = inverse;
+    var inverse = yy.prepareBlock($1, $2, $3, $3, false, @$),
+        program = new yy.Program([inverse], null, {}, yy.locInfo(@$));
+    program.chained = true;
 
     $$ = { strip: $1.strip, program: program, chain: true };
   }