From 7f01db2fb804b21c26e64c416e7d69d0abaf39f3 Mon Sep 17 00:00:00 2001 From: David Majda Date: Wed, 14 Sep 2016 16:08:14 +0200 Subject: [PATCH] Get rid of for-in loops The for-in statement in JavaScript iterates also over inherited properties. This is typically not desired and requires adding a check using Object.prototype.hasOwnProperty inside the loop. This commit replaces all for-in statements and related checks inside them with iteration over Object.keys(...). The iteration is performed using either Array.prototype.forEach of a plain for loop. --- bin/pegjs | 8 ++--- lib/compiler/index.js | 8 ++--- lib/peg.js | 10 +++--- .../generated-parser-behavior.spec.js | 35 +++++++++--------- spec/unit/compiler/passes/helpers.js | 36 ++++++++++--------- spec/unit/parser.spec.js | 23 ++++++------ 6 files changed, 58 insertions(+), 62 deletions(-) diff --git a/bin/pegjs b/bin/pegjs index f29f7f7..26ed52c 100755 --- a/bin/pegjs +++ b/bin/pegjs @@ -69,11 +69,9 @@ function addExtraOptions(options, json) { abort("The JSON with extra options has to represent an object."); } - for (let key in extraOptions) { - if (extraOptions.hasOwnProperty(key)) { - options[key] = extraOptions[key]; - } - } + Object.keys(extraOptions).forEach(key => { + options[key] = extraOptions[key]; + }); } /* diff --git a/lib/compiler/index.js b/lib/compiler/index.js index 03fcf32..72f4bbe 100644 --- a/lib/compiler/index.js +++ b/lib/compiler/index.js @@ -67,11 +67,9 @@ let compiler = { trace: false }); - for (let stage in passes) { - if (passes.hasOwnProperty(stage)) { - passes[stage].forEach(p => { p(ast, options); }); - } - } + Object.keys(passes).forEach(stage => { + passes[stage].forEach(p => { p(ast, options); }); + }); switch (options.output) { case "parser": return eval(ast.code); diff --git a/lib/peg.js b/lib/peg.js index 24e55da..ee85990 100644 --- a/lib/peg.js +++ b/lib/peg.js @@ -25,12 +25,10 @@ let peg = { function convertPasses(passes) { let converted = {}; - for (let stage in passes) { - if (passes.hasOwnProperty(stage)) { - converted[stage] = Object.keys(passes[stage]) - .map(name => passes[stage][name]); - } - } + Object.keys(passes).forEach(stage => { + converted[stage] = Object.keys(passes[stage]) + .map(name => passes[stage][name]); + }); return converted; } diff --git a/spec/behavior/generated-parser-behavior.spec.js b/spec/behavior/generated-parser-behavior.spec.js index 3c4c929..3119abd 100644 --- a/spec/behavior/generated-parser-behavior.spec.js +++ b/spec/behavior/generated-parser-behavior.spec.js @@ -10,11 +10,9 @@ describe("generated parser behavior", function() { function clone(object) { let result = {}; - for (let key in object) { - if (object.hasOwnProperty(key)) { - result[key] = object[key]; - } - } + Object.keys(object).forEach(key => { + result[key] = object[key]; + }); return result; } @@ -89,19 +87,20 @@ describe("generated parser behavior", function() { + jasmine.pp(e.message) + "."; } else { if (details) { - for (let key in details) { - if (details.hasOwnProperty(key)) { - if (!this.env.equals_(e[key], details[key])) { - this.message = () => - "Expected " + jasmine.pp(input) + " " - + "with options " + jasmine.pp(options) + " " - + "to fail to parse" - + (details ? " with details " + jasmine.pp(details) : "") + ", " - + "but " + jasmine.pp(key) + " " - + "is " + jasmine.pp(e[key]) + "."; - - return false; - } + let keys = Object.keys(details); + for (let i = 0; i < keys.length; i++) { + let key = keys[i]; + + if (!this.env.equals_(e[key], details[key])) { + this.message = () => + "Expected " + jasmine.pp(input) + " " + + "with options " + jasmine.pp(options) + " " + + "to fail to parse" + + (details ? " with details " + jasmine.pp(details) : "") + ", " + + "but " + jasmine.pp(key) + " " + + "is " + jasmine.pp(e[key]) + "."; + + return false; } } } diff --git a/spec/unit/compiler/passes/helpers.js b/spec/unit/compiler/passes/helpers.js index 9dbaa6d..12dbe23 100644 --- a/spec/unit/compiler/passes/helpers.js +++ b/spec/unit/compiler/passes/helpers.js @@ -28,12 +28,13 @@ beforeEach(function() { } else if (isObject(details)) { if (!isObject(value)) { return false; } - for (let key in details) { - if (details.hasOwnProperty(key)) { - if (!(key in value)) { return false; } + let keys = Object.keys(details); + for (let i = 0; i < keys.length; i++) { + let key = keys[i]; - if (!matchDetails(value[key], details[key])) { return false; } - } + if (!(key in value)) { return false; } + + if (!matchDetails(value[key], details[key])) { return false; } } return true; @@ -70,18 +71,19 @@ beforeEach(function() { + "but it did."; } else { if (details) { - for (let key in details) { - if (details.hasOwnProperty(key)) { - if (!this.env.equals_(e[key], details[key])) { - this.message = () => - "Expected the pass to report an error " - + "with details " + jasmine.pp(details) + " " - + "for grammar " + jasmine.pp(grammar) + ", " - + "but " + jasmine.pp(key) + " " - + "is " + jasmine.pp(e[key]) + "."; - - return false; - } + let keys = Object.keys(details); + for (let i = 0; i < keys.length; i++) { + let key = keys[i]; + + if (!this.env.equals_(e[key], details[key])) { + this.message = () => + "Expected the pass to report an error " + + "with details " + jasmine.pp(details) + " " + + "for grammar " + jasmine.pp(grammar) + ", " + + "but " + jasmine.pp(key) + " " + + "is " + jasmine.pp(e[key]) + "."; + + return false; } } } diff --git a/spec/unit/parser.spec.js b/spec/unit/parser.spec.js index c7ec8d4..b368036 100644 --- a/spec/unit/parser.spec.js +++ b/spec/unit/parser.spec.js @@ -204,17 +204,18 @@ describe("PEG.js grammar parser", function() { + jasmine.pp(e.message) + "."; } else { if (details) { - for (let key in details) { - if (details.hasOwnProperty(key)) { - if (!this.env.equals_(e[key], details[key])) { - this.message = () => - "Expected " + jasmine.pp(this.actual) + " to fail to parse" - + (details ? " with details " + jasmine.pp(details) : "") + ", " - + "but " + jasmine.pp(key) + " " - + "is " + jasmine.pp(e[key]) + "."; - - return false; - } + let keys = Object.keys(details); + for (let i = 0; i < keys.length; i++) { + let key = keys[i]; + + if (!this.env.equals_(e[key], details[key])) { + this.message = () => + "Expected " + jasmine.pp(this.actual) + " to fail to parse" + + (details ? " with details " + jasmine.pp(details) : "") + ", " + + "but " + jasmine.pp(key) + " " + + "is " + jasmine.pp(e[key]) + "."; + + return false; } } }