Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: improve prefer-node-protocol's performance #406

Merged
merged 3 commits into from
Feb 1, 2025
+131 −135
Merged

perf: improve prefer-node-protocol's performance #406

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc9d6c8
Improve `prefer-node-protocol`'s performance
pralkarz Jan 21, 2025
0544a84
Reduce the diff (hopefully)
pralkarz Jan 21, 2025
27b79f5
Revert "Reduce the diff (hopefully)"
pralkarz Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
266 changes: 131 additions & 135 deletions lib/rules/prefer-node-protocol.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
*/
"use strict"

const { getStringIfConstant } = require("@eslint-community/eslint-utils")

const { Range } = require("semver")

const getConfiguredNodeVersion = require("../util/get-configured-node-version")
const visitImport = require("../util/visit-import")
const visitRequire = require("../util/visit-require")
const mergeVisitorsInPlace = require("../util/merge-visitors-in-place")
const stripImportPathParams = require("../util/strip-import-path-params")

const {
NodeBuiltinModules,
} = require("../unsupported-features/node-builtins.js")
Expand All @@ -27,6 +28,104 @@ const messageId = "preferNodeProtocol"
const supportedRangeForEsm = new Range("^12.20.0 || >= 14.13.1")
const supportedRangeForCjs = new Range("^14.18.0 || >= 16.0.0")

/**
* @param {import('estree').Node} [node]
* @returns {node is import('estree').Literal}
*/
function isStringLiteral(node) {
return node?.type === "Literal" && typeof node.type === "string"
}

/**
* @param {import('eslint').Rule.RuleContext} context
* @param {import('../util/import-target.js').ModuleStyle} moduleStyle
* @returns {boolean}
*/
function isEnablingThisRule(context, moduleStyle) {
const version = getConfiguredNodeVersion(context)

// Only check Node.js version because this rule is meaningless if configured Node.js version doesn't match semver range.
if (!version.intersects(supportedRangeForEsm)) {
return false
}

// Only check when using `require`
if (
moduleStyle === "require" &&
!version.intersects(supportedRangeForCjs)
) {
return false
}

return true
}

/**
* @param {import('estree').Node} node
* @returns {boolean}
**/
function isValidRequireArgument(node) {
const rawName = getStringIfConstant(node)
if (typeof rawName !== "string") {
return false
}

const name = stripImportPathParams(rawName)
if (!isBuiltin(name)) {
return false
}

return true
}

/**
* @param {import('estree').Node | null | undefined} node
* @param {import('eslint').Rule.RuleContext} context
* @param {import('../util/import-target.js').ModuleStyle} moduleStyle
*/
function validate(node, context, moduleStyle) {
if (node == null) {
return
}

if (!isEnablingThisRule(context, moduleStyle)) {
return
}

if (!isStringLiteral(node)) {
return
}

if (moduleStyle === "require" && !isValidRequireArgument(node)) {
return
}

if (
!("value" in node) ||
typeof node.value !== "string" ||
node.value.startsWith("node:") ||
!isBuiltin(node.value) ||
!isBuiltin(`node:${node.value}`)
) {
return
}

context.report({
node,
messageId,
data: {
moduleName: node.value,
},
fix(fixer) {
const firstCharacterIndex = (node?.range?.[0] ?? 0) + 1
return fixer.replaceTextRange(
[firstCharacterIndex, firstCharacterIndex],
"node:"
)
},
})
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
Expand All @@ -52,139 +151,36 @@ module.exports = {
type: "suggestion",
},
create(context) {
/**
* @param {import('estree').Node} node
* @param {object} options
* @param {string} options.name
* @param {number} options.argumentsLength
* @returns {node is import('estree').CallExpression}
*/
function isCallExpression(node, { name, argumentsLength }) {
if (node?.type !== "CallExpression") {
return false
}

if (node.optional) {
return false
}

if (node.arguments.length !== argumentsLength) {
return false
}

if (
node.callee.type !== "Identifier" ||
node.callee.name !== name
) {
return false
}

return true
}

/**
* @param {import('estree').Node} [node]
* @returns {node is import('estree').Literal}
*/
function isStringLiteral(node) {
return node?.type === "Literal" && typeof node.type === "string"
}

/**
* @param {import('estree').Node | undefined} node
* @returns {node is import('estree').CallExpression}
*/
function isStaticRequire(node) {
return (
node != null &&
isCallExpression(node, {
name: "require",
argumentsLength: 1,
}) &&
isStringLiteral(node.arguments[0])
)
}

/**
* @param {import('eslint').Rule.RuleContext} context
* @param {import('../util/import-target.js').ModuleStyle} moduleStyle
* @returns {boolean}
*/
function isEnablingThisRule(context, moduleStyle) {
const version = getConfiguredNodeVersion(context)

// Only check Node.js version because this rule is meaningless if configured Node.js version doesn't match semver range.
if (!version.intersects(supportedRangeForEsm)) {
return false
}

// Only check when using `require`
if (
moduleStyle === "require" &&
!version.intersects(supportedRangeForCjs)
) {
return false
}

return true
}
return {
CallExpression(node) {
if (node.type !== "CallExpression") {
return
}

if (
node.optional ||
node.arguments.length !== 1 ||
node.callee.type !== "Identifier" ||
node.callee.name !== "require"
) {
return
}

return validate(node.arguments[0], context, "require")
},

/** @type {import('../util/import-target.js')[]} */
const targets = []
return [
visitImport(context, { includeCore: true }, importTargets => {
targets.push(...importTargets)
}),
visitRequire(context, { includeCore: true }, requireTargets => {
targets.push(
...requireTargets.filter(target =>
isStaticRequire(target.node.parent)
)
)
}),
{
"Program:exit"() {
for (const { node, moduleStyle } of targets) {
if (!isEnablingThisRule(context, moduleStyle)) {
continue
}

if (node.type === "TemplateLiteral") {
continue
}

if (
!("value" in node) ||
typeof node.value !== "string" ||
node.value.startsWith("node:") ||
!isBuiltin(node.value) ||
!isBuiltin(`node:${node.value}`)
) {
continue
}

context.report({
node,
messageId,
data: {
moduleName: node.value,
},
fix(fixer) {
const firstCharacterIndex =
(node?.range?.[0] ?? 0) + 1
return fixer.replaceTextRange(
[firstCharacterIndex, firstCharacterIndex],
"node:"
)
},
})
}
},
ExportAllDeclaration(node) {
return validate(node.source, context, "import")
},
].reduce(
(mergedVisitor, thisVisitor) =>
mergeVisitorsInPlace(mergedVisitor, thisVisitor),
{}
)
ExportNamedDeclaration(node) {
return validate(node.source, context, "import")
},
ImportDeclaration(node) {
return validate(node.source, context, "import")
},
ImportExpression(node) {
return validate(node.source, context, "import")
},
}
},
}
Loading