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 <samuel.maddock@gmail.com>

* Apply suggestions from code review

Co-Authored-By: Jeremy Apthorp <jeremya@chromium.org>

* Update remote.js

Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>
Co-authored-by: Jeremy Apthorp <jeremya@chromium.org>

.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",

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) {
+  for (const item of sorted) {
-    const item = sorted[id]
     if (Array.isArray(item.submenu)) {
       item.submenu = sortTemplate(item.submenu)
     }

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

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))

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) {

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

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