Repository: goldibex/targaryen
Pull request #55 (2016-10-21 11:20:46)

Fix catching unknown variable in rulesets
Work every node of a rule ast to check the identifier exists.


Patch:
diff --git a/lib/parser/rule.js b/lib/parser/rule.js
index a3735b9..d21b61a 100644
--- a/lib/parser/rule.js
+++ b/lib/parser/rule.js
@@ -51,6 +51,7 @@ function Rule(ruleStr, wildchildren, isWrite) {
   })(ast);
 
   scope.inferType(this._ast);
+  scope.assertIndentifiersExist(this._ast);
 
   if (this._ast.expression.inferredType !== 'boolean') {
     throw new Error('Expression does not evaluate to a boolean');
diff --git a/lib/parser/scope.js b/lib/parser/scope.js
index 40f38da..f8d02ce 100644
--- a/lib/parser/scope.js
+++ b/lib/parser/scope.js
@@ -136,6 +136,50 @@ Scope.prototype.inferType = function(node) {
 
 };
 
+Scope.prototype.assertIndentifiersExist = function(node) {
+  var stack = [node];
+
+  while (stack.length > 0) {
+    let currNode = stack.pop();
+
+    switch(currNode.type) {
+
+    case 'LogicalExpression':
+    case 'BinaryExpression':
+      stack.push(currNode.right, currNode.left);
+      break;
+
+    case 'ConditionalExpression':
+      stack.push(currNode.test, currNode.consequent, currNode.alternate);
+      break;
+
+    case 'ExpressionStatement':
+      stack.push(currNode.expression);
+      break;
+
+    case 'CallExpression':
+      stack.push(currNode.callee);
+      break;
+
+    case 'MemberExpression':
+      stack.push(currNode.object);
+      break;
+
+    case 'UnaryExpression':
+      stack.push(currNode.argument);
+      break;
+
+    case 'Identifier':
+      this._assertIdentifier(currNode);
+      break;
+
+    default:
+      break;
+
+    }
+  }
+};
+
 Scope.prototype._inferLiteral = function(node) {
 
   // one hiccup: is this a regular expression literal? esparse does not help with this
@@ -218,10 +262,16 @@ Scope.prototype._inferBinaryExpression = function(node) {
 
 Scope.prototype._inferIdentifier = function(node) {
 
+  this._assertIdentifier(node);
+  node.inferredType = this.types[node.name];
+
+};
+
+Scope.prototype._assertIdentifier = function(node) {
+
   if (!this.types[node.name]) {
     throw nodeError(node, 'Unknown variable "' + node.name + '"');
   }
-  node.inferredType = this.types[node.name];
 
 };
 


Test patch:
diff --git a/test/spec/lib/ruleset.js b/test/spec/lib/ruleset.js
index e4eec85..e2093fd 100644
--- a/test/spec/lib/ruleset.js
+++ b/test/spec/lib/ruleset.js
@@ -95,11 +95,11 @@ function getRoot() {
 
 }
 
-var invalidRulesets = [
-  null,
-  'wut',
-  { noTopLevelRules: true },
-  {
+var invalidRulesets = {
+  'are null': null,
+  'are a string': 'wut',
+  'are missing': { noTopLevelRules: true },
+  'include extra props': {
     rules: {
       '.read': true,
       '.write': true,
@@ -108,29 +108,41 @@ var invalidRulesets = [
     },
     otherStuff: true
   },
-  {
+  'include invalid index': {
     rules: {
       '.read': true,
       '.indexOn': true
     }
   },
-  {
+  'include unknown props': {
     rules: {
       '.read': true,
       '.woops': true
     }
   },
-  {
+  'include unknown wildchild': {
     rules: {
       '.read': '$somewhere === true'
     }
   },
-  {
+  'set index to an object': {
     rules: {
       '.indexOn': {}
     }
+  },
+  'include unknown variables': {
+    rules: {
+      ".validate": "something.val() + 1 === date.val()",
+      ".write": "something.val() / 2 > 0"
+    }
+  },
+  'include rules composed with unknown variables': {
+    rules: {
+      ".validate": "auth != null && something.val() + 1 === date.val()",
+      ".write": "auth != null && something.val() / 2 > 0"
+    }
   }
-];
+};
 
 var validRulesets = [{
   rules: {}
@@ -159,14 +171,12 @@ describe('Ruleset', function() {
 
   describe('constructor', function() {
 
-    it('rejects invalid rulesets', function() {
-
-      invalidRulesets.forEach(function(rules) {
+    Object.keys(invalidRulesets).forEach(function(reason) {
+      it(`rejects when rulesets ${reason}`, function() {
         expect(function() {
-          return new Ruleset(rules);
+          return new Ruleset(invalidRulesets[reason]);
         }).to.throw();
       });
-
     });
 
     it('accepts valid rulesets', function() {