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: Autofix not working if alphabetical order is used #2490

Merged
merged 1 commit into from Jul 7, 2022
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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Expand Up @@ -15,6 +15,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- [`order`]: move nested imports closer to main import entry ([#2396], thanks [@pri1311])
- [`no-restricted-paths`]: fix an error message ([#2466], thanks [@AdriAt360])
- [`no-restricted-paths`]: use `Minimatch.match` instead of `minimatch` to comply with Windows Native paths ([#2466], thanks [@AdriAt360])
- [`order`]: require with member expression could not be fixed if alphabetize.order was used ([#2490], thanks [@msvab])

### Changed
- [Tests] `named`: Run all TypeScript test ([#2427], thanks [@ProdigySim])
Expand Down Expand Up @@ -994,6 +995,7 @@ for info on changes for earlier releases.

[`memo-parser`]: ./memo-parser/README.md

[#2490]: https://github.com/import-js/eslint-plugin-import/pull/2490
[#2466]: https://github.com/import-js/eslint-plugin-import/pull/2466
[#2440]: https://github.com/import-js/eslint-plugin-import/pull/2440
[#2427]: https://github.com/import-js/eslint-plugin-import/pull/2427
Expand Down Expand Up @@ -1638,15 +1640,16 @@ for info on changes for earlier releases.
[@MikeyBeLike]: https://github.com/MikeyBeLike
[@mplewis]: https://github.com/mplewis
[@mrmckeb]: https://github.com/mrmckeb
[@msvab]: https://github.com/msvab
[@mx-bernhard]: https://github.com/mx-bernhard
[@nickofthyme]: https://github.com/nickofthyme
[@nicolashenry]: https://github.com/nicolashenry
[@noelebrun]: https://github.com/noelebrun
[@ntdb]: https://github.com/ntdb
[@nwalters512]: https://github.com/nwalters512
[@ombene]: https://github.com/ombene
[@OutdatedVersion]: https://github.com/OutdatedVersion
[@ota-meshi]: https://github.com/ota-meshi
[@OutdatedVersion]: https://github.com/OutdatedVersion
[@panrafal]: https://github.com/panrafal
[@paztis]: https://github.com/paztis
[@pcorpet]: https://github.com/pcorpet
Expand Down
29 changes: 20 additions & 9 deletions src/rules/order.js
Expand Up @@ -129,24 +129,35 @@ function findStartOfLineWithComments(sourceCode, node) {
return result;
}

function isPlainRequireModule(node) {
function isRequireExpression(expr) {
return expr != null &&
expr.type === 'CallExpression' &&
expr.callee != null &&
expr.callee.name === 'require' &&
expr.arguments != null &&
expr.arguments.length === 1 &&
expr.arguments[0].type === 'Literal';
}

function isSupportedRequireModule(node) {
if (node.type !== 'VariableDeclaration') {
return false;
}
if (node.declarations.length !== 1) {
return false;
}
const decl = node.declarations[0];
const result = decl.id &&
const isPlainRequire = decl.id &&
(decl.id.type === 'Identifier' || decl.id.type === 'ObjectPattern') &&
isRequireExpression(decl.init);
const isRequireWithMemberExpression = decl.id &&
(decl.id.type === 'Identifier' || decl.id.type === 'ObjectPattern') &&
decl.init != null &&
decl.init.type === 'CallExpression' &&
decl.init.callee != null &&
decl.init.callee.name === 'require' &&
decl.init.arguments != null &&
decl.init.arguments.length === 1 &&
decl.init.arguments[0].type === 'Literal';
return result;
decl.init.callee.type === 'MemberExpression' &&
isRequireExpression(decl.init.callee.object);
return isPlainRequire || isRequireWithMemberExpression;
}

function isPlainImportModule(node) {
Expand All @@ -158,7 +169,7 @@ function isPlainImportEquals(node) {
}

function canCrossNodeWhileReorder(node) {
return isPlainRequireModule(node) || isPlainImportModule(node) || isPlainImportEquals(node);
return isSupportedRequireModule(node) || isPlainImportModule(node) || isPlainImportEquals(node);
}

function canReorderItems(firstNode, secondNode) {
Expand Down Expand Up @@ -276,7 +287,7 @@ function getSorter(ascending) {
result = a < b ? -1 : 1;
}
}

return result * multiplier;
};
}
Expand Down
27 changes: 22 additions & 5 deletions tests/src/rules/order.js
Expand Up @@ -896,13 +896,13 @@ ruleTester.run('order', rule, {
import express from 'express';

import service from '@/api/service';

import fooParent from '../foo';

import fooSibling from './foo';

import index from './';

import internalDoesNotExistSoIsUnknown from '@/does-not-exist';
`,
options: [
Expand Down Expand Up @@ -2289,7 +2289,7 @@ ruleTester.run('order', rule, {
import b from "foo-bar";
`,
errors: [{
message: '`foo-bar` import should occur after import of `foo/barfoo`',
message: '`foo-bar` import should occur after import of `foo/barfoo`',
}],
}),
// Option alphabetize {order: 'asc': caseInsensitive: true}
Expand Down Expand Up @@ -2336,6 +2336,23 @@ ruleTester.run('order', rule, {
message: '`foo` import should occur before import of `Bar`',
}],
}),
// Option alphabetize {order: 'asc'} and require with member expression
test({
code: `
const b = require('./b').get();
const a = require('./a');
`,
output: `
const a = require('./a');
const b = require('./b').get();
`,
options: [{
alphabetize: { order: 'asc' },
}],
errors: [{
message: '`./a` import should occur before import of `./b`',
}],
}),
// Alphabetize with parent paths
test({
code: `
Expand Down