fix(eslint-plugin): shortest import sometimes falsely tried to use re…

…lative paths Example case was when a `junit/` folder existed and `junit-report-builder` was added in, causing the detection algorithm for paths to fail.
tablecheck · Sep 2, 2024 · d30de68 · d30de68
1 parent d885c04
commit d30de68
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 33 deletions.
diff --git a/packages/eslint-plugin/__tests__/fixtures/tsconfig.base_root.json b/packages/eslint-plugin/__tests__/fixtures/tsconfig.base_root.json
@@ -0,0 +1,10 @@
+{
+  "extends": "../tsconfig.base.json",
+  "compilerOptions": {
+    "paths": {
+      "~/*": ["./test_src/*"]
+    }
+  },
+  "include": ["test_src/**/*.ts", "test_src/*.tsx"],
+  "exclude": ["node_modules"]
+}
diff --git a/packages/eslint-plugin/__tests__/shortestImport.test.ts b/packages/eslint-plugin/__tests__/shortestImport.test.ts
@@ -1,6 +1,10 @@
 import { join as pathJoin } from 'path';
 
-import { RuleTester } from '@typescript-eslint/rule-tester';
+import {
+  type InvalidTestCase,
+  RuleTester,
+  type ValidTestCase,
+} from '@typescript-eslint/rule-tester';
 
 import { messageId, shortestImport as rule } from '../src/shortestImport';
 
@@ -15,6 +19,11 @@ const typescriptSetups = [
     project: './tsconfig.base_only.json',
     tsconfigRootDir: pathJoin(__dirname, 'fixtures'),
   },
+  {
+    name: 'baseUrl and different root',
+    project: './tsconfig.base_root.json',
+    tsconfigRootDir: pathJoin(__dirname, 'fixtures'),
+  },
   {
     name: 'root',
     project: './test_src/tsconfig.test_root.json',
@@ -27,12 +36,6 @@ const typescriptSetups = [
   },
 ] as const;
 
-type TestResult<T extends { path: string; fixedPath?: undefined | string }> =
-  (Omit<T, 'path' | 'fixedPath'> &
-    ([undefined] extends [T['fixedPath']]
-      ? { code: string }
-      : { code: string; output: string }))[];
-
 function buildCodeCase<
   T extends {
     name?: string;
@@ -75,26 +78,31 @@ function convertPathCaseToCodeCase<
     path: string;
     fixedPath?: undefined | string;
   } & Record<string, any>,
->(config: T[]): TestResult<T> {
-  return config.reduce((r, { path, fixedPath, ...rest }) => {
-    r.push(
-      buildCodeCase({
-        path,
-        fixedPath,
-        importType: 'default',
-        rest,
-      }) as never,
-    );
-    r.push(
-      buildCodeCase({
-        path,
-        fixedPath,
-        importType: 'dynamic',
-        rest,
-      }) as never,
-    );
-    return r;
-  }, [] as TestResult<T>);
+>(config: T[]) {
+  return config.reduce(
+    (r, { path, fixedPath, ...rest }) => {
+      r.push(
+        buildCodeCase({
+          path,
+          fixedPath,
+          importType: 'default',
+          rest,
+        }) as never,
+      );
+      r.push(
+        buildCodeCase({
+          path,
+          fixedPath,
+          importType: 'dynamic',
+          rest,
+        }) as never,
+      );
+      return r;
+    },
+    [] as (T extends { errors: any }
+      ? InvalidTestCase<'shortestImport', unknown[]>
+      : ValidTestCase<unknown[]>)[],
+  );
 }
 
 function mapConfig<
@@ -198,6 +206,10 @@ typescriptSetups.forEach((config) => {
           path: 'react',
           filename: './test_src/feature1/slice1/inner1/index.ts',
         },
+        {
+          path: 'test-package-name',
+          filename: './test_src/feature1/slice1/inner1/index.ts',
+        },
         {
           path: '.',
           filename: './test_src/feature1/slice1/inner1/index.ts',
@@ -235,14 +247,15 @@ typescriptSetups.forEach((config) => {
           errors: [{ messageId }],
         },
         {
-          skipConfigs: ['baseUrl Only'],
+          skipConfigs: ['baseUrl Only', 'baseUrl and different root'],
           name: 'prefer alias path over baseUrl resolve',
           path: 'feature2',
           fixedPath: '~/feature2',
           filename: './test_src/feature1/slice1/index.ts',
           errors: [{ messageId }],
         },
         {
+          skipConfigs: ['baseUrl and different root'],
           name: 'prefer relative parent path over alias/baseUrl',
           path: 'feature1/slice2',
           fixedPath: '../slice2',

diff --git a/packages/eslint-plugin/__tests__/test-package/.gitkeep b/packages/eslint-plugin/__tests__/test-package/.gitkeep
diff --git a/packages/eslint-plugin/src/shortestImport.ts b/packages/eslint-plugin/src/shortestImport.ts
@@ -33,9 +33,8 @@ class RuleChecker {
 
   constructor(compilerOptions: CompilerOptions) {
     const { baseUrl, pathsBasePath, rootDir, rootDirs } = compilerOptions;
-
     this.baseUrl = baseUrl;
-    this.pathsBasePath = pathsBasePath;
+    this.pathsBasePath = pathsBasePath ?? baseUrl;
     this.rootDir = rootDir;
     this.rootDirs = rootDirs;
     this.compilerPaths = this.composeCompilerPaths(compilerOptions.paths);
@@ -172,10 +171,8 @@ class RuleChecker {
     if (importPath.startsWith('@') || importPath === '.') return true;
     const isPathMapping = Object.keys(this.allPaths).some(
       (key) =>
-        importPath.startsWith(key) ||
-        importPath.startsWith(key.replace(/\/$/, '')),
+        importPath.startsWith(key) || importPath === key.replace(/\/$/, ''),
     );
-
     if (isPathMapping) return false;
     return !importPath.startsWith('.') && !importPath.startsWith('/');
   }