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

prefer-node-protocol: Always check require() #1827

Merged
merged 5 commits into from May 26, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 5 additions & 15 deletions docs/rules/prefer-node-protocol.md
Expand Up @@ -25,6 +25,10 @@ export {strict as default} from 'assert';
import fs from 'fs/promises';
```

```js
const fs = require('fs/promises');
```

## Pass

```js
Expand All @@ -51,20 +55,6 @@ import _ from 'lodash';
import fs from './fs.js';
```

## Options

Type: `object`

### `checkRequire`

Type: `boolean`\
Default: `false`

Currently, `require(…)` with the `node:` protocol is only available on Node.js 16. If you don't care about old versions, you can set this to `true`.

We'll remove this option and check `require(…)` by default once this feature get backported to v12.

```js
// eslint unicorn/prefer-node-protocol: ["error", {"checkRequire": true}]
const fs = require('fs'); // Fails
const fs = require('node:fs/promises');
```
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -11,7 +11,7 @@
"url": "https://sindresorhus.com"
},
"engines": {
"node": ">=14"
"node": ">=14.18"
},
"scripts": {
"create-rule": "node ./scripts/create-rule.mjs && npm run generate-rule-notices && npm run generate-rules-table",
Expand Down
2 changes: 1 addition & 1 deletion rules/filename-case.js
@@ -1,5 +1,5 @@
'use strict';
const path = require('path');
const path = require('node:path');
const {camelCase, kebabCase, snakeCase, upperFirst} = require('lodash');
const cartesianProductSamples = require('./utils/cartesian-product-samples.js');

Expand Down
65 changes: 22 additions & 43 deletions rules/prefer-node-protocol.js
Expand Up @@ -14,51 +14,31 @@ const importExportSourceSelector = [
'Literal.source',
].join('');

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const {checkRequire} = {
checkRequire: false,
...context.options[0],
};
const selectors = [importExportSourceSelector];
if (checkRequire) {
selectors.push(STATIC_REQUIRE_SOURCE_SELECTOR);
}
const selector = matches([
importExportSourceSelector,
STATIC_REQUIRE_SOURCE_SELECTOR,
]);

return {
[matches(selectors)](node) {
const {value} = node;
if (
typeof value !== 'string'
|| value.startsWith('node:')
|| !isBuiltinModule(value)
) {
return;
}
const create = () => ({
[selector](node) {
const {value} = node;
if (
typeof value !== 'string'
|| value.startsWith('node:')
|| !isBuiltinModule(value)
) {
return;
}

return {
node,
messageId: MESSAGE_ID,
data: {moduleName: value},
/** @param {import('eslint').Rule.RuleFixer} fixer */
fix: fixer => replaceStringLiteral(fixer, node, 'node:', 0, 0),
};
},
};
};

const schema = [
{
type: 'object',
additionalProperties: false,
properties: {
checkRequire: {
type: 'boolean',
default: false,
},
},
return {
node,
messageId: MESSAGE_ID,
data: {moduleName: value},
/** @param {import('eslint').Rule.RuleFixer} fixer */
fix: fixer => replaceStringLiteral(fixer, node, 'node:', 0, 0),
};
},
];
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
Expand All @@ -69,7 +49,6 @@ module.exports = {
description: 'Prefer using the `node:` protocol when importing Node.js builtin modules.',
},
fixable: 'code',
schema,
messages,
},
};
2 changes: 1 addition & 1 deletion rules/prevent-abbreviations.js
@@ -1,5 +1,5 @@
'use strict';
const path = require('path');
const path = require('node:path');
const {defaultsDeep, upperFirst, lowerFirst} = require('lodash');

const avoidCapture = require('./utils/avoid-capture.js');
Expand Down
2 changes: 1 addition & 1 deletion rules/utils/get-documentation-url.js
@@ -1,5 +1,5 @@
'use strict';
const path = require('path');
const path = require('node:path');
const packageJson = require('../../package.json');

const repoUrl = 'https://github.com/sindresorhus/eslint-plugin-unicorn';
Expand Down
4 changes: 2 additions & 2 deletions rules/utils/rule.js
@@ -1,6 +1,6 @@
'use strict';
const path = require('path');
const fs = require('fs');
const path = require('node:path');
const fs = require('node:fs');
const getDocumentationUrl = require('./get-documentation-url.js');

const isIterable = object => typeof object[Symbol.iterator] === 'function';
Expand Down
2 changes: 1 addition & 1 deletion scripts/internal-rules/prefer-disallow-over-forbid.js
@@ -1,5 +1,5 @@
'use strict';
const path = require('path');
const path = require('node:path');
const {matches} = require('../../rules/selectors/index.js');
const toLocation = require('../../rules/utils/to-location.js');

Expand Down
@@ -1,5 +1,5 @@
'use strict';
const path = require('path');
const path = require('node:path');

const messageId = path.basename(__filename, '.js');

Expand Down
8 changes: 3 additions & 5 deletions test/prefer-node-protocol.mjs
Expand Up @@ -8,7 +8,6 @@ test.snapshot({
'import unicorn from "unicorn";',
'import fs from "./fs";',
'import fs from "unknown-builtin-module";',
'const fs = require("fs");',
'import fs from "node:fs";',
outdent`
async function foo() {
Expand Down Expand Up @@ -60,8 +59,7 @@ test.snapshot({
],
});

// `options`
const checkRequireOptions = [{checkRequire: true}];
// `require`
test.snapshot({
valid: [
'const fs = require("node:fs");',
Expand All @@ -76,11 +74,11 @@ test.snapshot({
'const fs = require();',
'const fs = require(...["fs"]);',
'const fs = require("unicorn");',
].map(code => ({code, options: checkRequireOptions})),
],
invalid: [
'const {promises} = require("fs")',
'const fs = require(\'fs/promises\')',
].map(code => ({code, options: checkRequireOptions})),
],
});

test.babel({
Expand Down
20 changes: 0 additions & 20 deletions test/snapshots/prefer-node-protocol.mjs.md
Expand Up @@ -255,16 +255,6 @@ Generated by [AVA](https://avajs.dev).
## Invalid #1
1 | const {promises} = require("fs")

> Options

`␊
[␊
{␊
"checkRequire": true␊
}␊
]␊
`

> Output

`␊
Expand All @@ -281,16 +271,6 @@ Generated by [AVA](https://avajs.dev).
## Invalid #2
1 | const fs = require('fs/promises')

> Options

`␊
[␊
{␊
"checkRequire": true␊
}␊
]␊
`

> Output

`␊
Expand Down
Binary file modified test/snapshots/prefer-node-protocol.mjs.snap
Binary file not shown.