From da2378d887d5e9967728094a5cfac185e593dfbc Mon Sep 17 00:00:00 2001 From: David Majda Date: Fri, 18 Mar 2016 16:39:19 +0100 Subject: [PATCH] Rewrite handling of optional parameters Instead of testing arguments.length to see whether an optional parameter was passed to a function, compare its value to "undefined". This approach has two advantages: * It is in line with handling of default parameters in ES6. * Optional parameters are actually spelled out in the parameter list. There is also one important disadvantage, namely that it's impossible to pass "undefined" as an optional parameter value. This required a small change in two tests. Additional notes: * Default parameter values are set in assignments immediately after the function header. This reflects the fact that these assignments really belong to the parameter list (which is where they are in ES6). * Parameter values are checked against "void 0" in places where "undefined" can potentially be redefiend. --- lib/compiler.js | 8 +++-- lib/compiler/passes/generate-js.js | 14 ++++---- lib/parser.js | 7 ++-- lib/peg.js | 9 +++-- .../generated-parser-behavior.spec.js | 36 ++++++++++++------- spec/unit/compiler/passes/helpers.js | 7 ++-- 6 files changed, 50 insertions(+), 31 deletions(-) diff --git a/lib/compiler.js b/lib/compiler.js index e1e1642..244755b 100644 --- a/lib/compiler.js +++ b/lib/compiler.js @@ -38,10 +38,12 @@ var compiler = { * during the generation and some may protrude to the generated parser and * cause its malfunction. */ - compile: function(ast, passes) { - var options = arguments.length > 2 ? objects.clone(arguments[2]) : {}, - stage; + compile: function(ast, passes, options) { + options = options !== void 0 ? options : {}; + var stage; + + options = objects.clone(options); objects.defaults(options, { allowedStartRules: [ast.rules[0].name], cache: false, diff --git a/lib/compiler/passes/generate-js.js b/lib/compiler/passes/generate-js.js index 62a5eca..799231e 100644 --- a/lib/compiler/passes/generate-js.js +++ b/lib/compiler/passes/generate-js.js @@ -437,13 +437,12 @@ function generateJS(ast, options) { return code; }, - pop: function() { - var n, values; + pop: function(n) { + var values; - if (arguments.length === 0) { + if (n === void 0) { return s(this.sp--); } else { - n = arguments[0]; values = arrays.map(arrays.range(this.sp - n + 1, this.sp + 1), s); this.sp -= n; @@ -851,9 +850,10 @@ function generateJS(ast, options) { } parts.push([ - ' function peg$parse(input) {', - ' var options = arguments.length > 1 ? arguments[1] : {},', - ' parser = this,', + ' function peg$parse(input, options) {', + ' options = options !== void 0 ? options : {};', + '', + ' var parser = this,', '', ' peg$FAILED = {},', '' diff --git a/lib/parser.js b/lib/parser.js index 56c812a..dd38f53 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -28,9 +28,10 @@ module.exports = (function() { peg$subclass(peg$SyntaxError, Error); - function peg$parse(input) { - var options = arguments.length > 1 ? arguments[1] : {}, - parser = this, + function peg$parse(input, options) { + options = options !== void 0 ? options : {}; + + var parser = this, peg$FAILED = {}, diff --git a/lib/peg.js b/lib/peg.js index 912cbe8..04475bb 100644 --- a/lib/peg.js +++ b/lib/peg.js @@ -22,7 +22,9 @@ var PEG = { * errors are detected during the generation and some may protrude to the * generated parser and cause its malfunction. */ - buildParser: function(grammar) { + buildParser: function(grammar, options) { + options = options !== void 0 ? options : {}; + function convertPasses(passes) { var converted = {}, stage; @@ -35,8 +37,9 @@ var PEG = { return converted; } - var options = arguments.length > 1 ? objects.clone(arguments[1]) : {}, - plugins = "plugins" in options ? options.plugins : [], + options = objects.clone(options); + + var plugins = "plugins" in options ? options.plugins : [], config = { parser: this.parser, passes: convertPasses(this.compiler.passes) diff --git a/spec/behavior/generated-parser-behavior.spec.js b/spec/behavior/generated-parser-behavior.spec.js index 196491d..d45b1ed 100644 --- a/spec/behavior/generated-parser-behavior.spec.js +++ b/spec/behavior/generated-parser-behavior.spec.js @@ -34,14 +34,15 @@ describe("generated parser behavior", function() { beforeEach(function() { this.addMatchers({ - toParse: function(input, expected) { - var options = arguments.length > 2 ? arguments[2] : {}, - result; + toParse: function(input, expected, options) { + options = options !== undefined ? options : {}; + + var result; try { result = this.actual.parse(input, options); - if (arguments.length > 1) { + if (expected !== undefined) { this.message = function() { return "Expected " + jasmine.pp(input) + " " + "with options " + jasmine.pp(options) + " " @@ -58,7 +59,7 @@ describe("generated parser behavior", function() { this.message = function() { return "Expected " + jasmine.pp(input) + " " + "with options " + jasmine.pp(options) + " " - + "to parse" + (arguments.length > 1 ? " as " + jasmine.pp(expected) : "") + ", " + + "to parse" + (expected !== undefined ? " as " + jasmine.pp(expected) : "") + ", " + "but it failed to parse with message " + jasmine.pp(e.message) + "."; }; @@ -67,9 +68,10 @@ describe("generated parser behavior", function() { } }, - toFailToParse: function(input, details) { - var options = arguments.length > 2 ? arguments[2] : {}, - result; + toFailToParse: function(input, details, options) { + options = options !== undefined ? options : {}; + + var result; try { result = this.actual.parse(input, options); @@ -417,9 +419,14 @@ describe("generated parser behavior", function() { describe("positive semantic predicate", function() { describe("when the code returns a truthy value", function() { it("returns |undefined|", function() { - var parser = PEG.buildParser('start = &{ return true; }', options); + /* + * The |""| is needed so that the parser doesn't return just + * |undefined| which we can't compare against in |toParse| due to the + * way optional parameters work. + */ + var parser = PEG.buildParser('start = &{ return true; } ""', options); - expect(parser).toParse("", undefined); + expect(parser).toParse("", [undefined, ""]); }); }); @@ -617,9 +624,14 @@ describe("generated parser behavior", function() { describe("negative semantic predicate", function() { describe("when the code returns a falsey value", function() { it("returns |undefined|", function() { - var parser = PEG.buildParser('start = !{ return false; }', options); + /* + * The |""| is needed so that the parser doesn't return just + * |undefined| which we can't compare against in |toParse| due to the + * way optional parameters work. + */ + var parser = PEG.buildParser('start = !{ return false; } ""', options); - expect(parser).toParse("", undefined); + expect(parser).toParse("", [undefined, ""]); }); }); diff --git a/spec/unit/compiler/passes/helpers.js b/spec/unit/compiler/passes/helpers.js index 5c862d5..504ca18 100644 --- a/spec/unit/compiler/passes/helpers.js +++ b/spec/unit/compiler/passes/helpers.js @@ -4,7 +4,9 @@ beforeEach(function() { this.addMatchers({ - toChangeAST: function(grammar, details) { + toChangeAST: function(grammar, details, options) { + options = options !== undefined ? options : {}; + function matchDetails(value, details) { function isArray(value) { return Object.prototype.toString.apply(value) === "[object Array]"; @@ -42,8 +44,7 @@ beforeEach(function() { } } - var options = arguments.length > 2 ? arguments[2] : {}, - ast = PEG.parser.parse(grammar); + var ast = PEG.parser.parse(grammar); this.actual(ast, options);