From 5e4e50c5eb8ed89f111faf5b18cc2149560d7920 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 17 Mar 2020 13:17:55 -0700 Subject: [PATCH] fix: remove bad usages of for-in and guard against it (#22616) * fix: remove bad usages of for-in and guard against it * Apply suggestions from code review Co-Authored-By: Samuel Maddock * Apply suggestions from code review Co-Authored-By: Jeremy Apthorp * Update remote.js Co-authored-by: Samuel Maddock Co-authored-by: Jeremy Apthorp --- .eslintrc.json | 1 + lib/browser/api/menu.js | 5 ++--- lib/browser/api/net.js | 2 +- lib/browser/guest-view-manager.js | 6 +++--- lib/browser/utils.ts | 2 +- lib/renderer/api/remote.js | 4 ++-- spec/expect-helpers.js | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 91927273c4ab..83c0df2021fc 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -9,6 +9,7 @@ "no-var": "error", "no-unused-vars": 0, "no-global-assign": 0, + "guard-for-in": 2, "@typescript-eslint/no-unused-vars": ["error", { "vars": "all", "args": "after-used", diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 1587c8ce9fa6..50254424237e 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -33,7 +33,7 @@ const delegate = { }, menuWillShow: (menu) => { // Ensure radio groups have at least one menu item selected - for (const id in menu.groupsMap) { + for (const id of Object.keys(menu.groupsMap)) { const found = menu.groupsMap[id].find(item => item.checked) || null if (!found) v8Util.setHiddenValue(menu.groupsMap[id][0], 'checked', true) } @@ -201,8 +201,7 @@ function areValidTemplateItems (template) { function sortTemplate (template) { const sorted = sortMenuItems(template) - for (const id in sorted) { - const item = sorted[id] + for (const item of sorted) { if (Array.isArray(item.submenu)) { item.submenu = sortTemplate(item.submenu) } diff --git a/lib/browser/api/net.js b/lib/browser/api/net.js index 6c7a28e84b75..a6d11a604bcb 100644 --- a/lib/browser/api/net.js +++ b/lib/browser/api/net.js @@ -365,7 +365,7 @@ class ClientRequest extends Writable { this._started = true const stringifyValues = (obj) => { const ret = {} - for (const k in obj) { + for (const k of Object.keys(obj)) { ret[k] = obj[k].toString() } return ret diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index 7baab1b2ca00..77842fd09951 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -74,7 +74,7 @@ const createGuest = function (embedder, params) { // Clear the guest from map when it is destroyed. guest.once('destroyed', () => { - if (guestInstanceId in guestInstances) { + if (Object.prototype.hasOwnProperty.call(guestInstances, guestInstanceId)) { detachGuest(embedder, guestInstanceId) } }) @@ -288,7 +288,7 @@ const watchEmbedder = function (embedder) { // Forward embedder window visiblity change events to guest const onVisibilityChange = function (visibilityState) { - for (const guestInstanceId in guestInstances) { + for (const guestInstanceId of Object.keys(guestInstances)) { const guestInstance = guestInstances[guestInstanceId] guestInstance.visibilityState = visibilityState if (guestInstance.embedder === embedder) { @@ -302,7 +302,7 @@ const watchEmbedder = function (embedder) { // Usually the guestInstances is cleared when guest is destroyed, but it // may happen that the embedder gets manually destroyed earlier than guest, // and the embedder will be invalid in the usual code path. - for (const guestInstanceId in guestInstances) { + for (const guestInstanceId of Object.keys(guestInstances)) { const guestInstance = guestInstances[guestInstanceId] if (guestInstance.embedder === embedder) { detachGuest(embedder, parseInt(guestInstanceId)) diff --git a/lib/browser/utils.ts b/lib/browser/utils.ts index 91e3831e9b33..75bb0394e38a 100644 --- a/lib/browser/utils.ts +++ b/lib/browser/utils.ts @@ -18,7 +18,7 @@ export function createLazyInstance ( ) { let lazyModule: Object const module: any = {} - for (const method in (holder as any).prototype) { + for (const method in (holder as any).prototype) { // eslint-disable-line guard-for-in module[method] = (...args: any) => { // create new instance of module at runtime if none exists if (!lazyModule) { diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 618233e5c0d4..3c630db088b3 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -72,7 +72,7 @@ function wrapArgs (args, visited = new Set()) { members: [] } visited.add(value) - for (const prop in value) { + for (const prop in value) { // eslint-disable-line guard-for-in meta.members.push({ name: prop, value: valueToMeta(value[prop]) @@ -219,7 +219,7 @@ function metaToValue (meta) { exception: () => { throw metaToError(meta.value) } } - if (meta.type in types) { + if (Object.prototype.hasOwnProperty.call(types, meta.type)) { return types[meta.type]() } else { let ret diff --git a/spec/expect-helpers.js b/spec/expect-helpers.js index a825f6d2a5c2..659f63d6c059 100644 --- a/spec/expect-helpers.js +++ b/spec/expect-helpers.js @@ -1,7 +1,7 @@ function resolveSingleObjectGetters (object) { if (object && typeof object === 'object') { const newObject = {} - for (const key in object) { + for (const key in object) { // eslint-disable-line guard-for-in newObject[key] = resolveGetters(object[key])[0] } return newObject