diff --git a/configs/recommended.js b/configs/recommended.js index 158d265bdf..7196c57a90 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -52,6 +52,7 @@ module.exports = { 'unicorn/no-static-only-class': 'error', 'unicorn/no-thenable': 'error', 'unicorn/no-this-assignment': 'error', + 'unicorn/no-typeof-undefined': 'error', 'unicorn/no-unnecessary-await': 'error', 'unicorn/no-unreadable-array-destructuring': 'error', 'unicorn/no-unreadable-iife': 'error', diff --git a/docs/rules/no-typeof-undefined.md b/docs/rules/no-typeof-undefined.md new file mode 100644 index 0000000000..a0d99effe1 --- /dev/null +++ b/docs/rules/no-typeof-undefined.md @@ -0,0 +1,59 @@ +# Disallow comparing `undefined` using `typeof` + +✅ This rule is enabled in the `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs). + +🔧💡 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/developer-guide/working-with-rules#providing-suggestions). + + + + +Checking if a value is `undefined` by using `typeof value === 'undefined'` is needlessly verbose. It's generally better to compare against `undefined` directly. The only time `typeof` is needed is when a global variable potentially does not exists, in which case, using `globalThis.value === undefined` may be better. + +## Fail + +```js +function foo(bar) { + if (typeof bar === 'undefined') {} +} +``` + +```js +import foo from './foo.js'; + +if (typeof foo.bar !== 'undefined') {} +``` + +## Pass + +```js +function foo(bar) { + if (foo === undefined) {} +} +``` + +```js +import foo from './foo.js'; + +if (foo.bar !== undefined) {} +``` + +## Options + +### checkGlobalVariables + +Type: `boolean`\ +Default: `false` + +The rule ignores variables not defined in the file by default. + +Set it to `true` to check all variables. + +```js +// eslint unicorn/no-typeof-undefined: ["error", {"checkGlobalVariables": true}] +if (typeof undefinedVariable === 'undefined') {} // Fails +``` + +```js +// eslint unicorn/no-typeof-undefined: ["error", {"checkGlobalVariables": true}] +if (typeof Array === 'undefined') {} // Fails +``` diff --git a/readme.md b/readme.md index ea819880ce..76e8eb26d4 100644 --- a/readme.md +++ b/readme.md @@ -91,6 +91,7 @@ Use a [preset config](#preset-configs) or configure each rules in `package.json` | [no-static-only-class](docs/rules/no-static-only-class.md) | Disallow classes that only have static members. | ✅ | 🔧 | | | [no-thenable](docs/rules/no-thenable.md) | Disallow `then` property. | ✅ | | | | [no-this-assignment](docs/rules/no-this-assignment.md) | Disallow assigning `this` to a variable. | ✅ | | | +| [no-typeof-undefined](docs/rules/no-typeof-undefined.md) | Disallow comparing `undefined` using `typeof`. | ✅ | 🔧 | 💡 | | [no-unnecessary-await](docs/rules/no-unnecessary-await.md) | Disallow awaiting non-promise values. | ✅ | 🔧 | | | [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ | 🔧 | | | [no-unreadable-iife](docs/rules/no-unreadable-iife.md) | Disallow unreadable IIFEs. | ✅ | | | diff --git a/rules/no-static-only-class.js b/rules/no-static-only-class.js index 89268ec7f3..4fdcb932fb 100644 --- a/rules/no-static-only-class.js +++ b/rules/no-static-only-class.js @@ -49,7 +49,7 @@ function isStaticMember(node) { if ( isDeclare || isReadonly - || typeof accessibility !== 'undefined' + || accessibility !== undefined || (Array.isArray(decorators) && decorators.length > 0) // TODO: Remove this when we drop support for `@typescript-eslint/parser` v4 || key.type === 'TSPrivateIdentifier' diff --git a/rules/no-typeof-undefined.js b/rules/no-typeof-undefined.js new file mode 100644 index 0000000000..3ad5e6ef55 --- /dev/null +++ b/rules/no-typeof-undefined.js @@ -0,0 +1,140 @@ +'use strict'; +const isShadowed = require('./utils/is-shadowed.js'); +const {matches} = require('./selectors/index.js'); +const { + addParenthesizesToReturnOrThrowExpression, +} = require('./fix/index.js'); +const {removeSpacesAfter} = require('./fix/index.js'); +const isOnSameLine = require('./utils/is-on-same-line.js'); +const needsSemicolon = require('./utils/needs-semicolon.js'); +const { + isParenthesized, +} = require('./utils/parentheses.js'); + +const MESSAGE_ID_ERROR = 'no-typeof-undefined/error'; +const MESSAGE_ID_SUGGESTION = 'no-typeof-undefined/suggestion'; +const messages = { + [MESSAGE_ID_ERROR]: 'Compare with `undefined` directly instead of using `typeof`.', + [MESSAGE_ID_SUGGESTION]: 'Switch to `… {{operator}} undefined`.', +}; + +const selector = [ + 'BinaryExpression', + matches(['===', '!==', '==', '!='].map(operator => `[operator="${operator}"]`)), + '[left.type="UnaryExpression"]', + '[left.operator="typeof"]', + '[left.prefix]', + '[right.type="Literal"]', +].join(''); + +/** @param {import('eslint').Rule.RuleContext} context */ +const create = context => { + const { + checkGlobalVariables, + } = { + checkGlobalVariables: false, + ...context.options[0], + }; + + return { + [selector](binaryExpression) { + const {left: typeofNode, right: undefinedString, operator} = binaryExpression; + if (undefinedString.value !== 'undefined') { + return; + } + + const valueNode = typeofNode.argument; + const isGlobalVariable = valueNode.type === 'Identifier' + && !isShadowed(context.getScope(), valueNode); + + if (!checkGlobalVariables && isGlobalVariable) { + return; + } + + const sourceCode = context.getSourceCode(); + const [typeofToken, secondToken] = sourceCode.getFirstTokens(typeofNode, 2); + + const fix = function * (fixer) { + // Change `==`/`!=` to `===`/`!==` + if (operator === '==' || operator === '!=') { + const operatorToken = sourceCode.getTokenAfter( + typeofNode, + token => token.type === 'Punctuator' && token.value === operator, + ); + + yield fixer.insertTextAfter(operatorToken, '='); + } + + yield fixer.replaceText(undefinedString, 'undefined'); + + yield fixer.remove(typeofToken); + yield removeSpacesAfter(typeofToken, sourceCode, fixer); + + const {parent} = binaryExpression; + if ( + (parent.type === 'ReturnStatement' || parent.type === 'ThrowStatement') + && parent.argument === binaryExpression + && !isOnSameLine(typeofToken, secondToken) + && !isParenthesized(binaryExpression, sourceCode) + && !isParenthesized(typeofNode, sourceCode) + ) { + yield * addParenthesizesToReturnOrThrowExpression(fixer, parent, sourceCode); + return; + } + + const tokenBefore = sourceCode.getTokenBefore(binaryExpression); + if (needsSemicolon(tokenBefore, sourceCode, secondToken.value)) { + yield fixer.insertTextBefore(binaryExpression, ';'); + } + }; + + const problem = { + node: binaryExpression, + loc: typeofToken.loc, + messageId: MESSAGE_ID_ERROR, + }; + + if (isGlobalVariable) { + problem.suggest = [ + { + messageId: MESSAGE_ID_SUGGESTION, + data: {operator: operator.startsWith('!') ? '!==' : '==='}, + fix, + }, + ]; + } else { + problem.fix = fix; + } + + return problem; + }, + }; +}; + +const schema = [ + { + type: 'object', + additionalProperties: false, + properties: { + checkGlobalVariables: { + type: 'boolean', + default: false, + }, + }, + }, +]; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + description: 'Disallow comparing `undefined` using `typeof`.', + }, + fixable: 'code', + hasSuggestions: true, + schema, + messages, + }, +}; diff --git a/rules/utils/is-same-reference.js b/rules/utils/is-same-reference.js index 6f388a5372..21a4c80d4d 100644 --- a/rules/utils/is-same-reference.js +++ b/rules/utils/is-same-reference.js @@ -145,7 +145,7 @@ function isSameReference(left, right) { const nameA = getStaticPropertyName(left); // `x.y = x["y"]` - if (typeof nameA !== 'undefined') { + if (nameA !== undefined) { return ( isSameReference(left.object, right.object) && nameA === getStaticPropertyName(right) diff --git a/test/no-typeof-undefined.mjs b/test/no-typeof-undefined.mjs new file mode 100644 index 0000000000..d5895737f8 --- /dev/null +++ b/test/no-typeof-undefined.mjs @@ -0,0 +1,90 @@ +import outdent from 'outdent'; +import {getTester} from './utils/test.mjs'; + +const {test} = getTester(import.meta); + +test.snapshot({ + valid: [ + 'typeof a.b', + 'typeof a.b > "undefined"', + 'a.b === "undefined"', + 'void a.b === "undefined"', + '+a.b === "undefined"', + '++a.b === "undefined"', + 'a.b++ === "undefined"', + 'foo === undefined', + 'typeof a.b === "string"', + 'typeof foo === "undefined"', + 'foo = 2; typeof foo === "undefined"', + '/* globals foo: readonly */ typeof foo === "undefined"', + '/* globals globalThis: readonly */ typeof globalThis === "undefined"', + // Cases we are not checking + '"undefined" === typeof a.b', + 'const UNDEFINED = "undefined"; typeof a.b === UNDEFINED', + 'typeof a.b === `undefined`', + ], + invalid: [ + 'typeof a.b === "undefined"', + 'typeof a.b !== "undefined"', + 'typeof a.b == "undefined"', + 'typeof a.b != "undefined"', + 'typeof a.b == \'undefined\'', + 'let foo; typeof foo === "undefined"', + 'const foo = 1; typeof foo === "undefined"', + 'var foo; typeof foo === "undefined"', + 'var foo; var foo; typeof foo === "undefined"', + 'for (const foo of bar) typeof foo === "undefined";', + outdent` + let foo; + function bar() { + typeof foo === "undefined"; + } + `, + 'function foo() {typeof foo === "undefined"}', + 'function foo(bar) {typeof bar === "undefined"}', + 'function foo({bar}) {typeof bar === "undefined"}', + 'function foo([bar]) {typeof bar === "undefined"}', + 'typeof foo.bar === "undefined"', + outdent` + import foo from 'foo'; + typeof foo.bar === "undefined" + `, + // ASI + outdent` + foo + typeof [] === "undefined"; + `, + outdent` + foo + typeof (a ? b : c) === "undefined"; + `, + outdent` + function a() { + return typeof // comment + a.b === 'undefined'; + } + `, + outdent` + function a() { + return (typeof // ReturnStatement argument is parenthesized + a.b === 'undefined'); + } + `, + outdent` + function a() { + return (typeof // UnaryExpression is parenthesized + a.b) === 'undefined'; + } + `, + ], +}); + +// `checkGlobalVariables: true` +test.snapshot({ + valid: [ + ], + invalid: [ + 'typeof undefinedVariableIdentifier === "undefined"', + 'typeof Array !== "undefined"', + ].map(code => ({code, options: [{checkGlobalVariables: true}]})), +}); diff --git a/test/snapshots/no-typeof-undefined.mjs.md b/test/snapshots/no-typeof-undefined.mjs.md new file mode 100644 index 0000000000..0b0a257788 --- /dev/null +++ b/test/snapshots/no-typeof-undefined.mjs.md @@ -0,0 +1,450 @@ +# Snapshot report for `test/no-typeof-undefined.mjs` + +The actual snapshot is saved in `no-typeof-undefined.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | typeof a.b === "undefined" + +> Output + + `␊ + 1 | a.b === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof a.b === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #2 + 1 | typeof a.b !== "undefined" + +> Output + + `␊ + 1 | a.b !== undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof a.b !== "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #3 + 1 | typeof a.b == "undefined" + +> Output + + `␊ + 1 | a.b === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof a.b == "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #4 + 1 | typeof a.b != "undefined" + +> Output + + `␊ + 1 | a.b !== undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof a.b != "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #5 + 1 | typeof a.b == 'undefined' + +> Output + + `␊ + 1 | a.b === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof a.b == 'undefined'␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #6 + 1 | let foo; typeof foo === "undefined" + +> Output + + `␊ + 1 | let foo; foo === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | let foo; typeof foo === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #7 + 1 | const foo = 1; typeof foo === "undefined" + +> Output + + `␊ + 1 | const foo = 1; foo === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | const foo = 1; typeof foo === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #8 + 1 | var foo; typeof foo === "undefined" + +> Output + + `␊ + 1 | var foo; foo === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | var foo; typeof foo === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #9 + 1 | var foo; var foo; typeof foo === "undefined" + +> Output + + `␊ + 1 | var foo; var foo; foo === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | var foo; var foo; typeof foo === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #10 + 1 | for (const foo of bar) typeof foo === "undefined"; + +> Output + + `␊ + 1 | for (const foo of bar) foo === undefined;␊ + ` + +> Error 1/1 + + `␊ + > 1 | for (const foo of bar) typeof foo === "undefined";␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #11 + 1 | let foo; + 2 | function bar() { + 3 | typeof foo === "undefined"; + 4 | } + +> Output + + `␊ + 1 | let foo;␊ + 2 | function bar() {␊ + 3 | foo === undefined;␊ + 4 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | let foo;␊ + 2 | function bar() {␊ + > 3 | typeof foo === "undefined";␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + 4 | }␊ + ` + +## Invalid #12 + 1 | function foo() {typeof foo === "undefined"} + +> Output + + `␊ + 1 | function foo() {foo === undefined}␊ + ` + +> Error 1/1 + + `␊ + > 1 | function foo() {typeof foo === "undefined"}␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #13 + 1 | function foo(bar) {typeof bar === "undefined"} + +> Output + + `␊ + 1 | function foo(bar) {bar === undefined}␊ + ` + +> Error 1/1 + + `␊ + > 1 | function foo(bar) {typeof bar === "undefined"}␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #14 + 1 | function foo({bar}) {typeof bar === "undefined"} + +> Output + + `␊ + 1 | function foo({bar}) {bar === undefined}␊ + ` + +> Error 1/1 + + `␊ + > 1 | function foo({bar}) {typeof bar === "undefined"}␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #15 + 1 | function foo([bar]) {typeof bar === "undefined"} + +> Output + + `␊ + 1 | function foo([bar]) {bar === undefined}␊ + ` + +> Error 1/1 + + `␊ + > 1 | function foo([bar]) {typeof bar === "undefined"}␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #16 + 1 | typeof foo.bar === "undefined" + +> Output + + `␊ + 1 | foo.bar === undefined␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof foo.bar === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #17 + 1 | import foo from 'foo'; + 2 | typeof foo.bar === "undefined" + +> Output + + `␊ + 1 | import foo from 'foo';␊ + 2 | foo.bar === undefined␊ + ` + +> Error 1/1 + + `␊ + 1 | import foo from 'foo';␊ + > 2 | typeof foo.bar === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #18 + 1 | foo + 2 | typeof [] === "undefined"; + +> Output + + `␊ + 1 | foo␊ + 2 | ;[] === undefined;␊ + ` + +> Error 1/1 + + `␊ + 1 | foo␊ + > 2 | typeof [] === "undefined";␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #19 + 1 | foo + 2 | typeof (a ? b : c) === "undefined"; + +> Output + + `␊ + 1 | foo␊ + 2 | ;(a ? b : c) === undefined;␊ + ` + +> Error 1/1 + + `␊ + 1 | foo␊ + > 2 | typeof (a ? b : c) === "undefined";␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ` + +## Invalid #20 + 1 | function a() { + 2 | return typeof // comment + 3 | a.b === 'undefined'; + 4 | } + +> Output + + `␊ + 1 | function a() {␊ + 2 | return ( // comment␊ + 3 | a.b === undefined);␊ + 4 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function a() {␊ + > 2 | return typeof // comment␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + 3 | a.b === 'undefined';␊ + 4 | }␊ + ` + +## Invalid #21 + 1 | function a() { + 2 | return (typeof // ReturnStatement argument is parenthesized + 3 | a.b === 'undefined'); + 4 | } + +> Output + + `␊ + 1 | function a() {␊ + 2 | return (// ReturnStatement argument is parenthesized␊ + 3 | a.b === undefined);␊ + 4 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function a() {␊ + > 2 | return (typeof // ReturnStatement argument is parenthesized␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + 3 | a.b === 'undefined');␊ + 4 | }␊ + ` + +## Invalid #22 + 1 | function a() { + 2 | return (typeof // UnaryExpression is parenthesized + 3 | a.b) === 'undefined'; + 4 | } + +> Output + + `␊ + 1 | function a() {␊ + 2 | return (// UnaryExpression is parenthesized␊ + 3 | a.b) === undefined;␊ + 4 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | function a() {␊ + > 2 | return (typeof // UnaryExpression is parenthesized␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + 3 | a.b) === 'undefined';␊ + 4 | }␊ + ` + +## Invalid #1 + 1 | typeof undefinedVariableIdentifier === "undefined" + +> Options + + `␊ + [␊ + {␊ + "checkGlobalVariables": true␊ + }␊ + ]␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof undefinedVariableIdentifier === "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`… === undefined\`.␊ + 1 | undefinedVariableIdentifier === undefined␊ + ` + +## Invalid #2 + 1 | typeof Array !== "undefined" + +> Options + + `␊ + [␊ + {␊ + "checkGlobalVariables": true␊ + }␊ + ]␊ + ` + +> Error 1/1 + + `␊ + > 1 | typeof Array !== "undefined"␊ + | ^^^^^^ Compare with \`undefined\` directly instead of using \`typeof\`.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Switch to \`… !== undefined\`.␊ + 1 | Array !== undefined␊ + ` diff --git a/test/snapshots/no-typeof-undefined.mjs.snap b/test/snapshots/no-typeof-undefined.mjs.snap new file mode 100644 index 0000000000..21bc6a0339 Binary files /dev/null and b/test/snapshots/no-typeof-undefined.mjs.snap differ