[Refactor] add some more type info; switch for-ofs to forEaches

Author: ljharb

src/rules/consistent-type-specifier-style.js

@@ -6,6 +6,12 @@ function isComma(token) {
   return token.type === 'Punctuator' && token.value === ',';
 }
 
+/**
+ * @param {import('eslint').Rule.Fix[]} fixes
+ * @param {import('eslint').Rule.RuleFixer} fixer
+ * @param {import('eslint').SourceCode.SourceCode} sourceCode
+ * @param {(ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier)[]} specifiers
+ * */
 function removeSpecifiers(fixes, fixer, sourceCode, specifiers) {
   for (const specifier of specifiers) {
     // remove the trailing comma
@@ -17,6 +23,7 @@ function removeSpecifiers(fixes, fixer, sourceCode, specifiers) {
   }
 }
 
+/** @type {(node: import('estree').Node, sourceCode: import('eslint').SourceCode.SourceCode, specifiers: (ImportSpecifier | ImportNamespaceSpecifier)[], kind: 'type' | 'typeof') => string} */
 function getImportText(
   node,
   sourceCode,
@@ -38,6 +45,7 @@ function getImportText(
   return `import ${kind} {${names.join(', ')}} from ${sourceString};`;
 }
 
+/** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
     type: 'suggestion',
@@ -102,6 +110,7 @@ module.exports = {
 
     // prefer-top-level
     return {
+      /** @param {import('estree').ImportDeclaration} node */
       ImportDeclaration(node) {
         if (
           // already top-level is valid
@@ -120,9 +129,13 @@ module.exports = {
           return;
         }
 
+        /** @type {typeof node.specifiers} */
         const typeSpecifiers = [];
+        /** @type {typeof node.specifiers} */
         const typeofSpecifiers = [];
+        /** @type {typeof node.specifiers} */
         const valueSpecifiers = [];
+        /** @type {typeof node.specifiers[number]} */
         let defaultSpecifier = null;
         for (const specifier of node.specifiers) {
           if (specifier.type === 'ImportDefaultSpecifier') {
@@ -144,6 +157,7 @@ module.exports = {
         const newImports = `${typeImport}\n${typeofImport}`.trim();
 
         if (typeSpecifiers.length + typeofSpecifiers.length === node.specifiers.length) {
+          /** @type {('type' | 'typeof')[]} */
           // all specifiers have inline specifiers - so we replace the entire import
           const kind = [].concat(
             typeSpecifiers.length > 0 ? 'type' : [],
@@ -162,14 +176,15 @@ module.exports = {
           });
         } else {
           // remove specific specifiers and insert new imports for them
-          for (const specifier of typeSpecifiers.concat(typeofSpecifiers)) {
+          typeSpecifiers.concat(typeofSpecifiers).forEach((specifier) => {
             context.report({
               node: specifier,
               message: 'Prefer using a top-level {{kind}}-only import instead of inline {{kind}} specifiers.',
               data: {
                 kind: specifier.importKind,
               },
               fix(fixer) {
+                /** @type {import('eslint').Rule.Fix[]} */
                 const fixes = [];
 
                 // if there are no value specifiers, then the other report fixer will be called, not this one
@@ -215,7 +230,7 @@ module.exports = {
                 );
               },
             });
-          }
+          });
         }
       },
     };

src/rules/no-cycle.js

@@ -152,9 +152,9 @@ module.exports = {
           */
           if (path === myPath && toTraverse.length > 0) { return true; }
           if (route.length + 1 < maxDepth) {
-            for (const { source } of toTraverse) {
+            toTraverse.forEach(({ source }) => {
               untraversed.push({ mget: getter, route: route.concat(source) });
-            }
+            });
           }
         }
       }

src/rules/no-duplicates.js

@@ -74,6 +74,7 @@ function hasProblematicComments(node, sourceCode) {
   );
 }
 
+/** @type {(first: import('estree').ImportDeclaration, rest: import('estree').ImportDeclaration[], sourceCode: import('eslint').SourceCode.SourceCode, context: import('eslint').Rule.RuleContext) => import('eslint').Rule.ReportFixer | undefined} */
 function getFix(first, rest, sourceCode, context) {
   // Sorry ESLint <= 3 users, no autofix for you. Autofixing duplicate imports
   // requires multiple `fixer.whatever()` calls in the `fix`: We both need to
@@ -123,7 +124,7 @@ function getFix(first, rest, sourceCode, context) {
         isEmpty: !hasSpecifiers(node),
       };
     })
-    .filter(Boolean);
+    .filter((x) => !!x);
 
   const unnecessaryImports = restWithoutComments.filter((node) => !hasSpecifiers(node)
     && !hasNamespace(node)
@@ -139,6 +140,7 @@ function getFix(first, rest, sourceCode, context) {
     return undefined;
   }
 
+  /** @type {import('eslint').Rule.ReportFixer} */
   return (fixer) => {
     const tokens = sourceCode.getTokens(first);
     const openBrace = tokens.find((token) => isPunctuator(token, '{'));
@@ -185,6 +187,7 @@ function getFix(first, rest, sourceCode, context) {
       ['', !firstHasTrailingComma && !firstIsEmpty, firstExistingIdentifiers],
     );
 
+    /** @type {import('eslint').Rule.Fix[]} */
     const fixes = [];
 
     if (shouldAddSpecifiers && preferInline && first.importKind === 'type') {
@@ -228,7 +231,7 @@ function getFix(first, rest, sourceCode, context) {
     }
 
     // Remove imports whose specifiers have been moved into the first import.
-    for (const specifier of specifiers) {
+    specifiers.forEach((specifier) => {
       const importNode = specifier.importNode;
       fixes.push(fixer.remove(importNode));
 
@@ -237,25 +240,26 @@ function getFix(first, rest, sourceCode, context) {
       if (charAfterImport === '\n') {
         fixes.push(fixer.removeRange(charAfterImportRange));
       }
-    }
+    });
 
     // Remove imports whose default import has been moved to the first import,
     // and side-effect-only imports that are unnecessary due to the first
     // import.
-    for (const node of unnecessaryImports) {
+    unnecessaryImports.forEach((node) => {
       fixes.push(fixer.remove(node));
 
       const charAfterImportRange = [node.range[1], node.range[1] + 1];
       const charAfterImport = sourceCode.text.substring(charAfterImportRange[0], charAfterImportRange[1]);
       if (charAfterImport === '\n') {
         fixes.push(fixer.removeRange(charAfterImportRange));
       }
-    }
+    });
 
     return fixes;
   };
 }
 
+/** @type {(imported: Map<string, import('estree').ImportDeclaration[]>, context: import('eslint').Rule.RuleContext) => void} */
 function checkImports(imported, context) {
   for (const [module, nodes] of imported.entries()) {
     if (nodes.length > 1) {
@@ -270,16 +274,17 @@ function checkImports(imported, context) {
         fix, // Attach the autofix (if any) to the first import.
       });
 
-      for (const node of rest) {
+      rest.forEach((node) => {
         context.report({
           node: node.source,
           message,
         });
-      }
+      });
     }
   }
 }
 
+/** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
     type: 'problem',
@@ -305,10 +310,13 @@ module.exports = {
     ],
   },
 
+  /** @param {import('eslint').Rule.RuleContext} context */
   create(context) {
+    /** @type {boolean} */
     // Prepare the resolver from options.
-    const considerQueryStringOption = context.options[0]
-      && context.options[0].considerQueryString;
+    const considerQueryStringOption = context.options[0] && context.options[0].considerQueryString;
+    /** @type {boolean} */
+    const preferInline = context.options[0] && context.options[0]['prefer-inline'];
     const defaultResolver = (sourcePath) => resolve(sourcePath, context) || sourcePath;
     const resolver = considerQueryStringOption ? (sourcePath) => {
       const parts = sourcePath.match(/^([^?]*)\?(.*)$/);
@@ -318,19 +326,21 @@ module.exports = {
       return `${defaultResolver(parts[1])}?${parts[2]}`;
     } : defaultResolver;
 
+    /** @type {Map<unknown, { imported: Map<string, import('estree').ImportDeclaration[]>, nsImported: Map<string, import('estree').ImportDeclaration[]>, defaultTypesImported: Map<string, import('estree').ImportDeclaration[]>, namedTypesImported: Map<string, import('estree').ImportDeclaration[]>}>} */
     const moduleMaps = new Map();
 
+    /** @param {import('estree').ImportDeclaration} n */
+    /** @returns {typeof moduleMaps[keyof typeof moduleMaps]} */
     function getImportMap(n) {
       if (!moduleMaps.has(n.parent)) {
-        moduleMaps.set(n.parent, {
+        moduleMaps.set(n.parent, /** @type {typeof moduleMaps} */ {
           imported: new Map(),
           nsImported: new Map(),
           defaultTypesImported: new Map(),
           namedTypesImported: new Map(),
         });
       }
       const map = moduleMaps.get(n.parent);
-      const preferInline = context.options[0] && context.options[0]['prefer-inline'];
       if (!preferInline && n.importKind === 'type') {
         return n.specifiers.length > 0 && n.specifiers[0].type === 'ImportDefaultSpecifier' ? map.defaultTypesImported : map.namedTypesImported;
       }
@@ -342,7 +352,9 @@ module.exports = {
     }
 
     return {
+      /** @param {import('estree').ImportDeclaration} n */
       ImportDeclaration(n) {
+        /** @type {string} */
         // resolved path will cover aliased duplicates
         const resolvedPath = resolver(n.source.value);
         const importMap = getImportMap(n);

src/rules/no-mutable-exports.js

@@ -2,6 +2,7 @@ import { getScope } from 'eslint-module-utils/contextCompat';
 
 import docsUrl from '../docsUrl';
 
+/** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
     type: 'suggestion',
@@ -21,41 +22,41 @@ module.exports = {
       }
     }
 
+    /** @type {(scope: import('eslint').Scope.Scope, name: string) => void} */
     function checkDeclarationsInScope({ variables }, name) {
-      for (const variable of variables) {
-        if (variable.name === name) {
-          for (const def of variable.defs) {
-            if (def.type === 'Variable' && def.parent) {
+      variables
+        .filter((variable) => variable.name === name)
+        .forEach((variable) => {
+          variable.defs
+            .filter((def) => def.type === 'Variable' && def.parent)
+            .forEach((def) => {
               checkDeclaration(def.parent);
-            }
-          }
-        }
-      }
-    }
-
-    function handleExportDefault(node) {
-      const scope = getScope(context, node);
-
-      if (node.declaration.name) {
-        checkDeclarationsInScope(scope, node.declaration.name);
-      }
+            });
+        });
     }
 
-    function handleExportNamed(node) {
-      const scope = getScope(context, node);
+    return {
+      /** @param {import('estree').ExportDefaultDeclaration} node */
+      ExportDefaultDeclaration(node) {
+        const scope = getScope(context, node);
 
-      if (node.declaration)  {
-        checkDeclaration(node.declaration);
-      } else if (!node.source) {
-        for (const specifier of node.specifiers) {
-          checkDeclarationsInScope(scope, specifier.local.name);
+        if ('name' in node.declaration && node.declaration.name) {
+          checkDeclarationsInScope(scope, node.declaration.name);
         }
-      }
-    }
-
-    return {
-      ExportDefaultDeclaration: handleExportDefault,
-      ExportNamedDeclaration: handleExportNamed,
+      },
+
+      /** @param {import('estree').ExportNamedDeclaration} node */
+      ExportNamedDeclaration(node) {
+        const scope = getScope(context, node);
+
+        if ('declaration' in node && node.declaration)  {
+          checkDeclaration(node.declaration);
+        } else if (!('source' in node) || !node.source) {
+          node.specifiers.forEach((specifier) => {
+            checkDeclarationsInScope(scope, specifier.local.name);
+          });
+        }
+      },
     };
   },
 };