From 48af5db61da527f1f4ede38a9eb59bde5cdfc255 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 25 Jul 2024 11:22:09 -0700 Subject: [PATCH] Implement autofix for eslint sort imports rule (#6766) --- .../rules/sort-imports.js | 126 +++++++++++------- .../test/sort-imports.test.js | 24 ++-- 2 files changed, 90 insertions(+), 60 deletions(-) diff --git a/packages/dev/eslint-plugin-rsp-rules/rules/sort-imports.js b/packages/dev/eslint-plugin-rsp-rules/rules/sort-imports.js index f37a8355cd9..630ffc70713 100644 --- a/packages/dev/eslint-plugin-rsp-rules/rules/sort-imports.js +++ b/packages/dev/eslint-plugin-rsp-rules/rules/sort-imports.js @@ -16,7 +16,6 @@ module.exports = { }, create: function (context) { const sourceCode = context.getSourceCode(); - let previousDeclaration = null; /** * Gets the local name of the first imported module. @@ -24,67 +23,98 @@ module.exports = { * @returns {?string} the local name of the first imported module. */ function getFirstLocalMemberName(node) { - if (node.specifiers[0]) { + if (node.type === 'ImportDeclaration' && node.specifiers[0]) { return node.specifiers[0].local.name.toLowerCase(); } return null; - } return { - ImportDeclaration(node) { - if (previousDeclaration) { - let currentLocalMemberName = getFirstLocalMemberName(node), - previousLocalMemberName = getFirstLocalMemberName(previousDeclaration); + Program(node) { + let lastImportDeclaration = null; + node.body.forEach((statement, i) => { + if (statement.type === 'ImportDeclaration') { + const importSpecifiers = statement.specifiers.filter(specifier => specifier.type === 'ImportSpecifier'); + const getSortableName = specifier => specifier.local.name.toLowerCase(); + const firstUnsortedIndex = importSpecifiers.map(getSortableName).findIndex((name, index, array) => array[index - 1] > name); - if (previousLocalMemberName && - currentLocalMemberName && - currentLocalMemberName < previousLocalMemberName - ) { - context.report({ - node, - message: 'Imports should be sorted alphabetically.' - }); - } - } + if (firstUnsortedIndex !== -1) { + context.report({ + node: importSpecifiers[firstUnsortedIndex], + message: "Member '{{memberName}}' of the import declaration should be sorted alphabetically.", + data: {memberName: importSpecifiers[firstUnsortedIndex].local.name}, + fix(fixer) { + return fixer.replaceTextRange( + [importSpecifiers[0].range[0], importSpecifiers[importSpecifiers.length - 1].range[1]], + importSpecifiers + // Clone the importSpecifiers array to avoid mutating it + .slice() - const importSpecifiers = node.specifiers.filter(specifier => specifier.type === 'ImportSpecifier'); - const getSortableName = specifier => specifier.local.name.toLowerCase(); - const firstUnsortedIndex = importSpecifiers.map(getSortableName).findIndex((name, index, array) => array[index - 1] > name); + // Sort the array into the desired order + .sort((specifierA, specifierB) => { + const aName = getSortableName(specifierA); + const bName = getSortableName(specifierB); + return aName > bName ? 1 : -1; + }) - if (firstUnsortedIndex !== -1) { - context.report({ - node: importSpecifiers[firstUnsortedIndex], - message: "Member '{{memberName}}' of the import declaration should be sorted alphabetically.", - data: {memberName: importSpecifiers[firstUnsortedIndex].local.name}, - fix(fixer) { - return fixer.replaceTextRange( - [importSpecifiers[0].range[0], importSpecifiers[importSpecifiers.length - 1].range[1]], - importSpecifiers - // Clone the importSpecifiers array to avoid mutating it - .slice() + // Build a string out of the sorted list of import specifiers and the text between the originals + .reduce((sourceText, specifier, index) => { + const textAfterSpecifier = index === importSpecifiers.length - 1 + ? '' + : sourceCode.getText().slice(importSpecifiers[index].range[1], importSpecifiers[index + 1].range[0]); - // Sort the array into the desired order - .sort((specifierA, specifierB) => { - const aName = getSortableName(specifierA); - const bName = getSortableName(specifierB); - return aName > bName ? 1 : -1; - }) + return sourceText + sourceCode.getText(specifier) + textAfterSpecifier; + }, '') + ); + } + }); + } else if (lastImportDeclaration) { + let currentLocalMemberName = getFirstLocalMemberName(statement); + let previousLocalMemberName = getFirstLocalMemberName(lastImportDeclaration); + if (previousLocalMemberName && + currentLocalMemberName && + currentLocalMemberName < previousLocalMemberName + ) { + context.report({ + node, + message: 'Imports should be sorted alphabetically.', + fix(fixer) { + let allImports = []; + for (let statement of node.body) { + if (statement.type === 'ImportDeclaration') { + allImports.push(statement); + } else { + // Do not replace if there are other statements between imports. + break; + } + } - // Build a string out of the sorted list of import specifiers and the text between the originals - .reduce((sourceText, specifier, index) => { - const textAfterSpecifier = index === importSpecifiers.length - 1 - ? '' - : sourceCode.getText().slice(importSpecifiers[index].range[1], importSpecifiers[index + 1].range[0]); + let sortedImports = allImports.slice().sort((a, b) => { + let aName = getFirstLocalMemberName(a); + let bName = getFirstLocalMemberName(b); + if (aName === bName) { + return 0; + } + return aName < bName ? -1 : 1; + }); - return sourceText + sourceCode.getText(specifier) + textAfterSpecifier; - }, '') - ); + return fixer.replaceTextRange( + [allImports[0].range[0], allImports[allImports.length - 1].range[1]], + sortedImports.reduce((sourceText, statement, index) => { + const textAfterStatement = index === allImports.length - 1 + ? '' + : sourceCode.getText().slice(allImports[index].range[1], allImports[index + 1].range[0]); + return sourceText + sourceCode.getText(statement) + textAfterStatement; + }, '') + ); + } + }); + } } - }); - } - previousDeclaration = node; + lastImportDeclaration = statement; + } + }); } }; } diff --git a/packages/dev/eslint-plugin-rsp-rules/test/sort-imports.test.js b/packages/dev/eslint-plugin-rsp-rules/test/sort-imports.test.js index 5d60fa9a713..a56c1d99a88 100644 --- a/packages/dev/eslint-plugin-rsp-rules/test/sort-imports.test.js +++ b/packages/dev/eslint-plugin-rsp-rules/test/sort-imports.test.js @@ -42,19 +42,19 @@ import {B} from 'bar'; code: "import {B, A} from 'foo';", output: "import {A, B} from 'foo';", errors: 1 - } + }, // we don't support this case yet -// { -// code: ` -// import {B} from 'bar'; -// import {A} from 'foo'; -// `, -// output: ` -// import {A} from 'foo'; -// import {B} from 'bar'; -// `, -// errors: 1 -// } + { + code: ` +import {B} from 'bar'; +import {A} from 'foo'; +`, + output: ` +import {A} from 'foo'; +import {B} from 'bar'; +`, + errors: 1 + } ] } );