Skip to content

Commit

Permalink
[Fix] order: Fix import ordering in TypeScript module declarations
Browse files Browse the repository at this point in the history
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
  • Loading branch information
remcohaszing authored and ljharb committed Sep 12, 2021
1 parent 4ed7867 commit 47ea669
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 16 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
47 changes: 31 additions & 16 deletions src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')()`
Expand All @@ -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'];
Expand Down Expand Up @@ -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) {
Expand All @@ -621,7 +630,7 @@ module.exports = {
type: 'import',
},
ranks,
imported,
getBlockImports(node.parent),
pathGroupsExcludedImportTypes
);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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();
},
};
},
Expand Down
50 changes: 50 additions & 0 deletions tests/src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'}
Expand Down Expand Up @@ -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' },
},
],
}),
],
});
});
Expand Down

0 comments on commit 47ea669

Please sign in to comment.