Merge pull request #467 from hildjj/infinite-recursion

Fix #379.

CHANGELOG.md

@@ -27,6 +27,14 @@ Released: TBD
   `import` or `require` in the JavaScript output, depending on the value of
   the `format` parameter.  This will need explicit support from
   plugins, with a few new AST node types and a few visitor changes.
+- [#379](https://github.com/peggyjs/peggy/issues/379) Fix infinite recursion
+  issue by moving reportInfiniteRecursion to the new prepare pass, and having
+  it fail after finding the first issue.  This will require plugin authors
+  to ensure that reportInfiniteRecursion is in the new pass correctly.
+- [#463](https://github.com/peggyjs/peggy/issues/463) Drop support for
+  Internet Explorer.  Move to eslint flat configs in order to lint minimized
+  browser code for compatibility with
+  `defaults, maintained node versions, not op_mini all`.
 
 ### Minor Changes
 

lib/compiler/index.js

@@ -54,12 +54,12 @@ const compiler = {
   passes: {
     prepare: [
       addImportedRules,
+      reportInfiniteRecursion,
     ],
     check: [
       reportUndefinedRules,
       reportDuplicateRules,
       reportDuplicateLabels,
-      reportInfiniteRecursion,
       reportInfiniteRepetition,
       reportIncorrectPlucking,
       reportDuplicateImports,

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

@@ -21,20 +21,31 @@ function reportInfiniteRecursion(ast, options, session) {
 
   const check = visitor.build({
     rule(node) {
+      if (session.errors > 0) {
+        return;
+      }
       visitedRules.push(node.name);
       check(node.expression);
       visitedRules.pop();
     },
 
     sequence(node) {
+      if (session.errors > 0) {
+        return;
+      }
       node.elements.every(element => {
         check(element);
-
+        if (session.errors > 0) {
+          return false;
+        }
         return !asts.alwaysConsumesOnSuccess(ast, element);
       });
     },
 
     repeated(node) {
+      if (session.errors > 0) {
+        return;
+      }
       check(node.expression);
 
       // If an expression does not consume input then recursion
@@ -47,6 +58,10 @@ function reportInfiniteRecursion(ast, options, session) {
     },
 
     rule_ref(node) {
+      if (session.errors > 0) {
+        return;
+      }
+
       backtraceRefs.push(node);
 
       const rule = asts.findRule(ast, node.name);

test/behavior/generated-parser-behavior.spec.js

@@ -3032,13 +3032,32 @@ Error: Expected "import", "{", code block, comment, end of line, identifier, or
       }
     });
 
+    it("detects infinite loops before other errors", () => {
+      try {
+        peg.generate(`
+          start = start
+          dup = label:"foo" label:"bar"
+        `);
+      } catch (e) {
+        expect(e).with.property("stage", "prepare");
+        expect(e).with.property("problems").to.be.an("array");
+        // Get second elements of errors (error messages)
+        const messages = e.problems.filter(p => p[0] === "error").map(p => p[1]);
+
+        // Check that only infloop errors are present
+        expect(messages).to.deep.equal([
+          "Possible infinite loop when parsing (left recursion: start -> start)",
+        ]);
+      }
+    });
+
     it("reports multiple errors in each compilation stage", () => {
       try {
         peg.generate(`
           start = leftRecursion
           leftRecursion = duplicatedLabel:duplicatedRule duplicatedLabel:missingRule
           duplicatedRule = missingRule
-          duplicatedRule = start
+          duplicatedRule = "nothing"
         `);
       } catch (e) {
         expect(e).with.property("stage", "check");
@@ -3058,7 +3077,6 @@ Error: Expected "import", "{", code block, comment, end of line, identifier, or
           "Rule \"missingRule\" is not defined",
           "Rule \"duplicatedRule\" is already defined",
           "Label \"duplicatedLabel\" is already defined",
-          "Possible infinite loop when parsing (left recursion: duplicatedRule -> start -> leftRecursion -> duplicatedRule)",
         ]);
       }
     });

test/unit/compiler/passes/report-infinite-recursion.spec.js

@@ -157,5 +157,17 @@ describe("compiler pass |reportInfiniteRecursion|", () => {
       expect(pass).to.reportError("start = ''|2..3, start|");
       expect(pass).to.reportError("start = ''| 42 , start|");
     });
+
+    it("does not inifinite loop", () => {
+      // From https://github.com/peggyjs/peggy/issues/379
+      expect(pass).to.reportError(`
+        start = expr*
+
+        expr
+          = expr "++"
+      `, {
+        message: "Possible infinite loop when parsing (left recursion: start -> expr -> expr)",
+      });
+    });
   });
 });