Skip to content

Commit d7f7341

Browse files
fiskersindresorhus
fisker
and
sindresorhus
authored
Add no-typeof-undefined rule (#1966)
Co-authored-by: Sindre Sorhus <[email protected]>
1 parent e8c5156 commit d7f7341

9 files changed

+743
-2
lines changed

‎configs/recommended.js

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ module.exports = {
5252
'unicorn/no-static-only-class': 'error',
5353
'unicorn/no-thenable': 'error',
5454
'unicorn/no-this-assignment': 'error',
55+
'unicorn/no-typeof-undefined': 'error',
5556
'unicorn/no-unnecessary-await': 'error',
5657
'unicorn/no-unreadable-array-destructuring': 'error',
5758
'unicorn/no-unreadable-iife': 'error',

‎docs/rules/no-typeof-undefined.md

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# Disallow comparing `undefined` using `typeof`
2+
3+
✅ This rule is enabled in the `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs).
4+
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/developer-guide/working-with-rules#providing-suggestions).
6+
7+
<!-- end auto-generated rule header -->
8+
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
9+
10+
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.
11+
12+
## Fail
13+
14+
```js
15+
function foo(bar) {
16+
if (typeof bar === 'undefined') {}
17+
}
18+
```
19+
20+
```js
21+
import foo from './foo.js';
22+
23+
if (typeof foo.bar !== 'undefined') {}
24+
```
25+
26+
## Pass
27+
28+
```js
29+
function foo(bar) {
30+
if (foo === undefined) {}
31+
}
32+
```
33+
34+
```js
35+
import foo from './foo.js';
36+
37+
if (foo.bar !== undefined) {}
38+
```
39+
40+
## Options
41+
42+
### checkGlobalVariables
43+
44+
Type: `boolean`\
45+
Default: `false`
46+
47+
The rule ignores variables not defined in the file by default.
48+
49+
Set it to `true` to check all variables.
50+
51+
```js
52+
// eslint unicorn/no-typeof-undefined: ["error", {"checkGlobalVariables": true}]
53+
if (typeof undefinedVariable === 'undefined') {} // Fails
54+
```
55+
56+
```js
57+
// eslint unicorn/no-typeof-undefined: ["error", {"checkGlobalVariables": true}]
58+
if (typeof Array === 'undefined') {} // Fails
59+
```

‎readme.md

+1
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ Use a [preset config](#preset-configs) or configure each rules in `package.json`
9191
| [no-static-only-class](docs/rules/no-static-only-class.md) | Disallow classes that only have static members. | ✅ | 🔧 | |
9292
| [no-thenable](docs/rules/no-thenable.md) | Disallow `then` property. | ✅ | | |
9393
| [no-this-assignment](docs/rules/no-this-assignment.md) | Disallow assigning `this` to a variable. | ✅ | | |
94+
| [no-typeof-undefined](docs/rules/no-typeof-undefined.md) | Disallow comparing `undefined` using `typeof`. | ✅ | 🔧 | 💡 |
9495
| [no-unnecessary-await](docs/rules/no-unnecessary-await.md) | Disallow awaiting non-promise values. | ✅ | 🔧 | |
9596
| [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ | 🔧 | |
9697
| [no-unreadable-iife](docs/rules/no-unreadable-iife.md) | Disallow unreadable IIFEs. | ✅ | | |

‎rules/no-static-only-class.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function isStaticMember(node) {
4949
if (
5050
isDeclare
5151
|| isReadonly
52-
|| typeof accessibility !== 'undefined'
52+
|| accessibility !== undefined
5353
|| (Array.isArray(decorators) && decorators.length > 0)
5454
// TODO: Remove this when we drop support for `@typescript-eslint/parser` v4
5555
|| key.type === 'TSPrivateIdentifier'

‎rules/no-typeof-undefined.js

+140
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
'use strict';
2+
const isShadowed = require('./utils/is-shadowed.js');
3+
const {matches} = require('./selectors/index.js');
4+
const {
5+
addParenthesizesToReturnOrThrowExpression,
6+
} = require('./fix/index.js');
7+
const {removeSpacesAfter} = require('./fix/index.js');
8+
const isOnSameLine = require('./utils/is-on-same-line.js');
9+
const needsSemicolon = require('./utils/needs-semicolon.js');
10+
const {
11+
isParenthesized,
12+
} = require('./utils/parentheses.js');
13+
14+
const MESSAGE_ID_ERROR = 'no-typeof-undefined/error';
15+
const MESSAGE_ID_SUGGESTION = 'no-typeof-undefined/suggestion';
16+
const messages = {
17+
[MESSAGE_ID_ERROR]: 'Compare with `undefined` directly instead of using `typeof`.',
18+
[MESSAGE_ID_SUGGESTION]: 'Switch to `… {{operator}} undefined`.',
19+
};
20+
21+
const selector = [
22+
'BinaryExpression',
23+
matches(['===', '!==', '==', '!='].map(operator => `[operator="${operator}"]`)),
24+
'[left.type="UnaryExpression"]',
25+
'[left.operator="typeof"]',
26+
'[left.prefix]',
27+
'[right.type="Literal"]',
28+
].join('');
29+
30+
/** @param {import('eslint').Rule.RuleContext} context */
31+
const create = context => {
32+
const {
33+
checkGlobalVariables,
34+
} = {
35+
checkGlobalVariables: false,
36+
...context.options[0],
37+
};
38+
39+
return {
40+
[selector](binaryExpression) {
41+
const {left: typeofNode, right: undefinedString, operator} = binaryExpression;
42+
if (undefinedString.value !== 'undefined') {
43+
return;
44+
}
45+
46+
const valueNode = typeofNode.argument;
47+
const isGlobalVariable = valueNode.type === 'Identifier'
48+
&& !isShadowed(context.getScope(), valueNode);
49+
50+
if (!checkGlobalVariables && isGlobalVariable) {
51+
return;
52+
}
53+
54+
const sourceCode = context.getSourceCode();
55+
const [typeofToken, secondToken] = sourceCode.getFirstTokens(typeofNode, 2);
56+
57+
const fix = function * (fixer) {
58+
// Change `==`/`!=` to `===`/`!==`
59+
if (operator === '==' || operator === '!=') {
60+
const operatorToken = sourceCode.getTokenAfter(
61+
typeofNode,
62+
token => token.type === 'Punctuator' && token.value === operator,
63+
);
64+
65+
yield fixer.insertTextAfter(operatorToken, '=');
66+
}
67+
68+
yield fixer.replaceText(undefinedString, 'undefined');
69+
70+
yield fixer.remove(typeofToken);
71+
yield removeSpacesAfter(typeofToken, sourceCode, fixer);
72+
73+
const {parent} = binaryExpression;
74+
if (
75+
(parent.type === 'ReturnStatement' || parent.type === 'ThrowStatement')
76+
&& parent.argument === binaryExpression
77+
&& !isOnSameLine(typeofToken, secondToken)
78+
&& !isParenthesized(binaryExpression, sourceCode)
79+
&& !isParenthesized(typeofNode, sourceCode)
80+
) {
81+
yield * addParenthesizesToReturnOrThrowExpression(fixer, parent, sourceCode);
82+
return;
83+
}
84+
85+
const tokenBefore = sourceCode.getTokenBefore(binaryExpression);
86+
if (needsSemicolon(tokenBefore, sourceCode, secondToken.value)) {
87+
yield fixer.insertTextBefore(binaryExpression, ';');
88+
}
89+
};
90+
91+
const problem = {
92+
node: binaryExpression,
93+
loc: typeofToken.loc,
94+
messageId: MESSAGE_ID_ERROR,
95+
};
96+
97+
if (isGlobalVariable) {
98+
problem.suggest = [
99+
{
100+
messageId: MESSAGE_ID_SUGGESTION,
101+
data: {operator: operator.startsWith('!') ? '!==' : '==='},
102+
fix,
103+
},
104+
];
105+
} else {
106+
problem.fix = fix;
107+
}
108+
109+
return problem;
110+
},
111+
};
112+
};
113+
114+
const schema = [
115+
{
116+
type: 'object',
117+
additionalProperties: false,
118+
properties: {
119+
checkGlobalVariables: {
120+
type: 'boolean',
121+
default: false,
122+
},
123+
},
124+
},
125+
];
126+
127+
/** @type {import('eslint').Rule.RuleModule} */
128+
module.exports = {
129+
create,
130+
meta: {
131+
type: 'suggestion',
132+
docs: {
133+
description: 'Disallow comparing `undefined` using `typeof`.',
134+
},
135+
fixable: 'code',
136+
hasSuggestions: true,
137+
schema,
138+
messages,
139+
},
140+
};

‎rules/utils/is-same-reference.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ function isSameReference(left, right) {
145145
const nameA = getStaticPropertyName(left);
146146

147147
// `x.y = x["y"]`
148-
if (typeof nameA !== 'undefined') {
148+
if (nameA !== undefined) {
149149
return (
150150
isSameReference(left.object, right.object)
151151
&& nameA === getStaticPropertyName(right)

‎test/no-typeof-undefined.mjs

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import outdent from 'outdent';
2+
import {getTester} from './utils/test.mjs';
3+
4+
const {test} = getTester(import.meta);
5+
6+
test.snapshot({
7+
valid: [
8+
'typeof a.b',
9+
'typeof a.b > "undefined"',
10+
'a.b === "undefined"',
11+
'void a.b === "undefined"',
12+
'+a.b === "undefined"',
13+
'++a.b === "undefined"',
14+
'a.b++ === "undefined"',
15+
'foo === undefined',
16+
'typeof a.b === "string"',
17+
'typeof foo === "undefined"',
18+
'foo = 2; typeof foo === "undefined"',
19+
'/* globals foo: readonly */ typeof foo === "undefined"',
20+
'/* globals globalThis: readonly */ typeof globalThis === "undefined"',
21+
// Cases we are not checking
22+
'"undefined" === typeof a.b',
23+
'const UNDEFINED = "undefined"; typeof a.b === UNDEFINED',
24+
'typeof a.b === `undefined`',
25+
],
26+
invalid: [
27+
'typeof a.b === "undefined"',
28+
'typeof a.b !== "undefined"',
29+
'typeof a.b == "undefined"',
30+
'typeof a.b != "undefined"',
31+
'typeof a.b == \'undefined\'',
32+
'let foo; typeof foo === "undefined"',
33+
'const foo = 1; typeof foo === "undefined"',
34+
'var foo; typeof foo === "undefined"',
35+
'var foo; var foo; typeof foo === "undefined"',
36+
'for (const foo of bar) typeof foo === "undefined";',
37+
outdent`
38+
let foo;
39+
function bar() {
40+
typeof foo === "undefined";
41+
}
42+
`,
43+
'function foo() {typeof foo === "undefined"}',
44+
'function foo(bar) {typeof bar === "undefined"}',
45+
'function foo({bar}) {typeof bar === "undefined"}',
46+
'function foo([bar]) {typeof bar === "undefined"}',
47+
'typeof foo.bar === "undefined"',
48+
outdent`
49+
import foo from 'foo';
50+
typeof foo.bar === "undefined"
51+
`,
52+
// ASI
53+
outdent`
54+
foo
55+
typeof [] === "undefined";
56+
`,
57+
outdent`
58+
foo
59+
typeof (a ? b : c) === "undefined";
60+
`,
61+
outdent`
62+
function a() {
63+
return typeof // comment
64+
a.b === 'undefined';
65+
}
66+
`,
67+
outdent`
68+
function a() {
69+
return (typeof // ReturnStatement argument is parenthesized
70+
a.b === 'undefined');
71+
}
72+
`,
73+
outdent`
74+
function a() {
75+
return (typeof // UnaryExpression is parenthesized
76+
a.b) === 'undefined';
77+
}
78+
`,
79+
],
80+
});
81+
82+
// `checkGlobalVariables: true`
83+
test.snapshot({
84+
valid: [
85+
],
86+
invalid: [
87+
'typeof undefinedVariableIdentifier === "undefined"',
88+
'typeof Array !== "undefined"',
89+
].map(code => ({code, options: [{checkGlobalVariables: true}]})),
90+
});

0 commit comments

Comments
 (0)