diff --git a/docs/deprecated-rules.md b/docs/deprecated-rules.md index 5358b2bcd2..37b02d0c10 100644 --- a/docs/deprecated-rules.md +++ b/docs/deprecated-rules.md @@ -14,7 +14,7 @@ This rule was renamed to [`no-array-callback-reference`](rules/no-array-callback ## no-instanceof-array -Replaced by [`no-instanceof-builtin-object`](rules/no-instanceof-builtin-object) which covers more cases. +Replaced by [`no-instanceof-builtins`](rules/no-instanceof-builtins.md) which covers more cases. ## no-reduce diff --git a/docs/rules/no-instanceof-builtin-object.md b/docs/rules/no-instanceof-builtins.md similarity index 86% rename from docs/rules/no-instanceof-builtin-object.md rename to docs/rules/no-instanceof-builtins.md index 8aecfaf3d0..63f1fdb8eb 100644 --- a/docs/rules/no-instanceof-builtin-object.md +++ b/docs/rules/no-instanceof-builtins.md @@ -2,7 +2,7 @@ πŸ’Ό This rule is enabled in the βœ… `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs). -πŸ”§ This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). +πŸ”§πŸ’‘ 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). @@ -73,7 +73,7 @@ The matching strategy: - `'strict'` - Matches all built-in constructors. ```js -"unicorn/no-instanceof-builtin-object": [ +"unicorn/no-instanceof-builtins": [ "error", { "strategy": "strict" @@ -89,7 +89,7 @@ Default: `[]` Specify the constructors that should be validated. ```js -"unicorn/no-instanceof-builtin-object": [ +"unicorn/no-instanceof-builtins": [ "error", { "include": [ @@ -108,7 +108,7 @@ Default: `[]` Specifies the constructors that should be excluded, with this rule taking precedence over others. ```js -"unicorn/no-instanceof-builtin-object": [ +"unicorn/no-instanceof-builtins": [ "error", { "exclude": [ @@ -127,7 +127,7 @@ Default: `false` Specifies using [`Error.isError()`](https://github.com/tc39/proposal-is-error) to determine whether it is an error object. ```js -"unicorn/no-instanceof-builtin-object": [ +"unicorn/no-instanceof-builtins": [ "error", { "strategy": "strict", diff --git a/index.js b/index.js index e2495051e5..ecb8de01d9 100644 --- a/index.js +++ b/index.js @@ -5,7 +5,7 @@ import packageJson from './package.json' with {type: 'json'}; const deprecatedRules = createDeprecatedRules({ // {ruleId: ReplacementRuleId | ReplacementRuleId[]}, if no replacement, use `{ruleId: []}` - 'no-instanceof-array': 'unicorn/no-instanceof-builtin-object', + 'no-instanceof-array': 'unicorn/no-instanceof-builtins', }); const externalRules = { diff --git a/package.json b/package.json index b2a1f32b4d..760ad0424e 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "lint:js": "eslint", "lint:markdown": "markdownlint \"**/*.md\"", "lint:package-json": "npmPkgJsonLint .", + "rename-rule": "node ./scripts/rename-rule.js && npm run create-rules-index-file && npm run fix:eslint-docs", "run-rules-on-codebase": "eslint --config=./eslint.dogfooding.config.js", "smoke": "eslint-remote-tester --config ./test/smoke/eslint-remote-tester.config.js", "test": "npm-run-all --continue-on-error lint test:*", diff --git a/readme.md b/readme.md index 85aa8cf6e5..31d64fc49c 100644 --- a/readme.md +++ b/readme.md @@ -87,7 +87,7 @@ export default [ | [no-empty-file](docs/rules/no-empty-file.md) | Disallow empty files. | βœ… | | | | [no-for-loop](docs/rules/no-for-loop.md) | Do not use a `for` loop that can be replaced with a `for-of` loop. | βœ… | πŸ”§ | πŸ’‘ | | [no-hex-escape](docs/rules/no-hex-escape.md) | Enforce the use of Unicode escapes instead of hexadecimal escapes. | βœ… | πŸ”§ | | -| [no-instanceof-builtin-object](docs/rules/no-instanceof-builtin-object.md) | Disallow `instanceof` with built-in objects | βœ… | πŸ”§ | | +| [no-instanceof-builtins](docs/rules/no-instanceof-builtins.md) | Disallow `instanceof` with built-in objects | βœ… | πŸ”§ | πŸ’‘ | | [no-invalid-fetch-options](docs/rules/no-invalid-fetch-options.md) | Disallow invalid options in `fetch()` and `new Request()`. | βœ… | | | | [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. | βœ… | | | | [no-keyword-prefix](docs/rules/no-keyword-prefix.md) | Disallow identifiers starting with `new` or `class`. | | | | diff --git a/rules/index.js b/rules/index.js index 2ceca7cb8d..3bc1ce8c44 100644 --- a/rules/index.js +++ b/rules/index.js @@ -32,7 +32,7 @@ import noDocumentCookie from './no-document-cookie.js'; import noEmptyFile from './no-empty-file.js'; import noForLoop from './no-for-loop.js'; import noHexEscape from './no-hex-escape.js'; -import noInstanceofBuiltinObject from './no-instanceof-builtin-object.js'; +import noInstanceofBuiltins from './no-instanceof-builtins.js'; import noInvalidFetchOptions from './no-invalid-fetch-options.js'; import noInvalidRemoveEventListener from './no-invalid-remove-event-listener.js'; import noKeywordPrefix from './no-keyword-prefix.js'; @@ -160,7 +160,7 @@ const rules = { 'no-empty-file': createRule(noEmptyFile, 'no-empty-file'), 'no-for-loop': createRule(noForLoop, 'no-for-loop'), 'no-hex-escape': createRule(noHexEscape, 'no-hex-escape'), - 'no-instanceof-builtin-object': createRule(noInstanceofBuiltinObject, 'no-instanceof-builtin-object'), + 'no-instanceof-builtins': createRule(noInstanceofBuiltins, 'no-instanceof-builtins'), 'no-invalid-fetch-options': createRule(noInvalidFetchOptions, 'no-invalid-fetch-options'), 'no-invalid-remove-event-listener': createRule(noInvalidRemoveEventListener, 'no-invalid-remove-event-listener'), 'no-keyword-prefix': createRule(noKeywordPrefix, 'no-keyword-prefix'), diff --git a/rules/no-instanceof-builtin-object.js b/rules/no-instanceof-builtins.js similarity index 82% rename from rules/no-instanceof-builtin-object.js rename to rules/no-instanceof-builtins.js index e1701ec72b..5e915aef3b 100644 --- a/rules/no-instanceof-builtin-object.js +++ b/rules/no-instanceof-builtins.js @@ -6,22 +6,22 @@ import typedArray from './shared/typed-array.js'; const isInstanceofToken = token => token.value === 'instanceof' && token.type === 'Keyword'; -const MESSAGE_ID = 'no-instanceof-builtin-object'; +const MESSAGE_ID = 'no-instanceof-builtins'; +const MESSAGE_ID_SWITCH_TO_TYPE_OF = 'switch-to-type-of'; const messages = { [MESSAGE_ID]: 'Avoid using `instanceof` for type checking as it can lead to unreliable results.', + [MESSAGE_ID_SWITCH_TO_TYPE_OF]: 'Switch to `typeof … === \'{{type}}\'`.', }; -const looseStrategyConstructors = new Set([ +const primitiveWrappers = new Set([ 'String', 'Number', 'Boolean', 'BigInt', 'Symbol', - 'Array', - 'Function', ]); -const strictStrategyConstructors = new Set([ +const strictStrategyConstructors = [ // Error types ...builtinErrors, @@ -51,7 +51,7 @@ const strictStrategyConstructors = new Set([ 'Date', 'SharedArrayBuffer', 'FinalizationRegistry', -]); +]; const replaceWithFunctionCall = (node, sourceCode, functionName) => function * (fixer) { const {tokenStore, instanceofToken} = getInstanceOfToken(sourceCode, node); @@ -111,6 +111,12 @@ const create = context => { exclude = [], } = context.options[0] ?? {}; + const forbiddenConstructors = new Set( + strategy === 'strict' + ? [...strictStrategyConstructors, ...include] + : include, + ); + const {sourceCode} = context; return { @@ -122,9 +128,7 @@ const create = context => { return; } - if (!looseStrategyConstructors.has(right.name) && !strictStrategyConstructors.has(right.name) && !include.includes(right.name)) { - return; - } + const constructorName = right.name; /** @type {import('eslint').Rule.ReportDescriptor} */ const problem = { @@ -133,22 +137,31 @@ const create = context => { }; if ( - right.name === 'Array' - || (right.name === 'Error' && useErrorIsError) + constructorName === 'Array' + || (constructorName === 'Error' && useErrorIsError) ) { - const functionName = right.name === 'Array' ? 'Array.isArray' : 'Error.isError'; + const functionName = constructorName === 'Array' ? 'Array.isArray' : 'Error.isError'; problem.fix = replaceWithFunctionCall(node, sourceCode, functionName); return problem; } - // Loose strategy by default - if (looseStrategyConstructors.has(right.name)) { + if (constructorName === 'Function') { problem.fix = replaceWithTypeOfExpression(node, sourceCode); return problem; } - // Strict strategy - if (strategy !== 'strict' && include.length === 0) { + if (primitiveWrappers.has(constructorName)) { + problem.suggest = [ + { + messageId: MESSAGE_ID_SWITCH_TO_TYPE_OF, + data: {type: constructorName.toLowerCase()}, + fix: replaceWithTypeOfExpression(node, sourceCode), + }, + ]; + return problem; + } + + if (!forbiddenConstructors.has(constructorName)) { return; } @@ -204,6 +217,7 @@ const config = { include: [], exclude: [], }], + hasSuggestions: true, messages, }, }; diff --git a/scripts/rename-rule.js b/scripts/rename-rule.js index e1f313f280..c25d3caad2 100644 --- a/scripts/rename-rule.js +++ b/scripts/rename-rule.js @@ -75,7 +75,7 @@ const run = async () => { return; } - if (Reflect.has(rules, ruleId)) { + if (rules.includes(ruleId)) { console.log(`${ruleId} already exists.`); return; } diff --git a/test/no-instanceof-builtin-object.js b/test/no-instanceof-builtins.js similarity index 100% rename from test/no-instanceof-builtin-object.js rename to test/no-instanceof-builtins.js diff --git a/test/package.js b/test/package.js index 634c036baa..875b8bc6ab 100644 --- a/test/package.js +++ b/test/package.js @@ -33,7 +33,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([ 'prefer-math-min-max', 'consistent-existence-index-check', 'prefer-global-this', - 'no-instanceof-builtin-object', + 'no-instanceof-builtins', 'no-named-default', 'consistent-assert', 'no-accessor-recursion', diff --git a/test/snapshots/no-instanceof-builtin-object.js.snap b/test/snapshots/no-instanceof-builtin-object.js.snap deleted file mode 100644 index e304731776..0000000000 Binary files a/test/snapshots/no-instanceof-builtin-object.js.snap and /dev/null differ diff --git a/test/snapshots/no-instanceof-builtin-object.js.md b/test/snapshots/no-instanceof-builtins.js.md similarity index 95% rename from test/snapshots/no-instanceof-builtin-object.js.md rename to test/snapshots/no-instanceof-builtins.js.md index 2bf31be5f2..529e47ae83 100644 --- a/test/snapshots/no-instanceof-builtin-object.js.md +++ b/test/snapshots/no-instanceof-builtins.js.md @@ -1,6 +1,6 @@ -# Snapshot report for `test/no-instanceof-builtin-object.js` +# Snapshot report for `test/no-instanceof-builtins.js` -The actual snapshot is saved in `no-instanceof-builtin-object.js.snap`. +The actual snapshot is saved in `no-instanceof-builtins.js.snap`. Generated by [AVA](https://avajs.dev). @@ -12,17 +12,15 @@ Generated by [AVA](https://avajs.dev). 1 | foo instanceof String␊ ` -> Output - - `␊ - 1 | typeof foo === 'string'␊ - ` - > Error 1/1 `␊ > 1 | foo instanceof String␊ | ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'string'\`.␊ + 1 | typeof foo === 'string'␊ ` ## invalid(2): foo instanceof Number @@ -33,17 +31,15 @@ Generated by [AVA](https://avajs.dev). 1 | foo instanceof Number␊ ` -> Output - - `␊ - 1 | typeof foo === 'number'␊ - ` - > Error 1/1 `␊ > 1 | foo instanceof Number␊ | ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'number'\`.␊ + 1 | typeof foo === 'number'␊ ` ## invalid(3): foo instanceof Boolean @@ -54,17 +50,15 @@ Generated by [AVA](https://avajs.dev). 1 | foo instanceof Boolean␊ ` -> Output - - `␊ - 1 | typeof foo === 'boolean'␊ - ` - > Error 1/1 `␊ > 1 | foo instanceof Boolean␊ | ^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'boolean'\`.␊ + 1 | typeof foo === 'boolean'␊ ` ## invalid(4): foo instanceof BigInt @@ -75,17 +69,15 @@ Generated by [AVA](https://avajs.dev). 1 | foo instanceof BigInt␊ ` -> Output - - `␊ - 1 | typeof foo === 'bigint'␊ - ` - > Error 1/1 `␊ > 1 | foo instanceof BigInt␊ | ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'bigint'\`.␊ + 1 | typeof foo === 'bigint'␊ ` ## invalid(5): foo instanceof Symbol @@ -96,17 +88,15 @@ Generated by [AVA](https://avajs.dev). 1 | foo instanceof Symbol␊ ` -> Output - - `␊ - 1 | typeof foo === 'symbol'␊ - ` - > Error 1/1 `␊ > 1 | foo instanceof Symbol␊ | ^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'symbol'\`.␊ + 1 | typeof foo === 'symbol'␊ ` ## invalid(6): foo instanceof Function @@ -169,17 +159,15 @@ Generated by [AVA](https://avajs.dev). ]␊ ` -> Output - - `␊ - 1 | typeof fooStrict === 'string'␊ - ` - > Error 1/1 `␊ > 1 | fooStrict instanceof String␊ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'string'\`.␊ + 1 | typeof fooStrict === 'string'␊ ` ## invalid(2): fooStrict instanceof Number @@ -200,17 +188,15 @@ Generated by [AVA](https://avajs.dev). ]␊ ` -> Output - - `␊ - 1 | typeof fooStrict === 'number'␊ - ` - > Error 1/1 `␊ > 1 | fooStrict instanceof Number␊ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'number'\`.␊ + 1 | typeof fooStrict === 'number'␊ ` ## invalid(3): fooStrict instanceof Boolean @@ -231,17 +217,15 @@ Generated by [AVA](https://avajs.dev). ]␊ ` -> Output - - `␊ - 1 | typeof fooStrict === 'boolean'␊ - ` - > Error 1/1 `␊ > 1 | fooStrict instanceof Boolean␊ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'boolean'\`.␊ + 1 | typeof fooStrict === 'boolean'␊ ` ## invalid(4): fooStrict instanceof BigInt @@ -262,17 +246,15 @@ Generated by [AVA](https://avajs.dev). ]␊ ` -> Output - - `␊ - 1 | typeof fooStrict === 'bigint'␊ - ` - > Error 1/1 `␊ > 1 | fooStrict instanceof BigInt␊ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'bigint'\`.␊ + 1 | typeof fooStrict === 'bigint'␊ ` ## invalid(5): fooStrict instanceof Symbol @@ -293,17 +275,15 @@ Generated by [AVA](https://avajs.dev). ]␊ ` -> Output - - `␊ - 1 | typeof fooStrict === 'symbol'␊ - ` - > Error 1/1 `␊ > 1 | fooStrict instanceof Symbol␊ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid using \`instanceof\` for type checking as it can lead to unreliable results.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`typeof … === 'symbol'\`.␊ + 1 | typeof fooStrict === 'symbol'␊ ` ## invalid(6): fooStrict instanceof Function diff --git a/test/snapshots/no-instanceof-builtins.js.snap b/test/snapshots/no-instanceof-builtins.js.snap new file mode 100644 index 0000000000..b719aab735 Binary files /dev/null and b/test/snapshots/no-instanceof-builtins.js.snap differ