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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Object-Import Errors #1823

Merged
merged 1 commit into from Jun 15, 2020
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
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