Bullet-proof report left recursion

lib/compiler/passes/report-left-recursion.js

@@ -5,26 +5,80 @@ var arrays       = require("../../utils/arrays"),
 
 /* Checks that no left recursion is present. */
 function reportLeftRecursion(ast) {
+  // Each visitor function returns |true|, if some input wiil be consumed in any case,
+  // and |false| otherwize. In last case left recursion is possible.
+  function returnFalse() { return false; };
+  function visitExpressionAndReturnFalse(node, appliedRules) {
+    check(node.expression, appliedRules);
+    return false;
+  };
+
+  var detected = {};
+
   var check = visitor.build({
+    grammar: function(node) {
+      arrays.each(node.rules, function(rule) {
+        // If rule already detected, not needed check it again
+        if (!detected[rule.name]) {
+          check(rule, []);
+        }
+      });
+    },
     rule: function(node, appliedRules) {
-      check(node.expression, appliedRules.concat(node.name));
+      // For initial call |appliedRules| is empty, so that not trigger
+      var hasLeftRecursion = arrays.contains(appliedRules, node.name);
+      appliedRules.push(node.name);
+      try {
+        if (hasLeftRecursion) {
+          arrays.each(appliedRules, function(name) { detected[name] = true; });
+          // If you need multimply error detection in one pass, just not throw exception
+          // there, but notify your backend.
+          throw new GrammarError('Left recursion detected: ' + appliedRules.join('->'));
+          return false;
+        } else {
+          return check(node.expression, appliedRules);
+        }
+      } finally {
+        appliedRules.pop();
+      }
+    },
+    choice: function(node, appliedRules) {
+      for (var i = 0; i < node.alternatives.length; ++i) {
+        // If all alternatives can not consume any input, than left recursion is possible.
+        if (!check(node.alternatives[i], appliedRules)) {
+          return false;
+        }
+      }
+      // If all alternatives consume some input, than, if has any alternative,
+      // |choice| consume input.
+      return node.alternatives.length > 0;
     },
-
     sequence: function(node, appliedRules) {
-      check(node.elements[0], appliedRules);
+      for (var i = 0; i < node.elements.length; ++i) {
+        // Left recursion is inpossible, because we make at least one step forward
+        if (check(node.elements[i], appliedRules)) {
+          return true;
+        }
+      }
+      return false;
     },
-
+    simple_and:   visitExpressionAndReturnFalse,
+    simple_not:   visitExpressionAndReturnFalse,
+    optional:     visitExpressionAndReturnFalse,
+    zero_or_more: visitExpressionAndReturnFalse,
+    semantic_and: returnFalse,
+    semantic_not: returnFalse,
     rule_ref: function(node, appliedRules) {
-      if (arrays.contains(appliedRules, node.name)) {
-        throw new GrammarError(
-          "Left recursion detected for rule \"" + node.name + "\"."
-        );
-      }
-      check(asts.findRule(ast, node.name), appliedRules);
-    }
+      var rule = asts.findRule(ast, node.name);
+      // If rule not exist, not trigger left recursion, because we don't know it.
+      return rule ? check(rule, appliedRules) : true;
+    },
+    literal: function(n) { return n.value.length > 0; },
+    "class": function(n) { return n.parts.length > 0; },
+    any:     function()  { return true; }
   });
 
-  check(ast, []);
+  check(ast);
 }
 
 module.exports = reportLeftRecursion;

lib/compiler/visitor.js

@@ -13,7 +13,7 @@ var visitor = {
     function visitExpression(node) {
       var extraArgs = Array.prototype.slice.call(arguments, 1);
 
-      visit.apply(null, [node.expression].concat(extraArgs));
+      return visit.apply(null, [node.expression].concat(extraArgs));
     }
 
     function visitChildren(property) {

spec/unit/compiler/passes/helpers.js

@@ -80,6 +80,7 @@ beforeEach(function() {
         if (this.isNot) {
           this.message = function() {
             return "Expected the pass not to report an error"
+                 + (details ? " with details " + jasmine.pp(details) : "") + ", "
                  + "for grammar " + jasmine.pp(grammar) + ", "
                  + "but it did.";
           };

spec/unit/compiler/passes/report-left-recursion.spec.js

@@ -2,28 +2,83 @@ describe("compiler pass |reportLeftRecursion|", function() {
   var pass = PEG.compiler.passes.check.reportLeftRecursion;
 
   it("reports direct left recursion", function() {
-    expect(pass).toReportError('start = start', {
-      message: 'Left recursion detected for rule \"start\".'
-    });
+    var tests = [
+      'start = start;',
+      'start = start "a";',
+      'start = start {};',
+      'start = "a"? start;',
+      'start = "a"* start;',
+      'start = !"a" start;',
+      'start = &"a" start;',
+      'start = !{} start;',
+      'start = &{} start;',
+      'start = "" start;',
+      'start = [] start;'
+    ];
+    for (var i = 0; i < tests.length; ++i) {
+      expect(pass).toReportError(tests[i], {
+        message: 'Left recursion detected: start->start'
+      });
+    }
   });
 
   it("reports indirect left recursion", function() {
-    expect(pass).toReportError([
-      'start = stop',
-      'stop  = start'
-    ].join("\n"), {
-      message: 'Left recursion detected for rule \"start\".'
-    });
+    var tests = [
+      'start = stop; stop = start;',
+      'start = stop; stop = start "a";',
+      'start = stop; stop = start {};',
+      'start = stop; stop = "a"? start;',
+      'start = stop; stop = "a"* start;',
+      'start = stop; stop = !"a" start;',
+      'start = stop; stop = &"a" start;',
+      'start = stop; stop = !{} start;',
+      'start = stop; stop = &{} start;',
+      'start = stop; stop = "" start;',
+      'start = stop; stop = [] start;'
+    ];
+    for (var i = 0; i < tests.length; ++i) {
+      expect(pass).toReportError(tests[i], {
+        message: 'Left recursion detected: start->stop->start'
+      });
+    }
   });
 
-  describe("in sequences", function() {
-    it("reports left recursion only for the first element", function() {
-      expect(pass).toReportError('start = start "a" "b"', {
-        message: 'Left recursion detected for rule \"start\".'
-      });
+  it("not reports left recursion, if some rules in possible recursion path not exist", function() {
+    var tests = [
+      'start = nonexist start;',
+      'start = nonexist start "a";',
+      'start = nonexist start {};',
+      'start = nonexist "a"? start;',
+      'start = nonexist "a"* start;',
+      'start = nonexist !"a" start;',
+      'start = nonexist &"a" start;',
+      'start = nonexist !{} start;',
+      'start = nonexist &{} start;',
+      'start = nonexist "" start;',
+      'start = nonexist [] start;'
+    ];
+    for (var i = 0; i < tests.length; ++i) {
+      expect(pass).not.toReportError(tests[i]);
+    }
+  });
 
-      expect(pass).not.toReportError('start = "a" start "b"');
-      expect(pass).not.toReportError('start = "a" "b" start');
+  describe("in sequences", function() {
+    it("not report left recursion when preceding elements consume input", function() {
+      var tests = [
+        'start = "a" start "b"',
+        'start = "a" "b" start',
+        'start = "a"+ start',
+        'start = ["a"] start',
+        'start = $"a" start',
+        'start = . start',
+        'start = stop; stop = "a" start',
+        'start = (. {}) start',
+        'start = (!{} .) start',
+        'start = (&{} .) start',
+      ];
+      for (var i = 0; i < tests.length; ++i) {
+        expect(pass).not.toReportError(tests[i]);
+      }
     });
   });
 });