Skip to content

Commit

Permalink
[Fix] order/TypeScript: properly support import = object expressions
Browse files Browse the repository at this point in the history
Just like ordinary `import x =` expressions, `export import x =` expressions can come with a number of different module-references.
Either a require-expression such as `export import fs = require("fs")`, a literal such as `export import Console = console;` or an object-path `export import log = console.log`.

This means, that the `isExport` property merely says whether the `TSImportEqualsDeclaration` has a leading `export`, but not what the `moduleReference` looks like.

----

This arguably is a semver-minor, but since it should have been included in #1785, I'm calling this a bugfix.

Fixes #1821. Fixes #1808.
  • Loading branch information
manuth authored and ljharb committed Jun 12, 2020
1 parent b22a183 commit f5d95e8
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 19 deletions.
1 change: 1 addition & 0 deletions .babelrc
@@ -1,6 +1,7 @@
{
"presets": [ "es2015-argon" ],
"sourceMaps": "inline",
"retainLines": true,
"env": {
"test": {
"plugins": [
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-unused-modules`]: consider exported TypeScript interfaces, types and enums ([#1819], thanks [@nicolashenry])

### Fixed
- [`order`]/TypeScript: properly support `import = object` expressions ([#1823], thanks [@manuth])
- [`no-extraneous-dependencies`]/TypeScript: do not error when importing type from dev dependencies ([#1820], thanks [@fernandopasik])
- [`default`]: avoid crash with `export =` ([#1822], thanks [@AndrewLeedham])

Expand Down Expand Up @@ -712,6 +713,7 @@ for info on changes for earlier releases.
[`memo-parser`]: ./memo-parser/README.md

[#1824]: https://github.com/benmosher/eslint-plugin-import/pull/1824
[#1823]: https://github.com/benmosher/eslint-plugin-import/pull/1823
[#1822]: https://github.com/benmosher/eslint-plugin-import/pull/1822
[#1820]: https://github.com/benmosher/eslint-plugin-import/pull/1820
[#1819]: https://github.com/benmosher/eslint-plugin-import/pull/1819
Expand Down
9 changes: 7 additions & 2 deletions docs/rules/order.md
Expand Up @@ -22,6 +22,8 @@ import bar from './bar';
import baz from './bar/baz';
// 6. "index" of the current directory
import main from './';
// 7. "object"-imports (only available in TypeScript)
import log = console.log;
```

Unassigned imports are ignored, as the order they are imported in may be important.
Expand Down Expand Up @@ -77,12 +79,15 @@ This rule supports the following options:

### `groups: [array]`:

How groups are defined, and the order to respect. `groups` must be an array of `string` or [`string`]. The only allowed `string`s are: `"builtin"`, `"external"`, `"internal"`, `"unknown"`, `"parent"`, `"sibling"`, `"index"`. The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example:
How groups are defined, and the order to respect. `groups` must be an array of `string` or [`string`]. The only allowed `string`s are:
`"builtin"`, `"external"`, `"internal"`, `"unknown"`, `"parent"`, `"sibling"`, `"index"`, `"object"`.
The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example:
```js
[
'builtin', // Built-in types are first
['sibling', 'parent'], // Then sibling and parent types. They can be mingled together
'index', // Then the index file
'object',
// Then the rest: internal and external type
]
```
Expand All @@ -91,7 +96,7 @@ The default value is `["builtin", "external", "parent", "sibling", "index"]`.
You can set the options like this:

```js
"import/order": ["error", {"groups": ["index", "sibling", "parent", "internal", "external", "builtin"]}]
"import/order": ["error", {"groups": ["index", "sibling", "parent", "internal", "external", "builtin", "object"]}]
```

### `pathGroups: [array of objects]`:
Expand Down
32 changes: 19 additions & 13 deletions src/rules/order.js
Expand Up @@ -248,14 +248,14 @@ function makeOutOfOrderReport(context, imported) {
}

function getSorter(ascending) {
let multiplier = (ascending ? 1 : -1)
const multiplier = ascending ? 1 : -1

return function importsSorter(importA, importB) {
let result

if ((importA < importB) || importB === null) {
if (importA < importB) {
result = -1
} else if ((importA > importB) || importA === null) {
} else if (importA > importB) {
result = 1
} else {
result = 0
Expand Down Expand Up @@ -310,24 +310,29 @@ function computePathRank(ranks, pathGroups, path, maxPosition) {
}
}

function computeRank(context, ranks, name, type, excludedImportTypes) {
const impType = importType(name, context)
function computeRank(context, node, ranks, name, type, excludedImportTypes) {
let impType
if (type === 'import:object') {
impType = 'object'
} else {
impType = importType(name, context)
}
let rank
if (!excludedImportTypes.has(impType)) {
rank = computePathRank(ranks.groups, ranks.pathGroups, name, ranks.maxPosition)
}
if (typeof rank === 'undefined') {
rank = ranks.groups[impType]
}
if (type !== 'import') {
if (type !== 'import' && !type.startsWith('import:')) {
rank += 100
}

return rank
}

function registerNode(context, node, name, type, ranks, imported, excludedImportTypes) {
const rank = computeRank(context, ranks, name, type, excludedImportTypes)
const rank = computeRank(context, node, ranks, name, type, excludedImportTypes)
if (rank !== -1) {
imported.push({name, rank, node})
}
Expand All @@ -338,7 +343,7 @@ function isInVariableDeclarator(node) {
(node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent))
}

const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index']
const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index', 'object']

// Creates an object with type-rank pairs.
// Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
Expand Down Expand Up @@ -563,7 +568,7 @@ module.exports = {
create: function importOrderRule (context) {
const options = context.options[0] || {}
const newlinesBetweenImports = options['newlines-between'] || 'ignore'
const pathGroupsExcludedImportTypes = new Set(options['pathGroupsExcludedImportTypes'] || ['builtin', 'external'])
const pathGroupsExcludedImportTypes = new Set(options['pathGroupsExcludedImportTypes'] || ['builtin', 'external', 'object'])
const alphabetize = getAlphabetizeConfig(options)
let ranks

Expand Down Expand Up @@ -609,18 +614,19 @@ module.exports = {
},
TSImportEqualsDeclaration: function handleImports(node) {
let name
let type
if (node.moduleReference.type === 'TSExternalModuleReference') {
name = node.moduleReference.expression.value
} else if (node.isExport) {
name = node.moduleReference.name
type = 'import'
} else {
name = null
name = context.getSourceCode().getText(node.moduleReference)
type = 'import:object'
}
registerNode(
context,
node,
name,
'import',
type,
ranks,
imported,
pathGroupsExcludedImportTypes
Expand Down
4 changes: 2 additions & 2 deletions tests/src/cli.js
Expand Up @@ -58,14 +58,14 @@ describe('CLI regression tests', function () {
nodeType: results.results[0].messages[0].nodeType, // we don't care about this one
ruleId: 'json/*',
severity: 2,
source: '\n',
source: results.results[0].messages[0].source, // NewLine-characters might differ depending on git-settings
},
],
errorCount: 1,
warningCount: 0,
fixableErrorCount: 0,
fixableWarningCount: 0,
source: ',\n',
source: results.results[0].source, // NewLine-characters might differ depending on git-settings
},
],
errorCount: 1,
Expand Down
75 changes: 73 additions & 2 deletions tests/src/rules/order.js
Expand Up @@ -711,6 +711,49 @@ ruleTester.run('order', rule, {
},
],
}),
...flatMap(getTSParsers, parser => [
// Order of the `import ... = require(...)` syntax
test({
code: `
import blah = require('./blah');
import { hello } from './hello';`,
parser,
options: [
{
alphabetize: {
order: 'asc',
},
},
],
}),
// Order of object-imports
test({
code: `
import blah = require('./blah');
import log = console.log;`,
parser,
options: [
{
alphabetize: {
order: 'asc',
},
},
],
}),
test({
code: `
import debug = console.debug;
import log = console.log;`,
parser,
options: [
{
alphabetize: {
order: 'asc',
},
},
],
}),
]),
],
invalid: [
// builtin before external module (require)
Expand Down Expand Up @@ -1167,6 +1210,7 @@ ruleTester.run('order', rule, {
}],
}),
...flatMap(getTSParsers(), parser => [
// Order of the `import ... = require(...)` syntax
test({
code: `
var fs = require('fs');
Expand All @@ -1183,7 +1227,7 @@ ruleTester.run('order', rule, {
message: '`fs` import should occur after import of `../foo/bar`',
}],
}),
{
test({
code: `
var async = require('async');
var fs = require('fs');
Expand All @@ -1196,7 +1240,7 @@ ruleTester.run('order', rule, {
errors: [{
message: '`fs` import should occur before import of `async`',
}],
},
}),
test({
code: `
import sync = require('sync');
Expand All @@ -1219,6 +1263,33 @@ ruleTester.run('order', rule, {
message: '`async` import should occur before import of `sync`',
}],
}),
// Order of object-imports
test({
code: `
import log = console.log;
import blah = require('./blah');`,
parser,
errors: [{
message: '`./blah` import should occur before import of `console.log`',
}],
}),
// Alphabetization of object-imports
test({
code: `
import log = console.log;
import debug = console.debug;`,
parser,
errors: [{
message: '`console.debug` import should occur before import of `console.log`',
}],
options: [
{
alphabetize: {
order: 'asc',
},
},
],
}),
]),
// Default order using import with custom import alias
test({
Expand Down

0 comments on commit f5d95e8

Please sign in to comment.