Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add
no-typeof-undefined rule (#1966)
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
- Loading branch information
1 parent
e8c5156
commit d7f7341
Showing
9 changed files
with
743 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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). | ||
|
|
||
| <!-- end auto-generated rule header --> | ||
| <!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` --> | ||
|
|
||
| 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 | ||
| ``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, | ||
| }, | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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}]})), | ||
| }); |
Oops, something went wrong.