Merge pull request #930 from wycats/visitor-update

Add parent tracking and mutation to AST visitors

3 files changed, 192 insertions, 38 deletions

diff --git a/docs/compiler-api.md b/docs/compiler-api.md
index 98ca894..74af672 100644
--- a/docs/compiler-api.md
+++ b/docs/compiler-api.md
@@ -204,6 +204,12 @@ var scanner = new ImportScanner();
 scanner.accept(ast);
 ```
 
+The current node's ancestors will be maintained in the `parents` array, with the most recent parent listed first.
+
+The visitor may also be configured to operate in mutation mode by setting the `mutation` field to true. When in this mode, handler methods may return any valid AST node and it will replace the one they are currently operating on. Returning `false` will remove the given value (if valid) and returning `undefined` will leave the node in tact. This return structure only apply to mutation mode and non-mutation mode visitors are free to return whatever values they wish.
+
+Implementors that may need to support mutation mode are encouraged to utilize the `acceptKey`, `acceptRequired` and `acceptArray` helpers which provide the conditional overwrite behavior as well as implement sanity checks where pertinent.
+
 ## JavaScript Compiler
 
 The `Handlebars.JavaScriptCompiler` object has a number of methods that may be customized to alter the output of the compiler:
diff --git a/lib/handlebars/compiler/visitor.js b/lib/handlebars/compiler/visitor.js
index c2480ad..3fb37fb 100644
--- a/lib/handlebars/compiler/visitor.js
+++ b/lib/handlebars/compiler/visitor.js
@@ -1,66 +1,109 @@
-/*jshint unused: false */
-function Visitor() {}
+import Exception from "../exception";
+import AST from "./ast";
+
+function Visitor() {
+  this.parents = [];
+}
 
 Visitor.prototype = {
   constructor: Visitor,
+  mutating: false,
 
-  accept: function(object) {
-    return object && this[object.type](object);
+  // Visits a given value. If mutating, will replace the value if necessary.
+  acceptKey: function(node, name) {
+    var value = this.accept(node[name]);
+    if (this.mutating) {
+      // Hacky sanity check:
+      if (value && (!value.type || !AST[value.type])) {
+        throw new Exception('Unexpected node type "' + value.type + '" found when accepting ' + name + ' on ' + node.type);
+      }
+      node[name] = value;
+    }
   },
 
-  Program: function(program) {
-    var body = program.body,
-        i, l;
+  // Performs an accept operation with added sanity check to ensure
+  // required keys are not removed.
+  acceptRequired: function(node, name) {
+    this.acceptKey(node, name);
 
-    for(i=0, l=body.length; i<l; i++) {
-      this.accept(body[i]);
+    if (!node[name]) {
+      throw new Exception(node.type + ' requires ' + name);
     }
   },
 
+  // Traverses a given array. If mutating, empty respnses will be removed
+  // for child elements.
+  acceptArray: function(array) {
+    for (var i = 0, l = array.length; i < l; i++) {
+      this.acceptKey(array, i);
+
+      if (!array[i]) {
+        array.splice(i, 1);
+        i--;
+        l--;
+      }
+    }
+  },
+
+  accept: function(object) {
+    if (!object) {
+      return;
+    }
+
+    if (this.current) {
+      this.parents.unshift(this.current);
+    }
+    this.current = object;
+
+    var ret = this[object.type](object);
+
+    this.current = this.parents.shift();
+
+    if (!this.mutating || ret) {
+      return ret;
+    } else if (ret !== false) {
+      return object;
+    }
+  },
+
+  Program: function(program) {
+    this.acceptArray(program.body);
+  },
+
   MustacheStatement: function(mustache) {
-    this.accept(mustache.sexpr);
+    this.acceptRequired(mustache, 'sexpr');
   },
 
   BlockStatement: function(block) {
-    this.accept(block.sexpr);
-    this.accept(block.program);
-    this.accept(block.inverse);
+    this.acceptRequired(block, 'sexpr');
+    this.acceptKey(block, 'program');
+    this.acceptKey(block, 'inverse');
   },
 
   PartialStatement: function(partial) {
-    this.accept(partial.partialName);
-    this.accept(partial.context);
-    this.accept(partial.hash);
+    this.acceptRequired(partial, 'sexpr');
   },
 
-  ContentStatement: function(content) {},
-  CommentStatement: function(comment) {},
+  ContentStatement: function(/* content */) {},
+  CommentStatement: function(/* comment */) {},
 
   SubExpression: function(sexpr) {
-    var params = sexpr.params, paramStrings = [], hash;
-
-    this.accept(sexpr.path);
-    for(var i=0, l=params.length; i<l; i++) {
-      this.accept(params[i]);
-    }
-    this.accept(sexpr.hash);
+    this.acceptRequired(sexpr, 'path');
+    this.acceptArray(sexpr.params);
+    this.acceptKey(sexpr, 'hash');
   },
 
-  PathExpression: function(path) {},
+  PathExpression: function(/* path */) {},
 
-  StringLiteral: function(string) {},
-  NumberLiteral: function(number) {},
-  BooleanLiteral: function(bool) {},
+  StringLiteral: function(/* string */) {},
+  NumberLiteral: function(/* number */) {},
+  BooleanLiteral: function(/* bool */) {},
 
   Hash: function(hash) {
-    var pairs = hash.pairs;
-
-    for(var i=0, l=pairs.length; i<l; i++) {
-      this.accept(pairs[i]);
-    }
+    this.acceptArray(hash.pairs);
   },
   HashPair: function(pair) {
-    this.accept(pair.value);
+    this.acceptRequired(pair, 'value');
   }
 };
 
diff --git a/spec/visitor.js b/spec/visitor.js
index 17a948e..0c23c0d 100644
--- a/spec/visitor.js
+++ b/spec/visitor.js
@@ -1,7 +1,7 @@
-/*global Handlebars */
+/*global Handlebars, shouldThrow */
 
 describe('Visitor', function() {
-  if (!Handlebars.Visitor) {
+  if (!Handlebars.Visitor || !Handlebars.print) {
     return;
   }
 
@@ -23,9 +23,15 @@ describe('Visitor', function() {
     };
     visitor.BooleanLiteral = function(bool) {
       equal(bool.value, true);
+
+      equal(this.parents.length, 4);
+      equal(this.parents[0].type, 'SubExpression');
+      equal(this.parents[1].type, 'SubExpression');
+      equal(this.parents[2].type, 'BlockStatement');
+      equal(this.parents[3].type, 'Program');
     };
     visitor.PathExpression = function(id) {
-      equal(/foo\.bar$/.test(id.original), true);
+      equal(/(foo\.)?bar$/.test(id.original), true);
     };
     visitor.ContentStatement = function(content) {
       equal(content.value, ' ');
@@ -36,4 +42,103 @@ describe('Visitor', function() {
 
     visitor.accept(Handlebars.parse('{{#foo.bar (foo.bar 1 "2" true) foo=@foo.bar}}{{!comment}}{{> bar }} {{/foo.bar}}'));
   });
+
+  it('should return undefined');
+
+  describe('mutating', function() {
+    describe('fields', function() {
+      it('should replace value', function() {
+        var visitor = new Handlebars.Visitor();
+
+        visitor.mutating = true;
+        visitor.StringLiteral = function(string) {
+          return new Handlebars.AST.NumberLiteral(42, string.locInfo);
+        };
+
+        var ast = Handlebars.parse('{{foo foo="foo"}}');
+        visitor.accept(ast);
+        equals(Handlebars.print(ast), '{{ PATH:foo [] HASH{foo=NUMBER{42}} }}\n');
+      });
+      it('should treat undefined resonse as identity', function() {
+        var visitor = new Handlebars.Visitor();
+        visitor.mutating = true;
+
+        var ast = Handlebars.parse('{{foo foo=42}}');
+        visitor.accept(ast);
+        equals(Handlebars.print(ast), '{{ PATH:foo [] HASH{foo=NUMBER{42}} }}\n');
+      });
+      it('should remove false responses', function() {
+        var visitor = new Handlebars.Visitor();
+
+        visitor.mutating = true;
+        visitor.Hash = function() {
+          return false;
+        };
+
+        var ast = Handlebars.parse('{{foo foo=42}}');
+        visitor.accept(ast);
+        equals(Handlebars.print(ast), '{{ PATH:foo [] }}\n');
+      });
+      it('should throw when removing required values', function() {
+        shouldThrow(function() {
+          var visitor = new Handlebars.Visitor();
+
+          visitor.mutating = true;
+          visitor.SubExpression = function() {
+            return false;
+          };
+
+          var ast = Handlebars.parse('{{foo 42}}');
+          visitor.accept(ast);
+        }, Handlebars.Exception, 'MustacheStatement requires sexpr');
+      });
+      it('should throw when returning non-node responses', function() {
+        shouldThrow(function() {
+          var visitor = new Handlebars.Visitor();
+
+          visitor.mutating = true;
+          visitor.SubExpression = function() {
+            return {};
+          };
+
+          var ast = Handlebars.parse('{{foo 42}}');
+          visitor.accept(ast);
+        }, Handlebars.Exception, 'Unexpected node type "undefined" found when accepting sexpr on MustacheStatement');
+      });
+    });
+    describe('arrays', function() {
+      it('should replace value', function() {
+        var visitor = new Handlebars.Visitor();
+
+        visitor.mutating = true;
+        visitor.StringLiteral = function(string) {
+          return new Handlebars.AST.NumberLiteral(42, string.locInfo);
+        };
+
+        var ast = Handlebars.parse('{{foo "foo"}}');
+        visitor.accept(ast);
+        equals(Handlebars.print(ast), '{{ PATH:foo [NUMBER{42}] }}\n');
+      });
+      it('should treat undefined resonse as identity', function() {
+        var visitor = new Handlebars.Visitor();
+        visitor.mutating = true;
+
+        var ast = Handlebars.parse('{{foo 42}}');
+        visitor.accept(ast);
+        equals(Handlebars.print(ast), '{{ PATH:foo [NUMBER{42}] }}\n');
+      });
+      it('should remove false responses', function() {
+        var visitor = new Handlebars.Visitor();
+
+        visitor.mutating = true;
+        visitor.NumberLiteral = function() {
+          return false;
+        };
+
+        var ast = Handlebars.parse('{{foo 42}}');
+        visitor.accept(ast);
+        equals(Handlebars.print(ast), '{{ PATH:foo [] }}\n');
+      });
+    });
+  });
 });