From 47ea669d2c68b2e6e67ed20d93b71d42147dbccd Mon Sep 17 00:00:00 2001 From: Remco Haszing Date: Sun, 12 Sep 2021 16:08:21 +0200 Subject: [PATCH] [Fix] `order`: Fix import ordering in TypeScript module declarations Without this, `import/order` checks if all imports in a file are sorted. The autofix would then move all imports to the type of the file, breaking TypeScript module declarations. Closes #2217 --- CHANGELOG.md | 3 +++ src/rules/order.js | 47 ++++++++++++++++++++++++------------- tests/src/rules/order.js | 50 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbb9ecb82..803333d08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Fixed - [`no-unresolved`]: ignore type-only imports ([#2220], thanks [@jablko]) +- [`order`]: fix sorting imports inside TypeScript module declarations ([#2226], thanks [@remcohaszing]) ### Changed - [Refactor] switch to an internal replacement for `pkg-up` and `read-pkg-up` ([#2047], thanks [@mgwalker]) @@ -913,6 +914,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#2226]: https://github.com/import-js/eslint-plugin-import/pull/2226 [#2220]: https://github.com/import-js/eslint-plugin-import/pull/2220 [#2219]: https://github.com/import-js/eslint-plugin-import/pull/2219 [#2212]: https://github.com/import-js/eslint-plugin-import/pull/2212 @@ -1520,6 +1522,7 @@ for info on changes for earlier releases. [@ramasilveyra]: https://github.com/ramasilveyra [@randallreedjr]: https://github.com/randallreedjr [@redbugz]: https://github.com/redbugz +[@remcohaszing]: https://github.com/remcohaszing [@rfermann]: https://github.com/rfermann [@rhettlivingston]: https://github.com/rhettlivingston [@rhys-vdw]: https://github.com/rhys-vdw diff --git a/src/rules/order.js b/src/rules/order.js index 2f4ef08b7..194a3fd53 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -336,7 +336,7 @@ function registerNode(context, importEntry, ranks, imported, excludedImportTypes } } -function isModuleLevelRequire(node) { +function getRequireBlock(node) { let n = node; // Handle cases like `const baz = require('foo').bar.baz` // and `const foo = require('foo')()` @@ -346,11 +346,13 @@ function isModuleLevelRequire(node) { ) { n = n.parent; } - return ( + if ( n.parent.type === 'VariableDeclarator' && n.parent.parent.type === 'VariableDeclaration' && n.parent.parent.parent.type === 'Program' - ); + ) { + return n.parent.parent.parent; + } } const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index', 'object', 'type']; @@ -605,7 +607,14 @@ module.exports = { }, }; } - let imported = []; + const importMap = new Map(); + + function getBlockImports(node) { + if (!importMap.has(node)) { + importMap.set(node, []); + } + return importMap.get(node); + } return { ImportDeclaration: function handleImports(node) { @@ -621,7 +630,7 @@ module.exports = { type: 'import', }, ranks, - imported, + getBlockImports(node.parent), pathGroupsExcludedImportTypes ); } @@ -652,12 +661,16 @@ module.exports = { type, }, ranks, - imported, + getBlockImports(node.parent), pathGroupsExcludedImportTypes ); }, CallExpression: function handleRequires(node) { - if (!isStaticRequire(node) || !isModuleLevelRequire(node)) { + if (!isStaticRequire(node)) { + return; + } + const block = getRequireBlock(node); + if (!block) { return; } const name = node.arguments[0].value; @@ -670,22 +683,24 @@ module.exports = { type: 'require', }, ranks, - imported, + getBlockImports(block), pathGroupsExcludedImportTypes ); }, 'Program:exit': function reportAndReset() { - if (newlinesBetweenImports !== 'ignore') { - makeNewlinesBetweenReport(context, imported, newlinesBetweenImports); - } + importMap.forEach((imported) => { + if (newlinesBetweenImports !== 'ignore') { + makeNewlinesBetweenReport(context, imported, newlinesBetweenImports); + } - if (alphabetize.order !== 'ignore') { - mutateRanksToAlphabetize(imported, alphabetize); - } + if (alphabetize.order !== 'ignore') { + mutateRanksToAlphabetize(imported, alphabetize); + } - makeOutOfOrderReport(context, imported); + makeOutOfOrderReport(context, imported); + }); - imported = []; + importMap.clear(); }, }; }, diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 61bf1bb70..146306259 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -2462,6 +2462,24 @@ context('TypeScript', function () { }, ], }), + // Imports inside module declaration + test({ + code: ` + import type { CopyOptions } from 'fs'; + import type { ParsedPath } from 'path'; + + declare module 'my-module' { + import type { CopyOptions } from 'fs'; + import type { ParsedPath } from 'path'; + } + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + }, + ], + }), ], invalid: [ // Option alphabetize: {order: 'asc'} @@ -2655,6 +2673,38 @@ context('TypeScript', function () { }], options: [{ warnOnUnassignedImports: true }], }), + // Imports inside module declaration + test({ + code: ` + import type { ParsedPath } from 'path'; + import type { CopyOptions } from 'fs'; + + declare module 'my-module' { + import type { ParsedPath } from 'path'; + import type { CopyOptions } from 'fs'; + } + `, + output: ` + import type { CopyOptions } from 'fs'; + import type { ParsedPath } from 'path'; + + declare module 'my-module' { + import type { CopyOptions } from 'fs'; + import type { ParsedPath } from 'path'; + } + `, + errors: [{ + message: '`fs` import should occur before import of `path`', + },{ + message: '`fs` import should occur before import of `path`', + }], + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + }, + ], + }), ], }); });