Repository: un-ts/eslint-plugin-import-x
Pull request #147 (2024-09-04 15:34:53)

fix(`no-unused-modules`): default `src` to `process.cwd()` again
Fix initialization of option `src` to again default to `process.cwd()` per [the documentation](https://github.com/un-ts/eslint-plugin-import-x/blob/master/docs/rules/no-unused-modules.md#options).

de896f46b1e3d3bcc51015d2e0269e6ae80dc87d (released in v0.3.1) added an incorrect [default parameter value of `[]` for `src`](https://github.com/un-ts/eslint-plugin-import-x/blob/master/src/rules/no-unused-modules.ts#L309). This introduced a regression that breaks discovery of source files unless option `src` is explicitly set by the consumer, resulting in false positives for `unusedExports`.

changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 59a39a005a8461dedec20c20a25dd529cefccf2b

**The changes in this PR will be included in the next version bump.**

<details><summary>This PR includes changesets to release 1 package</summary>

  | Name                   | Type  |
| ---------------------- | ----- |
| eslint-plugin-import-x | Patch |

</details>

Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/nchevsky/eslint-plugin-import-x/new/rules/no-unused-modules?filename=.changeset/famous-stingrays-shave.md&value=---%0A%22eslint-plugin-import-x%22%3A%20patch%0A---%0A%0Afix(%60no-unused-modules%60)%3A%20default%20%60src%60%20to%20%60%5Bprocess.cwd()%5D%60%0A)


codesandbox-ci[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/un-ts/eslint-plugin-import-x/pr/147/builds/538371) or the icon next to each commit SHA.

Patch:
diff --git a/.changeset/chilled-pandas-tickle.md b/.changeset/chilled-pandas-tickle.md
new file mode 100644
index 00000000..a29d3076
--- /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/flat-doors-chew.md b/.changeset/flat-doors-chew.md
new file mode 100644
index 00000000..a006e392
--- /dev/null
+++ b/.changeset/flat-doors-chew.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Fix regression in rule `no-unused-modules` which would incorrectly initialize option `src` to `[]` instead of `[process.cwd()]`, breaking file discovery.
diff --git a/.changeset/weak-shirts-smell.md b/.changeset/weak-shirts-smell.md
new file mode 100644
index 00000000..398c375f
--- /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 1464b820..11bd9b54 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 97aa3737..c08d6063 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 ecb7b0f7..197484bc 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 5524bfbe..b706541a 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/src/rules/no-unused-modules.ts b/src/rules/no-unused-modules.ts
index 7c759727..6a2f4b81 100644
--- a/src/rules/no-unused-modules.ts
+++ b/src/rules/no-unused-modules.ts
@@ -291,13 +291,6 @@ const determineUsage = () => {
   }
 }
 
-const getSrc = (src?: string[]) => {
-  if (src) {
-    return src
-  }
-  return [process.cwd()]
-}
-
 /**
  * prepare the lists of existing imports and exports - should only be executed once at
  * the start of a new eslint run
@@ -306,7 +299,7 @@ let srcFiles: Set<string>
 let lastPrepareKey: string
 
 const doPreparation = (
-  src: string[] = [],
+  src: string[],
   ignoreExports: string[],
   context: RuleContext,
 ) => {
@@ -324,7 +317,7 @@ const doPreparation = (
   ignoredFiles.clear()
   filesOutsideSrc.clear()
 
-  srcFiles = resolveFiles(getSrc(src), ignoreExports, context)
+  srcFiles = resolveFiles(src, ignoreExports, context)
   prepareImportsAndExports(srcFiles, context)
   determineUsage()
   lastPrepareKey = prepareKey
@@ -486,7 +479,7 @@ export = createRule<Options[], MessageId>({
   defaultOptions: [],
   create(context) {
     const {
-      src,
+      src = [process.cwd()],
       ignoreExports = [],
       missingExports,
       unusedExports,
@@ -559,7 +552,7 @@ export = createRule<Options[], MessageId>({
 
       // make sure file to be linted is included in source files
       if (!srcFiles.has(filename)) {
-        srcFiles = resolveFiles(getSrc(src), ignoreExports, context)
+        srcFiles = resolveFiles(src, ignoreExports, context)
         if (!srcFiles.has(filename)) {
           filesOutsideSrc.add(filename)
           return


Test patch:
diff --git a/test/rules/newline-after-import.spec.ts b/test/rules/newline-after-import.spec.ts
index 81efd8d9..a56f27a1 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 bdd50233..3be9b76d 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,