feat(import-target): Add resolution error reason (#264)

eslint-community · May 7, 2024 · 982a723 · 982a723
1 parent 60bf29f
commit 982a723
Show file tree

Hide file tree

Showing 47 changed files with 214 additions and 156 deletions.
diff --git a/lib/util/check-existence.js b/lib/util/check-existence.js
@@ -4,11 +4,7 @@
  */
 "use strict"
 
-const path = require("path")
-const exists = require("./exists")
 const getAllowModules = require("./get-allow-modules")
-const isTypescript = require("./is-typescript")
-const { convertJsExtensionToTs } = require("../util/map-typescript-extension")
 
 /**
  * Reports a missing file from ImportTarget
@@ -17,13 +13,16 @@ const { convertJsExtensionToTs } = require("../util/map-typescript-extension")
  * @returns {void}
  */
 function markMissing(context, target) {
+    // This should never happen... this is just a fallback for typescript
+    target.resolveError ??= `"${target.name}" is not found`
+
     context.report({
         node: target.node,
         loc: /** @type {import('eslint').AST.SourceLocation} */ (
             target.node.loc
         ),
         messageId: "notFound",
-        data: /** @type {Record<string, *>} */ (target),
+        data: { resolveError: target.resolveError },
     })
 }
 
@@ -38,52 +37,20 @@ function markMissing(context, target) {
  * @returns {void}
  */
 exports.checkExistence = function checkExistence(context, targets) {
+    /** @type {Set<string | undefined>} */
     const allowed = new Set(getAllowModules(context))
 
-    target: for (const target of targets) {
-        if (
-            target.moduleName != null &&
-            !allowed.has(target.moduleName) &&
-            target.filePath == null
-        ) {
-            markMissing(context, target)
-            continue
-        }
-
-        if (
-            target.moduleName != null ||
-            target.filePath == null ||
-            exists(target.filePath)
-        ) {
+    for (const target of targets) {
+        if (allowed.has(target.moduleName)) {
             continue
         }
 
-        if (isTypescript(context) === false) {
+        if (target.resolveError != null) {
             markMissing(context, target)
-            continue
         }
-
-        const parsed = path.parse(target.filePath)
-        const pathWithoutExt = path.resolve(parsed.dir, parsed.name)
-
-        const reversedExtensions = convertJsExtensionToTs(
-            context,
-            target.filePath,
-            parsed.ext
-        )
-
-        for (const reversedExtension of reversedExtensions) {
-            const reversedPath = pathWithoutExt + reversedExtension
-
-            if (exists(reversedPath)) {
-                continue target
-            }
-        }
-
-        markMissing(context, target)
     }
 }
 
 exports.messages = {
-    notFound: '"{{name}}" is not found.',
+    notFound: "{{resolveError}}",
 }
diff --git a/lib/util/import-target.js b/lib/util/import-target.js
@@ -131,6 +131,12 @@ module.exports = class ImportTarget {
          */
         this.moduleName = this.getModuleName()
 
+        /**
+         * This is the full resolution failure reasons
+         * @type {string | null}
+         */
+        this.resolveError = null
+
         /**
          * The full path of this import target.
          * If the target is a module and it does not exist then this is `null`.
@@ -239,6 +245,19 @@ module.exports = class ImportTarget {
         return [this.options.basedir]
     }
 
+    /**
+     * @param {string} baseDir
+     * @param {unknown} error
+     * @returns {void}
+     */
+    handleResolutionError(baseDir, error) {
+        if (error instanceof Error === false) {
+            throw error
+        }
+
+        this.resolveError = error.message
+    }
+
     /**
      * Resolve the given id to file paths.
      * @returns {string | null} The resolved path.
@@ -274,24 +293,28 @@ module.exports = class ImportTarget {
             extensionAlias = getTypescriptExtensionMap(this.context).backward
         }
 
-        const requireResolve = resolver.create.sync({
+        /** @type {import('enhanced-resolve').ResolveOptionsOptionalFS} */
+        this.resolverConfig = {
             conditionNames,
             extensions,
             mainFields,
             mainFiles,
 
             extensionAlias,
             alias,
-        })
+        }
+
+        const requireResolve = resolver.create.sync(this.resolverConfig)
 
         const cwd = this.context.settings?.cwd ?? process.cwd()
         for (const directory of this.getPaths()) {
+            const baseDir = resolve(cwd, directory)
+
             try {
-                const baseDir = resolve(cwd, directory)
                 const resolved = requireResolve(baseDir, this.name)
                 if (typeof resolved === "string") return resolved
-            } catch {
-                continue
+            } catch (error) {
+                this.handleResolutionError(baseDir, error)
             }
         }
 

diff --git a/package.json b/package.json
@@ -117,6 +117,6 @@
         "*.md": "markdownlint --fix"
     },
     "imports": {
-        "#eslint-rule-tester": "./tests/eslint-rule-tester.js"
+        "#test-helpers": "./tests/test-helpers.js"
     }
 }
diff --git a/tests/fixtures/no-missing/node_modules/misconfigured-default/build/index.js b/tests/fixtures/no-missing/node_modules/misconfigured-default/build/index.js
diff --git a/tests/fixtures/no-missing/node_modules/misconfigured-default/package.json b/tests/fixtures/no-missing/node_modules/misconfigured-default/package.json
diff --git a/tests/fixtures/no-missing/node_modules/misconfigured-default/src/index.ts b/tests/fixtures/no-missing/node_modules/misconfigured-default/src/index.ts
diff --git a/tests/helpers.js b/tests/helpers.js
diff --git a/tests/lib/configs/eslintrc.js b/tests/lib/configs/eslintrc.js
@@ -3,8 +3,6 @@
 const assert = require("assert")
 const path = require("path")
 const { LegacyESLint } = require("eslint/use-at-your-own-risk")
-// const {ESLint} = require("eslint")
-const { gtEslintV8 } = require("../../helpers")
 const originalCwd = process.cwd()
 
 // this is needed as `recommended` config was cached
@@ -15,7 +13,7 @@ function clearRequireCache() {
 }
 
 describe("node/recommended config", () => {
-    ;(gtEslintV8 ? describe : describe.skip)("in CJS directory", () => {
+    describe("in CJS directory", () => {
         const root = path.resolve(__dirname, "../../fixtures/configs/cjs/")
 
         /** @type {Linter} */
@@ -84,7 +82,7 @@ describe("node/recommended config", () => {
                     endLine: 1,
                     line: 1,
                     messageId: "notFound",
-                    message: '"foo" is not found.',
+                    message: `Can't resolve 'foo' in '${root}'`,
                     nodeType: "Literal",
                     ruleId: "n/no-missing-import",
                     severity: 2,
@@ -124,34 +122,31 @@ describe("node/recommended config", () => {
                     endLine: 1,
                     line: 1,
                     messageId: "notFound",
-                    message: '"foo" is not found.',
+                    message: `Can't resolve 'foo' in '${root}'`,
                     nodeType: "Literal",
                     ruleId: "n/no-missing-import",
                     severity: 2,
                 },
             ])
         })
-        ;(gtEslintV8 ? it : it.skip)(
-            "*.cjs files should be a script.",
-            async () => {
-                const report = await linter.lintText("import 'foo'", {
-                    filePath: path.join(root, "test.cjs"),
-                })
-
-                assert.deepStrictEqual(report[0].messages, [
-                    {
-                        column: 1,
-                        fatal: true,
-                        line: 1,
-                        message:
-                            "Parsing error: 'import' and 'export' may appear only with 'sourceType: module'",
-                        ruleId: null,
-                        nodeType: null,
-                        severity: 2,
-                    },
-                ])
-            }
-        )
+        it("*.cjs files should be a script.", async () => {
+            const report = await linter.lintText("import 'foo'", {
+                filePath: path.join(root, "test.cjs"),
+            })
+
+            assert.deepStrictEqual(report[0].messages, [
+                {
+                    column: 1,
+                    fatal: true,
+                    line: 1,
+                    message:
+                        "Parsing error: 'import' and 'export' may appear only with 'sourceType: module'",
+                    ruleId: null,
+                    nodeType: null,
+                    severity: 2,
+                },
+            ])
+        })
 
         it("*.mjs files should be a module.", async () => {
             const report = await linter.lintText("import 'foo'", {
@@ -165,7 +160,7 @@ describe("node/recommended config", () => {
                     endLine: 1,
                     line: 1,
                     messageId: "notFound",
-                    message: '"foo" is not found.',
+                    message: `Can't resolve 'foo' in '${root}'`,
                     nodeType: "Literal",
                     ruleId: "n/no-missing-import",
                     severity: 2,

diff --git a/tests/lib/rules/callback-return.js b/tests/lib/rules/callback-return.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/callback-return")
 const ruleTester = new RuleTester()
 

diff --git a/tests/lib/rules/exports-style.js b/tests/lib/rules/exports-style.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/exports-style")
 
 new RuleTester({ languageOptions: { ecmaVersion: 11 } }).run(

diff --git a/tests/lib/rules/file-extension-in-import.js b/tests/lib/rules/file-extension-in-import.js
@@ -6,7 +6,7 @@
 
 const path = require("path")
 const { Linter } = require("eslint")
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("../../test-helpers").RuleTester
 const rule = require("../../../lib/rules/file-extension-in-import")
 
 const DynamicImportSupported = (() => {

diff --git a/tests/lib/rules/global-require.js b/tests/lib/rules/global-require.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/global-require")
 
 const ERROR = { messageId: "unexpected", type: "CallExpression" }

diff --git a/tests/lib/rules/handle-callback-err.js b/tests/lib/rules/handle-callback-err.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/handle-callback-err")
 const ruleTester = new RuleTester()
 

diff --git a/tests/lib/rules/hashbang.js b/tests/lib/rules/hashbang.js
@@ -5,7 +5,7 @@
 "use strict"
 
 const path = require("path")
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/shebang")
 
 /**

diff --git a/tests/lib/rules/no-callback-literal.js b/tests/lib/rules/no-callback-literal.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/no-callback-literal")
 const tsParser = require("@typescript-eslint/parser")
 

diff --git a/tests/lib/rules/no-deprecated-api.js b/tests/lib/rules/no-deprecated-api.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const { RuleTester } = require("#eslint-rule-tester")
+const { RuleTester } = require("#test-helpers")
 const rule = require("../../../lib/rules/no-deprecated-api")
 
 const ruleTester = new RuleTester()

diff --git a/tests/lib/rules/no-exports-assign.js b/tests/lib/rules/no-exports-assign.js
@@ -4,7 +4,7 @@
  */
 "use strict"
 
-const { RuleTester } = require("#eslint-rule-tester")
+const { RuleTester } = require("#test-helpers")
 const rule = require("../../../lib/rules/no-exports-assign.js")
 
 new RuleTester({

diff --git a/tests/lib/rules/no-extraneous-import.js b/tests/lib/rules/no-extraneous-import.js
@@ -6,7 +6,7 @@
 
 const path = require("path")
 const { Linter } = require("eslint")
-const { RuleTester } = require("#eslint-rule-tester")
+const { RuleTester } = require("#test-helpers")
 const rule = require("../../../lib/rules/no-extraneous-import")
 
 const DynamicImportSupported = (() => {

diff --git a/tests/lib/rules/no-extraneous-require.js b/tests/lib/rules/no-extraneous-require.js
@@ -5,7 +5,7 @@
 "use strict"
 
 const path = require("path")
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/no-extraneous-require")
 
 /**

diff --git a/tests/lib/rules/no-hide-core-modules.js b/tests/lib/rules/no-hide-core-modules.js
@@ -14,7 +14,7 @@
 //------------------------------------------------------------------------------
 
 const path = require("path")
-const RuleTester = require("#eslint-rule-tester").RuleTester
+const RuleTester = require("#test-helpers").RuleTester
 const rule = require("../../../lib/rules/no-hide-core-modules")
 
 //------------------------------------------------------------------------------