refactor: backport a few upstream changes (#171)

Author: SukkaW

Diff for: .changeset/fast-carpets-teach.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Perf: avoid regexp during parser choosing

Diff for: .changeset/hot-jokes-grab.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Add extra guard for rule `no-named-as-default`. A few guards are borrowed from https://github.com/import-js/eslint-plugin-import/pull/3032, but we don't sync the rest of changes from upstream since we have already implemented a way more performant check.

Diff for: .changeset/old-years-rest.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+More test cases for `no-named-export` and `no-defualt-export` rule specifically with non-module `sourceType`

Diff for: .changeset/orange-dryers-mix.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Fix `export` when there is only one `TSDeclareFunction` (https://github.com/import-js/eslint-plugin-import/pull/3065)

Diff for: .changeset/shaggy-mayflies-applaud.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Prevent `ExportMap`'s cache is being tainted by incompatible parser (e.g. old `babel-eslint`). The cache is now skipped w/ incompatible parsers, which might introduce performance impacts only for those who are using incompatible parsers. (https://github.com/import-js/eslint-plugin-import/pull/3062)

Diff for: .changeset/spicy-mangos-pull.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Docs: fix a few typos here and there

Diff for: .changeset/tough-elephants-divide.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-import-x": patch
+---
+
+Properly fix espree parser w/ ESLint Flat Config

Diff for: docs/rules/no-relative-packages.md

@@ -6,8 +6,7 @@
 
 Use this rule to prevent importing packages through relative paths.
 
-It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling
-package using `../package` relative path, while direct `package` is the correct one.
+It's useful in a monorepo setup, where it's possible to import a sibling package using `../package` relative path, while direct `package` is the correct one.
 
 ## Examples
 

Diff for: docs/rules/no-restricted-paths.md

@@ -5,7 +5,7 @@
 Some projects contain files which are not always meant to be executed in the same environment.
 For example consider a web application that contains specific code for the server and some specific code for the browser/client. In this case you don’t want to import server-only files in your client code.
 
-In order to prevent such scenarios this rule allows you to define restricted zones where you can forbid files from imported if they match a specific path.
+In order to prevent such scenarios this rule allows you to define restricted zones where you can forbid files from being imported if they match a specific path.
 
 ## Rule Details
 

Diff for: src/rules/export.ts

@@ -1,3 +1,4 @@
+import { AST_NODE_TYPES } from '@typescript-eslint/utils'
 import type { TSESTree } from '@typescript-eslint/utils'
 
 import {
@@ -30,37 +31,24 @@ const rootProgram = 'root'
 const tsTypePrefix = 'type:'
 
 /**
- * Detect function overloads like:
+ * Remove function overloads like:
+ *
  * ```ts
  * export function foo(a: number);
  * export function foo(a: string);
  * export function foo(a: number|string) { return a; }
  * ```
  */
-function isTypescriptFunctionOverloads(nodes: Set<TSESTree.Node>) {
-  const nodesArr = [...nodes]
-
-  const idents = nodesArr.flatMap(node =>
-    'declaration' in node && node.declaration?.type === 'TSDeclareFunction'
-      ? node.declaration.id!.name
-      : [],
-  )
-
-  if (new Set(idents).size !== idents.length) {
-    return true
-  }
-
-  const types = new Set(nodesArr.map(node => `${node.parent!.type}` as const))
-  if (!types.has('TSDeclareFunction')) {
-    return false
-  }
-  if (types.size === 1) {
-    return true
-  }
-  if (types.size === 2 && types.has('FunctionDeclaration')) {
-    return true
+function removeTypescriptFunctionOverloads(nodes: Set<TSESTree.Node>) {
+  for (const node of nodes) {
+    const declType =
+      node.type === AST_NODE_TYPES.ExportDefaultDeclaration
+        ? node.declaration.type
+        : node.parent?.type
+    if (declType === AST_NODE_TYPES.TSDeclareFunction) {
+      nodes.delete(node)
+    }
   }
-  return false
 }
 
 /**
@@ -277,14 +265,17 @@ export = createRule<[], MessageId>({
       'Program:exit'() {
         for (const [, named] of namespace) {
           for (const [name, nodes] of named) {
+            if (nodes.size === 0) {
+              continue
+            }
+
+            removeTypescriptFunctionOverloads(nodes)
+
             if (nodes.size <= 1) {
               continue
             }
 
-            if (
-              isTypescriptFunctionOverloads(nodes) ||
-              isTypescriptNamespaceMerging(nodes)
-            ) {
+            if (isTypescriptNamespaceMerging(nodes)) {
               continue
             }
 

Diff for: src/rules/no-named-as-default.ts

@@ -49,6 +49,17 @@ export = createRule<[], MessageId>({
           return
         }
 
+        if (!exportMapOfImported.hasDefault) {
+          // The rule is triggered for default imports/exports, so if the imported module has no default
+          // this means we're dealing with incorrect source code anyway
+          return
+        }
+
+        if (!exportMapOfImported.has(nameValue)) {
+          // The name used locally for the default import was not even used in the imported module.
+          return
+        }
+
         if (
           exportMapOfImported.exports.has('default') &&
           exportMapOfImported.exports.has(nameValue)

Diff for: src/utils/export-map.ts

@@ -128,7 +128,11 @@ export class ExportMap {
 
     exportMap.mtime = stats().mtime.valueOf()
 
-    exportCache.set(cacheKey, exportMap)
+    // If the visitor keys were not populated, then we shouldn't save anything to the cache,
+    // since the parse results may not be reliable.
+    if (exportMap.visitorKeys) {
+      exportCache.set(cacheKey, exportMap)
+    }
 
     return exportMap
   }

Diff for: src/utils/parse.ts

@@ -29,16 +29,22 @@ function withoutProjectParserOptions(
 const log = debug('eslint-plugin-import-x:parse')
 
 function keysFromParser(
-  parserPath: string | TSESLint.Parser.ParserModule,
+  _parserPath: string | TSESLint.Parser.ParserModule,
   parserInstance: TSESLint.Parser.ParserModule,
   parsedResult?: TSESLint.Parser.ParseResult,
 ) {
   // Exposed by @typescript-eslint/parser and @babel/eslint-parser
   if (parsedResult && parsedResult.visitorKeys) {
     return parsedResult.visitorKeys
   }
-  if (typeof parserPath === 'string' && /.*espree.*/.test(parserPath)) {
-    // @ts-expect-error - no type yet
+
+  // The espree parser doesn't have the `parseForESLint` function, so we don't ended up with a
+  // `parsedResult` here, but it does expose the visitor keys on the parser instance that we can use.
+  if (
+    parserInstance &&
+    'VisitorKeys' in parserInstance &&
+    parserInstance.VisitorKeys
+  ) {
     return parserInstance.VisitorKeys as TSESLint.SourceCode.VisitorKeys
   }
   return null

Diff for: test/fixtures/order-redirect-scoped/module/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "order-redirect-module",
+  "name": "order-redirect-module-scoped",
   "private": true,
   "main": "../other-module/file.js"
 }

Diff for: test/rules/export.spec.ts

@@ -58,6 +58,14 @@ ruleTester.run('export', rule, {
         }
       `,
     },
+    {
+      code: `
+      export default function foo(param: string): boolean;
+      export default function foo(param: string, param1?: number): boolean {
+        return param && param1;
+      }
+    `,
+    },
   ],
 
   invalid: [
@@ -160,6 +168,15 @@ ruleTester.run('export', rule, {
         },
       },
     }),
+
+    test({
+      code: `
+        export default function a(): void;
+        export default function a() {}
+        export { x as default };
+      `,
+      errors: ['Multiple default exports.', 'Multiple default exports.'],
+    }),
   ],
 })
 
@@ -510,7 +527,7 @@ describe('TypeScript', () => {
       }),
       test({
         code: `
-            export function Foo();
+            export function Foo() { };
             export class Foo { }
             export namespace Foo { }
           `,
@@ -529,7 +546,7 @@ describe('TypeScript', () => {
       test({
         code: `
             export const Foo = 'bar';
-            export function Foo();
+            export function Foo() { };
             export namespace Foo { }
           `,
         errors: [

Diff for: test/rules/no-cycle.spec.ts

@@ -11,8 +11,8 @@ const error = (message: string) => ({ message })
 
 const test = <T extends ValidTestCase>(def: T) =>
   _test({
-    ...def,
     filename: testFilePath('./cycles/depth-zero.js'),
+    ...def,
   })
 
 ruleTester.run('no-cycle', rule, {

Diff for: test/rules/no-default-export.spec.ts

@@ -8,6 +8,17 @@ const ruleTester = new TSESLintRuleTester()
 
 ruleTester.run('no-default-export', rule, {
   valid: [
+    test({
+      code: 'module.exports = function foo() {}',
+      languageOptions: {
+        parserOptions: {
+          sourceType: 'script',
+        },
+      },
+    }),
+    test({
+      code: 'module.exports = function foo() {}',
+    }),
     test({
       code: `
         export const foo = 'foo';

Diff for: test/rules/no-named-export.spec.ts

@@ -8,6 +8,17 @@ const ruleTester = new TSESLintRuleTester()
 
 ruleTester.run('no-named-export', rule, {
   valid: [
+    test({
+      code: 'module.export.foo = function () {}',
+    }),
+    test({
+      code: 'module.exports = function foo() {}',
+      languageOptions: {
+        parserOptions: {
+          sourceType: 'script',
+        },
+      },
+    }),
     test({
       code: 'export default function bar() {};',
     }),

Diff for: test/utils/export-map.spec.ts

@@ -30,6 +30,17 @@ describe('ExportMap', () => {
     )
   })
 
+  it('does not return a cached copy if the parse does not yield a visitor keys', function () {
+    const mockContext = {
+      ...fakeContext,
+      parserPath: 'not-real',
+    }
+    expect(ExportMap.get('./named-exports', mockContext)).toBeDefined()
+    expect(ExportMap.get('./named-exports', mockContext)).not.toBe(
+      ExportMap.get('./named-exports', mockContext),
+    )
+  })
+
   it('does not return a cached copy after modification', done => {
     const firstAccess = ExportMap.get('./mutator', fakeContext)
     expect(firstAccess).toBeDefined()