Skip to content

Commit

Permalink
[Refactor] add some more type info; switch for-ofs to forEaches
Browse files Browse the repository at this point in the history
  • Loading branch information
@ljharb
ljharb committed Sep 24, 2024
1 parent a9815da commit 5c9757c
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 44 deletions.
19 changes: 17 additions & 2 deletions src/rules/consistent-type-specifier-style.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -38,6 +45,7 @@ function getImportText(
return `import ${kind} {${names.join(', ')}} from ${sourceString};`;
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'suggestion',
Expand Down Expand Up @@ -102,6 +110,7 @@ module.exports = {

// prefer-top-level
return {
/** @param {import('estree').ImportDeclaration} node */
ImportDeclaration(node) {
if (
// already top-level is valid
Expand All @@ -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') {
Expand All @@ -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' : [],
Expand All @@ -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
Expand Down Expand Up @@ -215,7 +230,7 @@ module.exports = {
);
},
});
}
});
}
},
};
Expand Down
4 changes: 2 additions & 2 deletions src/rules/no-cycle.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) });
}
});
}
}
}
Expand Down
34 changes: 23 additions & 11 deletions src/rules/no-duplicates.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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, '{'));
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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));

Expand All @@ -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) {
Expand All @@ -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',
Expand All @@ -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(/^([^?]*)\?(.*)$/);
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
59 changes: 30 additions & 29 deletions src/rules/no-mutable-exports.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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);
});
}
},
};
},
};

0 comments on commit 5c9757c

Please sign in to comment.