Skip to content

Commit

Permalink
feat(import-target): Add resolution error reason (#264)
Browse files Browse the repository at this point in the history
  • Loading branch information
aladdin-add authored May 7, 2024
1 parent 60bf29f commit 982a723
Show file tree
Hide file tree
Showing 47 changed files with 214 additions and 156 deletions.
51 changes: 9 additions & 42 deletions lib/util/check-existence.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 },
})
}

Expand All @@ -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}}",
}
33 changes: 28 additions & 5 deletions lib/util/import-target.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,6 @@
"*.md": "markdownlint --fix"
},
"imports": {
"#eslint-rule-tester": "./tests/eslint-rule-tester.js"
"#test-helpers": "./tests/test-helpers.js"
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 0 additions & 10 deletions tests/helpers.js

This file was deleted.

49 changes: 22 additions & 27 deletions tests/lib/configs/eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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} */
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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'", {
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/callback-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/exports-style.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/file-extension-in-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (() => {
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/global-require.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/handle-callback-err.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/hashbang.js
Original file line number Diff line number Diff line change
Expand Up @@ -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")

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/no-callback-literal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/no-deprecated-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/no-exports-assign.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/no-extraneous-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (() => {
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/no-extraneous-require.js
Original file line number Diff line number Diff line change
Expand Up @@ -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")

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/rules/no-hide-core-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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")

//------------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 982a723

Please sign in to comment.