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.

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

Add n/prefer-node-protocol rule #183

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ For [Shareable Configs](https://eslint.org/docs/latest/developer-guide/shareable
| [prefer-global/text-encoder](docs/rules/prefer-global/text-encoder.md) | enforce either `TextEncoder` or `require("util").TextEncoder` | | | |
| [prefer-global/url](docs/rules/prefer-global/url.md) | enforce either `URL` or `require("url").URL` | | | |
| [prefer-global/url-search-params](docs/rules/prefer-global/url-search-params.md) | enforce either `URLSearchParams` or `require("url").URLSearchParams` | | | |
| [prefer-node-protocol](docs/rules/prefer-node-protocol.md) | enforce using the `node:` protocol when importing Node.js builtin modules. | | 🔧 | |
| [prefer-promises/dns](docs/rules/prefer-promises/dns.md) | enforce `require("dns").promises` | | | |
| [prefer-promises/fs](docs/rules/prefer-promises/fs.md) | enforce `require("fs").promises` | | | |
| [process-exit-as-throw](docs/rules/process-exit-as-throw.md) | require that `process.exit()` expressions use the same code path as `throw` | ☑️ 🟢 ✅ ☑️ 🟢 ✅ | | |
Expand Down
73 changes: 73 additions & 0 deletions docs/rules/prefer-node-protocol.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# Enforce using the `node:` protocol when importing Node.js builtin modules (`n/prefer-node-protocol`)

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->

Older built-in Node modules such as fs now can be imported via either their name or `node:` + their name:

```js
import fs from "fs"
import fs from "node:fs"
```

The prefixed versions are nice because they can't be overridden by user modules and are similarly formatted to prefix-only modules such as node:test.

Note that Node.js support for this feature began in:

> v16.0.0, v14.18.0 (`require()`)
> v14.13.1, v12.20.0 (`import`)
scagood marked this conversation as resolved.
Show resolved Hide resolved

## 📖 Rule Details

This rule enforces that `node:` protocol is prepended to built-in Node modules when importing or exporting built-in Node modules.

👍 Examples of **correct** code for this rule:

```js
/*eslint n/prefer-node-protocol: error */

import fs from "node:fs"

export { promises } from "node:fs"

const fs = require("node:fs")
```

👎 Examples of **incorrect** code for this rule:

```js
/*eslint n/prefer-node-protocol: error */

import fs from "fs"

export { promises } from "fs"

const fs = require("fs")
```

### Configured Node.js version range

[Configured Node.js version range](../../../README.md#configured-nodejs-version-range)

### Options

```json
{
"n/prefer-node-protocol": ["error", {
"version": ">=16.0.0",
}]
}
```

#### version

As mentioned above, this rule reads the [`engines`] field of `package.json`.
But, you can overwrite the version by `version` option.

The `version` option accepts [the valid version range of `node-semver`](https://github.com/npm/node-semver#range-grammar).

## 🔎 Implementation

- [Rule source](../../lib/rules/prefer-node-protocol.js)
- [Test source](../../tests/lib/rules/prefer-node-protocol.js)
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const rules = {
"prefer-global/text-encoder": require("./rules/prefer-global/text-encoder"),
"prefer-global/url-search-params": require("./rules/prefer-global/url-search-params"),
"prefer-global/url": require("./rules/prefer-global/url"),
"prefer-node-protocol": require("./rules/prefer-node-protocol"),
"prefer-promises/dns": require("./rules/prefer-promises/dns"),
"prefer-promises/fs": require("./rules/prefer-promises/fs"),
"process-exit-as-throw": require("./rules/process-exit-as-throw"),
Expand Down
150 changes: 150 additions & 0 deletions lib/rules/prefer-node-protocol.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/**
* @author Yusuke Iinuma
* See LICENSE file in root directory for full license.
*/
"use strict"

const isBuiltinModule = require("is-builtin-module")
const getConfiguredNodeVersion = require("../util/get-configured-node-version")
const getSemverRange = require("../util/get-semver-range")
const visitImport = require("../util/visit-import")
const visitRequire = require("../util/visit-require")
const mergeVisitorsInPlace = require("../util/merge-visitors-in-place")

const messageId = "preferNodeProtocol"

module.exports = {
meta: {
docs: {
description:
"enforce using the `node:` protocol when importing Node.js builtin modules.",
recommended: true,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the recommended config changes would be a breaking change.

Suggested change
recommended: true,
recommended: false,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review 🙏
I fixed it.

url: "https://github.com/eslint-community/eslint-plugin-n/blob/HEAD/docs/rules/prefer-node-protocol.md",
},
fixable: "code",
messages: {
[messageId]: "Prefer `node:{{moduleName}}` over `{{moduleName}}`.",
},
schema: [
{
type: "object",
properties: {
version: getConfiguredNodeVersion.schema,
},
additionalProperties: false,
},
],
type: "suggestion",
},
create(context) {
scagood marked this conversation as resolved.
Show resolved Hide resolved
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
}

function isStringLiteral(node) {
return node?.type === "Literal" && typeof node.type === "string"
}

function isStaticRequire(node) {
return (
isCallExpression(node, {
name: "require",
argumentsLength: 1,
}) && isStringLiteral(node.arguments[0])
)
}

function isEnablingThisRule(context, moduleStyle) {
const version = getConfiguredNodeVersion(context)

const supportedVersionForEsm = "^12.20.0 || >= 14.13.1"
// Only check Node.js version because this rule is meaningless if configured Node.js version doesn't match semver range.
if (!version.intersects(getSemverRange(supportedVersionForEsm))) {
return false
}

const supportedVersionForCjs = "^14.18.0 || >= 16.0.0"
// Only check when using `require`
if (
moduleStyle === "require" &&
!version.intersects(getSemverRange(supportedVersionForCjs))
) {
return false
}

return true
}

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)
)
scagood marked this conversation as resolved.
Show resolved Hide resolved
)
}),
{
"Program:exit"() {
for (const { node, moduleStyle } of targets) {
if (!isEnablingThisRule(context, moduleStyle)) {
return
}

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

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

context.report({
node,
messageId,
fix(fixer) {
const firstCharacterIndex = node.range[0] + 1
return fixer.replaceTextRange(
[firstCharacterIndex, firstCharacterIndex],
"node:"
)
},
})
}
},
},
].reduce(
(mergedVisitor, thisVisitor) =>
mergeVisitorsInPlace(mergedVisitor, thisVisitor),
{}
)
},
}
2 changes: 1 addition & 1 deletion lib/util/strip-import-path-params.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
"use strict"

module.exports = function stripImportPathParams(path) {
const i = path.indexOf("!")
const i = path.toString().indexOf("!")
scagood marked this conversation as resolved.
Show resolved Hide resolved
return i === -1 ? path : path.slice(0, i)
}