Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] order: Fix import ordering in TypeScript module declarations #2226

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
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
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
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