Skip to content

Commit 41548c4

Browse files
fiskerfisker
authoredJan 24, 2025··
no-instanceof-builtin-object: Rename to no-instanceof-builtins and use suggestions for unsafe cases (#2537)
·
v58.0.0v57.0.0
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 ‎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 ‎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 ‎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.

‎test/snapshots/no-instanceof-builtin-object.js.md ‎test/snapshots/no-instanceof-builtins.js.md

+42-62
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
# Snapshot report for `test/no-instanceof-builtin-object.js`
1+
# Snapshot report for `test/no-instanceof-builtins.js`
22

3-
The actual snapshot is saved in `no-instanceof-builtin-object.js.snap`.
3+
The actual snapshot is saved in `no-instanceof-builtins.js.snap`.
44

55
Generated by [AVA](https://avajs.dev).
66

@@ -12,17 +12,15 @@ Generated by [AVA](https://avajs.dev).
1212
1 | foo instanceof String␊
1313
`
1414

15-
> Output
16-
17-
`␊
18-
1 | typeof foo === 'string'␊
19-
`
20-
2115
> Error 1/1
2216
2317
`␊
2418
> 1 | foo instanceof String␊
2519
| ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
20+
␊
21+
--------------------------------------------------------------------------------␊
22+
Suggestion 1/1: Switch to \`typeof … === 'string'\`.␊
23+
1 | typeof foo === 'string'␊
2624
`
2725

2826
## invalid(2): foo instanceof Number
@@ -33,17 +31,15 @@ Generated by [AVA](https://avajs.dev).
3331
1 | foo instanceof Number␊
3432
`
3533

36-
> Output
37-
38-
`␊
39-
1 | typeof foo === 'number'␊
40-
`
41-
4234
> Error 1/1
4335
4436
`␊
4537
> 1 | foo instanceof Number␊
4638
| ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
39+
␊
40+
--------------------------------------------------------------------------------␊
41+
Suggestion 1/1: Switch to \`typeof … === 'number'\`.␊
42+
1 | typeof foo === 'number'␊
4743
`
4844

4945
## invalid(3): foo instanceof Boolean
@@ -54,17 +50,15 @@ Generated by [AVA](https://avajs.dev).
5450
1 | foo instanceof Boolean␊
5551
`
5652

57-
> Output
58-
59-
`␊
60-
1 | typeof foo === 'boolean'␊
61-
`
62-
6353
> Error 1/1
6454
6555
`␊
6656
> 1 | foo instanceof Boolean␊
6757
| ^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
58+
␊
59+
--------------------------------------------------------------------------------␊
60+
Suggestion 1/1: Switch to \`typeof … === 'boolean'\`.␊
61+
1 | typeof foo === 'boolean'␊
6862
`
6963

7064
## invalid(4): foo instanceof BigInt
@@ -75,17 +69,15 @@ Generated by [AVA](https://avajs.dev).
7569
1 | foo instanceof BigInt␊
7670
`
7771

78-
> Output
79-
80-
`␊
81-
1 | typeof foo === 'bigint'␊
82-
`
83-
8472
> Error 1/1
8573
8674
`␊
8775
> 1 | foo instanceof BigInt␊
8876
| ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
77+
␊
78+
--------------------------------------------------------------------------------␊
79+
Suggestion 1/1: Switch to \`typeof … === 'bigint'\`.␊
80+
1 | typeof foo === 'bigint'␊
8981
`
9082

9183
## invalid(5): foo instanceof Symbol
@@ -96,17 +88,15 @@ Generated by [AVA](https://avajs.dev).
9688
1 | foo instanceof Symbol␊
9789
`
9890

99-
> Output
100-
101-
`␊
102-
1 | typeof foo === 'symbol'␊
103-
`
104-
10591
> Error 1/1
10692
10793
`␊
10894
> 1 | foo instanceof Symbol␊
10995
| ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
96+
␊
97+
--------------------------------------------------------------------------------␊
98+
Suggestion 1/1: Switch to \`typeof … === 'symbol'\`.␊
99+
1 | typeof foo === 'symbol'␊
110100
`
111101

112102
## invalid(6): foo instanceof Function
@@ -169,17 +159,15 @@ Generated by [AVA](https://avajs.dev).
169159
]␊
170160
`
171161

172-
> Output
173-
174-
`␊
175-
1 | typeof fooStrict === 'string'␊
176-
`
177-
178162
> Error 1/1
179163
180164
`␊
181165
> 1 | fooStrict instanceof String␊
182166
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
167+
␊
168+
--------------------------------------------------------------------------------␊
169+
Suggestion 1/1: Switch to \`typeof … === 'string'\`.␊
170+
1 | typeof fooStrict === 'string'␊
183171
`
184172

185173
## invalid(2): fooStrict instanceof Number
@@ -200,17 +188,15 @@ Generated by [AVA](https://avajs.dev).
200188
]␊
201189
`
202190

203-
> Output
204-
205-
`␊
206-
1 | typeof fooStrict === 'number'␊
207-
`
208-
209191
> Error 1/1
210192
211193
`␊
212194
> 1 | fooStrict instanceof Number␊
213195
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
196+
␊
197+
--------------------------------------------------------------------------------␊
198+
Suggestion 1/1: Switch to \`typeof … === 'number'\`.␊
199+
1 | typeof fooStrict === 'number'␊
214200
`
215201

216202
## invalid(3): fooStrict instanceof Boolean
@@ -231,17 +217,15 @@ Generated by [AVA](https://avajs.dev).
231217
]␊
232218
`
233219

234-
> Output
235-
236-
`␊
237-
1 | typeof fooStrict === 'boolean'␊
238-
`
239-
240220
> Error 1/1
241221
242222
`␊
243223
> 1 | fooStrict instanceof Boolean␊
244224
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
225+
␊
226+
--------------------------------------------------------------------------------␊
227+
Suggestion 1/1: Switch to \`typeof … === 'boolean'\`.␊
228+
1 | typeof fooStrict === 'boolean'␊
245229
`
246230

247231
## invalid(4): fooStrict instanceof BigInt
@@ -262,17 +246,15 @@ Generated by [AVA](https://avajs.dev).
262246
]␊
263247
`
264248

265-
> Output
266-
267-
`␊
268-
1 | typeof fooStrict === 'bigint'␊
269-
`
270-
271249
> Error 1/1
272250
273251
`␊
274252
> 1 | fooStrict instanceof BigInt␊
275253
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
254+
␊
255+
--------------------------------------------------------------------------------␊
256+
Suggestion 1/1: Switch to \`typeof … === 'bigint'\`.␊
257+
1 | typeof fooStrict === 'bigint'␊
276258
`
277259

278260
## invalid(5): fooStrict instanceof Symbol
@@ -293,17 +275,15 @@ Generated by [AVA](https://avajs.dev).
293275
]␊
294276
`
295277

296-
> Output
297-
298-
`␊
299-
1 | typeof fooStrict === 'symbol'␊
300-
`
301-
302278
> Error 1/1
303279
304280
`␊
305281
> 1 | fooStrict instanceof Symbol␊
306282
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊
283+
␊
284+
--------------------------------------------------------------------------------␊
285+
Suggestion 1/1: Switch to \`typeof … === 'symbol'\`.␊
286+
1 | typeof fooStrict === 'symbol'␊
307287
`
308288

309289
## invalid(6): fooStrict instanceof Function

‎test/snapshots/no-instanceof-builtins.js.snap

3.41 KB
Binary file not shown.

0 commit comments

Comments
 (0)
Please sign in to comment.