Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-typeof-undefined rule #1966

Merged
merged 14 commits into from
Nov 20, 2022
1 change: 1 addition & 0 deletions configs/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
49 changes: 49 additions & 0 deletions docs/rules/no-typeof-undefined.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Enforce compare with `undefined` directly
fisker marked this conversation as resolved.
Show resolved Hide resolved

✅ 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).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

Enforce compare value with `undefined` directly instead of compare `typeof value` with `'undefined'`.
fisker marked this conversation as resolved.
Show resolved Hide resolved

## Fail

```js
if (typeof foo === 'undefined') {}
```

```js
if (typeof foo !== 'undefined') {}
```

## Pass

```js
if (foo === undefined) {}
```

```js
if (foo !== undefined) {}
```

## Options

### checkGlobalVariables

Type: `boolean`\
Default: `true`

Set it to `false` to ignore variables not defined in file.

```js
// eslint unicorn/no-typeof-undefined: ["error", {"checkGlobalVariables": false}]
if (typeof undefinedVariable === 'undefined') {} // Passes
```

```js
// eslint unicorn/no-typeof-undefined: ["error", {"checkGlobalVariables": false}]
if (typeof Array === 'undefined') {} // Passes
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) | Enforce compare with `undefined` directly. | ✅ | 🔧 | |
| [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. | ✅ | | |
Expand Down
2 changes: 1 addition & 1 deletion rules/no-static-only-class.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
125 changes: 125 additions & 0 deletions rules/no-typeof-undefined.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
'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';
const messages = {
[MESSAGE_ID_ERROR]: 'Compare with `undefined` directly instead of `typeof` check.',
fisker marked this conversation as resolved.
Show resolved Hide resolved
};

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: true,
...context.options[0],
};

return {
[selector](binaryExpression) {
const {left: typeofNode, right: undefinedString} = binaryExpression;
if (undefinedString.value !== 'undefined') {
return;
}

const valueNode = typeofNode.argument;

if (
valueNode.type === 'Identifier'
&& !checkGlobalVariables
&& !isShadowed(context.getScope(), valueNode)
) {
return;
}

const sourceCode = context.getSourceCode();
const [typeofToken, secondToken] = sourceCode.getFirstTokens(typeofNode, 2);

return {
node: binaryExpression,
loc: typeofToken.loc,
messageId: MESSAGE_ID_ERROR,
* fix(fixer) {
// Change `==`/`!=` to `===`/`!==`
const {operator} = binaryExpression;
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, ';');
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as

const {test, parent} = node;
, but I'm not going to make a reusable function in this PR, will do later.

},
};
},
};
};

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: 'Enforce compare with `undefined` directly.',
},
fixable: 'code',
schema,
messages,
},
};
2 changes: 1 addition & 1 deletion rules/utils/is-same-reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
85 changes: 85 additions & 0 deletions test/no-typeof-undefined.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
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 a.b === "string"',
'"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\'',
'typeof undefinedVariableIdentifier == \'undefined\'',
// ASI
outdent`
foo
typeof [] === "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: false`
test.snapshot({
valid: [
'typeof foo === "undefined"',
'foo = 2; typeof foo === "undefined"',
'/* globals foo: readonly */ typeof foo === "undefined"',
'/* globals globalThis: readonly */ typeof globalThis === "undefined"',
].map(code => ({code, options: [{checkGlobalVariables: false}]})),
invalid: [
'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"
`,
].map(code => ({code, options: [{checkGlobalVariables: false}]})),
});
Loading