From 398143398494b96834ef998597a735e8c5498fe2 Mon Sep 17 00:00:00 2001 From: David Majda Date: Sun, 6 Jan 2013 10:16:17 +0100 Subject: [PATCH] Fix too eager proxy rules removal Fixes GH-137. --- lib/compiler/passes/remove-proxy-rules.js | 12 +++++++----- spec/compiler/passes/remove-proxy-rules.spec.js | 17 +++++++++++++++-- spec/helpers.js | 9 ++++++--- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/lib/compiler/passes/remove-proxy-rules.js b/lib/compiler/passes/remove-proxy-rules.js index b5da0d2..6c59570 100644 --- a/lib/compiler/passes/remove-proxy-rules.js +++ b/lib/compiler/passes/remove-proxy-rules.js @@ -3,7 +3,7 @@ var utils = require("../../utils"); /* * Removes proxy rules -- that is, rules that only delegate to other rule. */ -module.exports = function(ast) { +module.exports = function(ast, options) { function isProxyRule(node) { return node.type === "rule" && node.expression.type === "rule_ref"; } @@ -55,15 +55,17 @@ module.exports = function(ast) { replace(ast, from, to); } - var indices = []; + var indices = [], + allowedStartRules = options.allowedStartRules !== undefined + ? options.allowedStartRules + : [ast.startRule]; utils.each(ast.rules, function(rule, i) { if (isProxyRule(rule)) { replaceRuleRefs(ast, rule.name, rule.expression.name); - if (rule.name === ast.startRule) { - ast.startRule = rule.expression.name; + if (!utils.contains(allowedStartRules, rule.name)) { + indices.push(i); } - indices.push(i); } }); diff --git a/spec/compiler/passes/remove-proxy-rules.spec.js b/spec/compiler/passes/remove-proxy-rules.spec.js index f72ac83..c8433df 100644 --- a/spec/compiler/passes/remove-proxy-rules.spec.js +++ b/spec/compiler/passes/remove-proxy-rules.spec.js @@ -19,8 +19,8 @@ describe("compiler pass |removeProxyRules|", function() { it("removes proxy rule from a rule", function() { expect(pass).toChangeAST(proxyGrammar('start = proxy'), { - startRule: "proxied", - rules: [ + rules: [ + { type: "rule", name: "start", expression: proxiedRefDetails }, { type: "rule", name: "proxied" } ] }); @@ -86,4 +86,17 @@ describe("compiler pass |removeProxyRules|", function() { it("removes proxy rule from a one or more", function() { expect(pass).toChangeAST(proxyGrammar('start = proxy+'), simpleDetails); }); + + it("doesn't remove a proxy rule listed in |allowedStartRules|", function() { + expect(pass).toChangeAST( + proxyGrammar('start = proxy'), + { allowedStartRules: ["proxy"] }, + { + rules: [ + { name: "proxy" }, + { name: "proxied" } + ] + } + ); + }); }); diff --git a/spec/helpers.js b/spec/helpers.js index 6b65989..64970ce 100644 --- a/spec/helpers.js +++ b/spec/helpers.js @@ -4,7 +4,7 @@ if (typeof module !== "undefined") { beforeEach(function() { this.addMatchers({ - toChangeAST: function(grammar, details) { + toChangeAST: function(grammar) { function matchDetails(value, details) { function isArray(value) { return Object.prototype.toString.apply(value) === "[object Array]"; @@ -40,12 +40,15 @@ beforeEach(function() { } } - var ast = PEG.parser.parse(grammar); + var options = arguments.length > 2 ? arguments[1] : {}, + details = arguments[arguments.length - 1], + ast = PEG.parser.parse(grammar); - this.actual(ast); + this.actual(ast, options); this.message = function() { return "Expected the pass " + + "with options " + jasmine.pp(options) + " " + (this.isNot ? "not " : "") + "to change the AST " + jasmine.pp(ast) + " " + "to match " + jasmine.pp(details) + ", "