diff --git a/.changeset/chilled-pandas-tickle.md b/.changeset/chilled-pandas-tickle.md new file mode 100644 index 000000000..a29d3076c --- /dev/null +++ b/.changeset/chilled-pandas-tickle.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-import-x": patch +--- + +Fix `newline-after-import`'s `considerComments` options when linting `require`, backports https://github.com/import-js/eslint-plugin-import/pull/2952 diff --git a/.changeset/weak-shirts-smell.md b/.changeset/weak-shirts-smell.md new file mode 100644 index 000000000..398c375f1 --- /dev/null +++ b/.changeset/weak-shirts-smell.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-import-x": patch +--- + +Fix `no-duplicates` for TypeScript, backports https://github.com/import-js/eslint-plugin-import/pull/3033 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1464b8206..11bd9b54f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,10 @@ on: - push - pull_request +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: ci: name: Lint and Test with Node.js ${{ matrix.node }} and ESLint ${{ matrix.eslint }} on ${{ matrix.os }} diff --git a/docs/rules/order.md b/docs/rules/order.md index 97aa3737c..c08d60631 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -77,6 +77,25 @@ import foo from './foo' var path = require('path') ``` +## Limitations of `--fix` + +Unbound imports are assumed to have side effects, and will never be moved/reordered. This can cause other imports to get "stuck" around them, and the fix to fail. + +```javascript +import b from 'b' +import 'format.css' // This will prevent --fix from working. +import a from 'a' +``` + +As a workaround, move unbound imports to be entirely above or below bound ones. + +```javascript +import 'format1.css' // OK +import b from 'b' +import a from 'a' +import 'format2.css' // OK +``` + ## Options This rule supports the following options: @@ -180,7 +199,8 @@ Example: ### `pathGroupsExcludedImportTypes: [array]` This defines import types that are not handled by configured pathGroups. -This is mostly needed when you want to handle path groups that look like external imports. + +If you have added path groups with patterns that look like `"builtin"` or `"external"` imports, you have to remove this group (`"builtin"` and/or `"external"`) from the default exclusion list (e.g., `["builtin", "external", "object"]`, etc) to sort these path groups correctly. Example: @@ -202,29 +222,7 @@ Example: } ``` -You can also use `patterns`(e.g., `react`, `react-router-dom`, etc). - -Example: - -```json -{ - "import-x/order": [ - "error", - { - "pathGroups": [ - { - "pattern": "react", - "group": "builtin", - "position": "before" - } - ], - "pathGroupsExcludedImportTypes": ["react"] - } - ] -} -``` - -The default value is `["builtin", "external", "object"]`. +[Import Type](https://github.com/un-ts/eslint-plugin-import-x/blob/ea7c13eb9b18357432e484b25dfa4451eca69c5b/src/utils/import-type.ts#L145) is resolved as a fixed string in predefined set, it can't be a `patterns` (e.g., `react`, `react-router-dom`, etc). ### `newlines-between: [ignore|always|always-and-inside-groups|never]` diff --git a/src/rules/newline-after-import.ts b/src/rules/newline-after-import.ts index ecb7b0f70..197484bce 100644 --- a/src/rules/newline-after-import.ts +++ b/src/rules/newline-after-import.ts @@ -177,6 +177,7 @@ export = createRule<[Options?], MessageId>({ function commentAfterImport( node: TSESTree.Node, nextComment: TSESTree.Comment, + type: 'import' | 'require', ) { const lineDifference = getLineDifference(node, nextComment) const EXPECTED_LINE_DIFFERENCE = options.count + 1 @@ -197,7 +198,7 @@ export = createRule<[Options?], MessageId>({ data: { count: options.count, lineSuffix: options.count > 1 ? 's' : '', - type: 'import', + type, }, fix: options.exactCount && EXPECTED_LINE_DIFFERENCE < lineDifference @@ -253,7 +254,7 @@ export = createRule<[Options?], MessageId>({ } if (nextComment) { - commentAfterImport(node, nextComment) + commentAfterImport(node, nextComment, 'import') } else if ( nextNode && nextNode.type !== 'ImportDeclaration' && @@ -301,7 +302,33 @@ export = createRule<[Options?], MessageId>({ (!nextRequireCall || !containsNodeOrEqual(nextStatement, nextRequireCall)) ) { - checkForNewLine(statementWithRequireCall, nextStatement, 'require') + let nextComment + if ( + 'comments' in statementWithRequireCall.parent && + statementWithRequireCall.parent.comments !== undefined && + options.considerComments + ) { + const endLine = node.loc.end.line + nextComment = statementWithRequireCall.parent.comments.find( + o => + o.loc.start.line >= endLine && + o.loc.start.line <= endLine + options.count + 1, + ) + } + + if (nextComment && nextComment !== undefined) { + commentAfterImport( + statementWithRequireCall, + nextComment, + 'require', + ) + } else { + checkForNewLine( + statementWithRequireCall, + nextStatement, + 'require', + ) + } } } }, diff --git a/src/rules/no-duplicates.ts b/src/rules/no-duplicates.ts index 5524bfbe1..b706541aa 100644 --- a/src/rules/no-duplicates.ts +++ b/src/rules/no-duplicates.ts @@ -200,6 +200,32 @@ function getFix( const fixes = [] + if (shouldAddSpecifiers && preferInline && first.importKind === 'type') { + // `import type {a} from './foo'` → `import {type a} from './foo'` + const typeIdentifierToken = tokens.find( + token => token.type === 'Identifier' && token.value === 'type', + ) + if (typeIdentifierToken) { + fixes.push( + fixer.removeRange([ + typeIdentifierToken.range[0], + typeIdentifierToken.range[1] + 1, + ]), + ) + } + + for (const identifier of tokens.filter(token => + firstExistingIdentifiers.has(token.value), + )) { + fixes.push( + fixer.replaceTextRange( + [identifier.range[0], identifier.range[1]], + `type ${identifier.value}`, + ), + ) + } + } + if (openBrace == null && shouldAddSpecifiers && shouldAddDefault()) { // `import './foo'` → `import def, {...} from './foo'` fixes.push( diff --git a/test/rules/newline-after-import.spec.ts b/test/rules/newline-after-import.spec.ts index 81efd8d97..a56f27a13 100644 --- a/test/rules/newline-after-import.spec.ts +++ b/test/rules/newline-after-import.spec.ts @@ -260,7 +260,7 @@ ruleTester.run('newline-after-import', rule, { options: [{ count: 4, exactCount: true }], }, { - code: `var foo = require('foo-module');\n\n\n\n// Some random comment\nvar foo = 'bar';`, + code: `var foo = require('foo-module');\n\n\n\n\n// Some random comment\nvar foo = 'bar';`, languageOptions: { parserOptions: { ecmaVersion: 2015, sourceType: 'module' }, }, @@ -479,6 +479,21 @@ ruleTester.run('newline-after-import', rule, { parserOptions: { ecmaVersion: 2015, sourceType: 'module' }, }, }, + { + code: `var foo = require('foo-module');\n\n\n// Some random comment\nvar foo = 'bar';`, + options: [{ count: 2, considerComments: true }], + }, + { + code: `var foo = require('foo-module');\n\n\n/**\n * Test comment\n */\nvar foo = 'bar';`, + options: [{ count: 2, considerComments: true }], + }, + { + code: `const foo = require('foo');\n\n\n// some random comment\nconst bar = function() {};`, + options: [{ count: 2, exactCount: true, considerComments: true }], + languageOptions: { + parserOptions: { ecmaVersion: 2015 }, + }, + }, ], invalid: [ @@ -1054,17 +1069,39 @@ ruleTester.run('newline-after-import', rule, { }, }, { - code: `const foo = require('foo');\n\n\n// some random comment\nconst bar = function() {};`, - output: null, - options: [{ count: 2, exactCount: true, considerComments: true }], + code: `var foo = require('foo-module');\nvar foo = require('foo-module');\n\n// Some random comment\nvar foo = 'bar';`, + output: `var foo = require('foo-module');\nvar foo = require('foo-module');\n\n\n// Some random comment\nvar foo = 'bar';`, + errors: [ + { + line: 2, + column: 1, + messageId: `newline`, + }, + ], + languageOptions: { + parserOptions: { + ecmaVersion: 2015, + sourceType: 'module', + }, + }, + options: [{ considerComments: true, count: 2 }], + }, + { + code: `var foo = require('foo-module');\n\n/**\n * Test comment\n */\nvar foo = 'bar';`, + output: `var foo = require('foo-module');\n\n\n/**\n * Test comment\n */\nvar foo = 'bar';`, errors: [ { line: 1, column: 1, - ...getRequireError(2), + messageId: `newline`, }, ], - languageOptions: { parserOptions: { ecmaVersion: 2015 } }, + languageOptions: { + parserOptions: { + ecmaVersion: 2015, + }, + }, + options: [{ considerComments: true, count: 2 }], }, ], }) diff --git a/test/rules/no-duplicates.spec.ts b/test/rules/no-duplicates.spec.ts index bdd502333..3be9b76dd 100644 --- a/test/rules/no-duplicates.spec.ts +++ b/test/rules/no-duplicates.spec.ts @@ -757,6 +757,24 @@ describe('TypeScript', () => { }, ], }), + test({ + code: "import type {x} from 'foo'; import {type y} from 'foo'", + ...parserConfig, + options: [{ 'prefer-inline': true }], + output: `import {type x,type y} from 'foo'; `, + errors: [ + { + line: 1, + column: 22, + message: "'foo' imported multiple times.", + }, + { + line: 1, + column: 50, + message: "'foo' imported multiple times.", + }, + ], + }), test({ code: "import {type x} from 'foo'; import type {y} from 'foo'", ...parserConfig,