Fix left recursion detection

So far, left recursion detector assumed that left recursion occurs only
when the recursive rule is at the very left-hand side of rule's
expression:

  start = start

This didn't catch cases like this:

  start = "a"? start

In general, if a rule reference can be reached without consuming any
input, it can lead to left recursion. This commit fixes the detector to
consider that.

Fixes #190.

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

@@ -5,13 +5,61 @@ var arrays       = require("../../utils/arrays"),
 
 /* Checks that no left recursion is present. */
 function reportLeftRecursion(ast) {
+  function matchesEmptyTrue()  { return true;  }
+  function matchesEmptyFalse() { return false; }
+
+  function matchesEmptyExpression(node) {
+    return matchesEmpty(node.expression);
+  }
+
+  var matchesEmpty = visitor.build({
+    rule: matchesEmptyExpression,
+
+    choice: function(node) {
+      return arrays.some(node.alternatives, matchesEmpty);
+    },
+
+    action: matchesEmptyExpression,
+
+    sequence: function(node) {
+      return arrays.every(node.elements, matchesEmpty);
+    },
+
+    labeled:      matchesEmptyExpression,
+    text:         matchesEmptyExpression,
+    simple_and:   matchesEmptyTrue,
+    simple_not:   matchesEmptyTrue,
+    optional:     matchesEmptyTrue,
+    zero_or_more: matchesEmptyTrue,
+    one_or_more:  matchesEmptyExpression,
+    semantic_and: matchesEmptyTrue,
+    semantic_not: matchesEmptyTrue,
+
+    rule_ref: function(node) {
+      return matchesEmpty(asts.findRule(ast, node.name));
+    },
+
+    literal: function(node) {
+      return node.value === "";
+    },
+
+    "class": matchesEmptyFalse,
+    any:     matchesEmptyFalse
+  });
+
   var check = visitor.build({
     rule: function(node, appliedRules) {
       check(node.expression, appliedRules.concat(node.name));
     },
 
     sequence: function(node, appliedRules) {
-      check(node.elements[0], appliedRules);
+      arrays.every(node.elements, function(element) {
+        if (element.type === "rule_ref") {
+          check(element, appliedRules);
+        }
+
+        return matchesEmpty(element);
+      });
     },
 
     rule_ref: function(node, appliedRules) {
@@ -20,6 +68,7 @@ function reportLeftRecursion(ast) {
           "Left recursion detected for rule \"" + node.name + "\"."
         );
       }
+
       check(asts.findRule(ast, node.name), appliedRules);
     }
   });

lib/utils/arrays.js

@@ -76,6 +76,30 @@ var arrays = {
 
   pluck: function(array, key) {
     return arrays.map(array, function (e) { return e[key]; });
+  },
+
+  every: function(array, predicate) {
+    var length = array.length, i;
+
+    for (i = 0; i < length; i++) {
+      if (!predicate(array[i])) {
+        return false;
+      }
+    }
+
+    return true;
+  },
+
+  some: function(array, predicate) {
+    var length = array.length, i;
+
+    for (i = 0; i < length; i++) {
+      if (predicate(array[i])) {
+        return true;
+      }
+    }
+
+    return false;
   }
 };
 

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

@@ -17,13 +17,70 @@ describe("compiler pass |reportLeftRecursion|", function() {
   });
 
   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("reports left recursion if all preceding elements match empty string", function() {
+      expect(pass).toReportError('start = "" "" "" start');
+    });
+
+    it("doesn't report left recursion if some preceding element doesn't match empty string", function() {
+      expect(pass).not.toReportError('start = "a" "" "" start');
+      expect(pass).not.toReportError('start = "" "a" "" start');
+      expect(pass).not.toReportError('start = "" "" "a" start');
+    });
+
+    it("computes empty string matching correctly", function() {
+      expect(pass).toReportError('start = ("" / "a" / "b") start');
+      expect(pass).toReportError('start = ("a" / "" / "b") start');
+      expect(pass).toReportError('start = ("a" / "b" / "") start');
+      expect(pass).not.toReportError('start = ("a" / "b" / "c") start');
+
+      expect(pass).toReportError('start = ("" { }) start');
+      expect(pass).not.toReportError('start = ("a" { }) start');
+
+      expect(pass).toReportError('start = ("" "" "") start');
+      expect(pass).not.toReportError('start = ("a" "" "") start');
+      expect(pass).not.toReportError('start = ("" "a" "") start');
+      expect(pass).not.toReportError('start = ("" "" "a") start');
+
+      expect(pass).toReportError('start = a:"" start');
+      expect(pass).not.toReportError('start = a:"a" start');
+
+      expect(pass).toReportError('start = $"" start');
+      expect(pass).not.toReportError('start = $"a" start');
+
+      expect(pass).toReportError('start = &"" start');
+      expect(pass).toReportError('start = &"a" start');
+
+      expect(pass).toReportError('start = !"" start');
+      expect(pass).toReportError('start = !"a" start');
+
+      expect(pass).toReportError('start = ""? start');
+      expect(pass).toReportError('start = "a"? start');
+
+      expect(pass).toReportError('start = ""* start');
+      expect(pass).toReportError('start = "a"* start');
+
+      expect(pass).toReportError('start = ""+ start');
+      expect(pass).not.toReportError('start = "a"+ start');
+
+      expect(pass).toReportError('start = &{ } start');
+
+      expect(pass).toReportError('start = !{ } start');
+
+      expect(pass).toReportError([
+        'start = a start',
+        'a = ""'
+      ].join('\n'));
+      expect(pass).not.toReportError([
+        'start = a start',
+        'a = "a"'
+      ].join('\n'));
+
+      expect(pass).toReportError('start = "" start');
+      expect(pass).not.toReportError('start = "a" start');
+
+      expect(pass).not.toReportError('start = [a-d] start');
 
-      expect(pass).not.toReportError('start = "a" start "b"');
-      expect(pass).not.toReportError('start = "a" "b" start');
+      expect(pass).not.toReportError('start = "." start');
     });
   });
 });