Skip to content

Commit 41548c4

Browse files
fiskerfisker
authored
no-instanceof-builtin-object: Rename to no-instanceof-builtins and use suggestions for unsafe cases (#2537)
1 parent ed8da1b commit 41548c4

13 files changed

+85
-90
lines changed

‎docs/deprecated-rules.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ This rule was renamed to [`no-array-callback-reference`](rules/no-array-callback
1414

1515
## no-instanceof-array
1616

17-
Replaced by [`no-instanceof-builtin-object`](rules/no-instanceof-builtin-object) which covers more cases.
17+
Replaced by [`no-instanceof-builtins`](rules/no-instanceof-builtins.md) which covers more cases.
1818

1919
## no-reduce
2020

‎docs/rules/no-instanceof-builtin-object.md renamed to ‎docs/rules/no-instanceof-builtins.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).
44

5-
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
5+
🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions).
66

77
<!-- end auto-generated rule header -->
88
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
@@ -73,7 +73,7 @@ The matching strategy:
7373
- `'strict'` - Matches all built-in constructors.
7474

7575
```js
76-
"unicorn/no-instanceof-builtin-object": [
76+
"unicorn/no-instanceof-builtins": [
7777
"error",
7878
{
7979
"strategy": "strict"
@@ -89,7 +89,7 @@ Default: `[]`
8989
Specify the constructors that should be validated.
9090

9191
```js
92-
"unicorn/no-instanceof-builtin-object": [
92+
"unicorn/no-instanceof-builtins": [
9393
"error",
9494
{
9595
"include": [
@@ -108,7 +108,7 @@ Default: `[]`
108108
Specifies the constructors that should be excluded, with this rule taking precedence over others.
109109

110110
```js
111-
"unicorn/no-instanceof-builtin-object": [
111+
"unicorn/no-instanceof-builtins": [
112112
"error",
113113
{
114114
"exclude": [
@@ -127,7 +127,7 @@ Default: `false`
127127
Specifies using [`Error.isError()`](https://github.com/tc39/proposal-is-error) to determine whether it is an error object.
128128

129129
```js
130-
"unicorn/no-instanceof-builtin-object": [
130+
"unicorn/no-instanceof-builtins": [
131131
"error",
132132
{
133133
"strategy": "strict",

‎index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import packageJson from './package.json' with {type: 'json'};
55

66
const deprecatedRules = createDeprecatedRules({
77
// {ruleId: ReplacementRuleId | ReplacementRuleId[]}, if no replacement, use `{ruleId: []}`
8-
'no-instanceof-array': 'unicorn/no-instanceof-builtin-object',
8+
'no-instanceof-array': 'unicorn/no-instanceof-builtins',
99
});
1010

1111
const externalRules = {

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
"lint:js": "eslint",
3535
"lint:markdown": "markdownlint \"**/*.md\"",
3636
"lint:package-json": "npmPkgJsonLint .",
37+
"rename-rule": "node ./scripts/rename-rule.js && npm run create-rules-index-file && npm run fix:eslint-docs",
3738
"run-rules-on-codebase": "eslint --config=./eslint.dogfooding.config.js",
3839
"smoke": "eslint-remote-tester --config ./test/smoke/eslint-remote-tester.config.js",
3940
"test": "npm-run-all --continue-on-error lint test:*",

‎readme.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export default [
8787
| [no-empty-file](docs/rules/no-empty-file.md) | Disallow empty files. | ✅ | | |
8888
| [no-for-loop](docs/rules/no-for-loop.md) | Do not use a `for` loop that can be replaced with a `for-of` loop. | ✅ | 🔧 | 💡 |
8989
| [no-hex-escape](docs/rules/no-hex-escape.md) | Enforce the use of Unicode escapes instead of hexadecimal escapes. | ✅ | 🔧 | |
90-
| [no-instanceof-builtin-object](docs/rules/no-instanceof-builtin-object.md) | Disallow `instanceof` with built-in objects | ✅ | 🔧 | |
90+
| [no-instanceof-builtins](docs/rules/no-instanceof-builtins.md) | Disallow `instanceof` with built-in objects | ✅ | 🔧 | 💡 |
9191
| [no-invalid-fetch-options](docs/rules/no-invalid-fetch-options.md) | Disallow invalid options in `fetch()` and `new Request()`. | ✅ | | |
9292
| [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. | ✅ | | |
9393
| [no-keyword-prefix](docs/rules/no-keyword-prefix.md) | Disallow identifiers starting with `new` or `class`. | | | |

‎rules/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import noDocumentCookie from './no-document-cookie.js';
3232
import noEmptyFile from './no-empty-file.js';
3333
import noForLoop from './no-for-loop.js';
3434
import noHexEscape from './no-hex-escape.js';
35-
import noInstanceofBuiltinObject from './no-instanceof-builtin-object.js';
35+
import noInstanceofBuiltins from './no-instanceof-builtins.js';
3636
import noInvalidFetchOptions from './no-invalid-fetch-options.js';
3737
import noInvalidRemoveEventListener from './no-invalid-remove-event-listener.js';
3838
import noKeywordPrefix from './no-keyword-prefix.js';
@@ -160,7 +160,7 @@ const rules = {
160160
'no-empty-file': createRule(noEmptyFile, 'no-empty-file'),
161161
'no-for-loop': createRule(noForLoop, 'no-for-loop'),
162162
'no-hex-escape': createRule(noHexEscape, 'no-hex-escape'),
163-
'no-instanceof-builtin-object': createRule(noInstanceofBuiltinObject, 'no-instanceof-builtin-object'),
163+
'no-instanceof-builtins': createRule(noInstanceofBuiltins, 'no-instanceof-builtins'),
164164
'no-invalid-fetch-options': createRule(noInvalidFetchOptions, 'no-invalid-fetch-options'),
165165
'no-invalid-remove-event-listener': createRule(noInvalidRemoveEventListener, 'no-invalid-remove-event-listener'),
166166
'no-keyword-prefix': createRule(noKeywordPrefix, 'no-keyword-prefix'),

‎rules/no-instanceof-builtin-object.js renamed to ‎rules/no-instanceof-builtins.js

+30-16
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,22 @@ import typedArray from './shared/typed-array.js';
66

77
const isInstanceofToken = token => token.value === 'instanceof' && token.type === 'Keyword';
88

9-
const MESSAGE_ID = 'no-instanceof-builtin-object';
9+
const MESSAGE_ID = 'no-instanceof-builtins';
10+
const MESSAGE_ID_SWITCH_TO_TYPE_OF = 'switch-to-type-of';
1011
const messages = {
1112
[MESSAGE_ID]: 'Avoid using `instanceof` for type checking as it can lead to unreliable results.',
13+
[MESSAGE_ID_SWITCH_TO_TYPE_OF]: 'Switch to `typeof … === \'{{type}}\'`.',
1214
};
1315

14-
const looseStrategyConstructors = new Set([
16+
const primitiveWrappers = new Set([
1517
'String',
1618
'Number',
1719
'Boolean',
1820
'BigInt',
1921
'Symbol',
20-
'Array',
21-
'Function',
2222
]);
2323

24-
const strictStrategyConstructors = new Set([
24+
const strictStrategyConstructors = [
2525
// Error types
2626
...builtinErrors,
2727

@@ -51,7 +51,7 @@ const strictStrategyConstructors = new Set([
5151
'Date',
5252
'SharedArrayBuffer',
5353
'FinalizationRegistry',
54-
]);
54+
];
5555

5656
const replaceWithFunctionCall = (node, sourceCode, functionName) => function * (fixer) {
5757
const {tokenStore, instanceofToken} = getInstanceOfToken(sourceCode, node);
@@ -111,6 +111,12 @@ const create = context => {
111111
exclude = [],
112112
} = context.options[0] ?? {};
113113

114+
const forbiddenConstructors = new Set(
115+
strategy === 'strict'
116+
? [...strictStrategyConstructors, ...include]
117+
: include,
118+
);
119+
114120
const {sourceCode} = context;
115121

116122
return {
@@ -122,9 +128,7 @@ const create = context => {
122128
return;
123129
}
124130

125-
if (!looseStrategyConstructors.has(right.name) && !strictStrategyConstructors.has(right.name) && !include.includes(right.name)) {
126-
return;
127-
}
131+
const constructorName = right.name;
128132

129133
/** @type {import('eslint').Rule.ReportDescriptor} */
130134
const problem = {
@@ -133,22 +137,31 @@ const create = context => {
133137
};
134138

135139
if (
136-
right.name === 'Array'
137-
|| (right.name === 'Error' && useErrorIsError)
140+
constructorName === 'Array'
141+
|| (constructorName === 'Error' && useErrorIsError)
138142
) {
139-
const functionName = right.name === 'Array' ? 'Array.isArray' : 'Error.isError';
143+
const functionName = constructorName === 'Array' ? 'Array.isArray' : 'Error.isError';
140144
problem.fix = replaceWithFunctionCall(node, sourceCode, functionName);
141145
return problem;
142146
}
143147

144-
// Loose strategy by default
145-
if (looseStrategyConstructors.has(right.name)) {
148+
if (constructorName === 'Function') {
146149
problem.fix = replaceWithTypeOfExpression(node, sourceCode);
147150
return problem;
148151
}
149152

150-
// Strict strategy
151-
if (strategy !== 'strict' && include.length === 0) {
153+
if (primitiveWrappers.has(constructorName)) {
154+
problem.suggest = [
155+
{
156+
messageId: MESSAGE_ID_SWITCH_TO_TYPE_OF,
157+
data: {type: constructorName.toLowerCase()},
158+
fix: replaceWithTypeOfExpression(node, sourceCode),
159+
},
160+
];
161+
return problem;
162+
}
163+
164+
if (!forbiddenConstructors.has(constructorName)) {
152165
return;
153166
}
154167

@@ -204,6 +217,7 @@ const config = {
204217
include: [],
205218
exclude: [],
206219
}],
220+
hasSuggestions: true,
207221
messages,
208222
},
209223
};

‎scripts/rename-rule.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ const run = async () => {
7575
return;
7676
}
7777

78-
if (Reflect.has(rules, ruleId)) {
78+
if (rules.includes(ruleId)) {
7979
console.log(`${ruleId} already exists.`);
8080
return;
8181
}

‎test/no-instanceof-builtin-object.js renamed to ‎test/no-instanceof-builtins.js

File renamed without changes.

‎test/package.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
3333
'prefer-math-min-max',
3434
'consistent-existence-index-check',
3535
'prefer-global-this',
36-
'no-instanceof-builtin-object',
36+
'no-instanceof-builtins',
3737
'no-named-default',
3838
'consistent-assert',
3939
'no-accessor-recursion',

‎test/snapshots/no-instanceof-builtin-object.js.snap

-3.32 KB
Binary file not shown.

0 commit comments

Comments
 (0)