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-unnecessary-await rule #1904

Merged
merged 13 commits into from Sep 20, 2022
Merged
1 change: 1 addition & 0 deletions configs/recommended.js
Expand Up @@ -50,6 +50,7 @@ module.exports = {
'unicorn/no-static-only-class': 'error',
'unicorn/no-thenable': 'error',
'unicorn/no-this-assignment': 'error',
'unicorn/no-unnecessary-await': 'error',
'unicorn/no-unreadable-array-destructuring': 'error',
'unicorn/no-unreadable-iife': 'error',
'unicorn/no-unsafe-regex': 'off',
Expand Down
30 changes: 30 additions & 0 deletions docs/rules/no-unnecessary-await.md
@@ -0,0 +1,30 @@
# Disallow awaiting non-promise values

<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
✅ *This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

🔧 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).*
<!-- /RULE_NOTICE -->

The [`await` operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await) should only be used on [`Promise`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) values.

## Fail

```js
await await promise;
```

```js
await [promise1, promise2];
```

## Pass

```js
await promise;
```

```js
await Promise.allSettled([promise1, promise2]);
```
1 change: 1 addition & 0 deletions readme.md
Expand Up @@ -91,6 +91,7 @@ Each rule has emojis denoting:
| [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-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. | ✅ | | |
| [no-unsafe-regex](docs/rules/no-unsafe-regex.md) | Disallow unsafe regular expressions. | | | |
Expand Down
21 changes: 21 additions & 0 deletions rules/fix/add-parenthesizes-to-return-or-throw-expression.js
@@ -0,0 +1,21 @@
'use strict';
const {isSemicolonToken} = require('eslint-utils');

function * addParenthesizesToReturnOrThrowExpression(fixer, node, sourceCode) {
if (node.type !== 'ReturnStatement' && node.type !== 'ThrowStatement') {
return;
}

const returnOrThrowToken = sourceCode.getFirstToken(node);
yield fixer.insertTextAfter(returnOrThrowToken, ' (');

const lastToken = sourceCode.getLastToken(node);
if (!isSemicolonToken(lastToken)) {
yield fixer.insertTextAfter(node, ')');
return;
}

yield fixer.insertTextBefore(lastToken, ')');
}

module.exports = addParenthesizesToReturnOrThrowExpression;
1 change: 1 addition & 0 deletions rules/fix/index.js
Expand Up @@ -19,4 +19,5 @@ module.exports = {
removeSpacesAfter: require('./remove-spaces-after.js'),
fixSpaceAroundKeyword: require('./fix-space-around-keywords.js'),
replaceStringLiteral: require('./replace-string-literal.js'),
addParenthesizesToReturnOrThrowExpression: require('./add-parenthesizes-to-return-or-throw-expression.js'),
};
8 changes: 4 additions & 4 deletions rules/fix/remove-spaces-after.js
@@ -1,9 +1,9 @@
'use strict';

function removeSpacesAfter(indexOrNode, sourceCode, fixer) {
let index = indexOrNode;
if (typeof indexOrNode === 'object' && Array.isArray(indexOrNode.range)) {
index = indexOrNode.range[1];
function removeSpacesAfter(indexOrNodeOrToken, sourceCode, fixer) {
let index = indexOrNodeOrToken;
if (typeof indexOrNodeOrToken === 'object' && Array.isArray(indexOrNodeOrToken.range)) {
index = indexOrNodeOrToken.range[1];
}

const textAfter = sourceCode.text.slice(index);
Expand Down
48 changes: 14 additions & 34 deletions rules/fix/switch-new-expression-to-call-expression.js
@@ -1,41 +1,17 @@
'use strict';
const isNewExpressionWithParentheses = require('../utils/is-new-expression-with-parentheses.js');
const {isParenthesized} = require('../utils/parentheses.js');
const isOnSameLine = require('../utils/is-on-same-line.js');
const addParenthesizesToReturnOrThrowExpression = require('./add-parenthesizes-to-return-or-throw-expression.js');
const removeSpaceAfter = require('./remove-spaces-after.js');

function * fixReturnOrThrowStatementArgument(newExpression, sourceCode, fixer) {
const {parent} = newExpression;
if (
(parent.type !== 'ReturnStatement' && parent.type !== 'ThrowStatement')
|| parent.argument !== newExpression
|| isParenthesized(newExpression, sourceCode)
) {
return;
}

const returnStatement = parent;
const returnToken = sourceCode.getFirstToken(returnStatement);
const classNode = newExpression.callee;

// Ideally, we should use first parenthesis of the `callee`, and should check spaces after the `new` token
// But adding extra parentheses is harmless, no need to be too complicated
if (returnToken.loc.start.line === classNode.loc.start.line) {
return;
}
function * switchNewExpressionToCallExpression(newExpression, sourceCode, fixer) {
const newToken = sourceCode.getFirstToken(newExpression);
yield fixer.remove(newToken);
yield removeSpaceAfter(newToken, sourceCode, fixer);

yield fixer.insertTextAfter(returnToken, ' (');
yield fixer.insertTextAfter(newExpression, ')');
}

function * switchNewExpressionToCallExpression(node, sourceCode, fixer) {
const [start] = node.range;
let end = start + 3; // `3` = length of `new`
const textAfter = sourceCode.text.slice(end);
const [leadingSpaces] = textAfter.match(/^\s*/);
end += leadingSpaces.length;
yield fixer.removeRange([start, end]);

if (!isNewExpressionWithParentheses(node, sourceCode)) {
yield fixer.insertTextAfter(node, '()');
if (!isNewExpressionWithParentheses(newExpression, sourceCode)) {
yield fixer.insertTextAfter(newExpression, '()');
}

/*
Expand All @@ -48,7 +24,11 @@ function * switchNewExpressionToCallExpression(node, sourceCode, fixer) {
}
```
*/
yield * fixReturnOrThrowStatementArgument(node, sourceCode, fixer);
if (!isOnSameLine(newToken, newExpression.callee) && !isParenthesized(newExpression, sourceCode)) {
// Ideally, we should use first parenthesis of the `callee`, and should check spaces after the `new` token
// But adding extra parentheses is harmless, no need to be too complicated
yield * addParenthesizesToReturnOrThrowExpression(fixer, newExpression.parent, sourceCode);
}
}

module.exports = switchNewExpressionToCallExpression;
103 changes: 103 additions & 0 deletions rules/no-unnecessary-await.js
@@ -0,0 +1,103 @@
'use strict';
const {
addParenthesizesToReturnOrThrowExpression,
removeSpacesAfter,
} = require('./fix/index.js');
const {isParenthesized} = require('./utils/parentheses.js');
const needsSemicolon = require('./utils/needs-semicolon.js');
const isOnSameLine = require('./utils/is-on-same-line.js');

const MESSAGE_ID = 'no-unnecessary-await';
const messages = {
[MESSAGE_ID]: 'Do not `await` non-promise value.',
};

function notPromise(node) {
switch (node.type) {
case 'ArrayExpression':
case 'ArrowFunctionExpression':
case 'AwaitExpression':
case 'BinaryExpression':
case 'ClassExpression':
case 'FunctionExpression':
case 'JSXElement':
case 'JSXFragment':
case 'Literal':
case 'TemplateLiteral':
case 'UnaryExpression':
case 'UpdateExpression':
return true;
case 'SequenceExpression':
return notPromise(node.expressions[node.expressions.length - 1]);
// No default
}

return false;
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
AwaitExpression(node) {
if (!notPromise(node.argument)) {
return;
}

const sourceCode = context.getSourceCode();
const awaitToken = sourceCode.getFirstToken(node);
const problem = {
node,
loc: awaitToken.loc,
messageId: MESSAGE_ID,
};

const valueNode = node.argument;
if (
// Removing `await` may change them to a declaration, if there is no `id` will cause SyntaxError
valueNode.type === 'FunctionExpression'
|| valueNode.type === 'ClassExpression'
// `+await +1` -> `++1`
|| (
node.parent.type === 'UnaryExpression'
&& valueNode.type === 'UnaryExpression'
&& node.parent.operator === valueNode.operator
)
) {
return problem;
}

return Object.assign(problem, {
/** @param {import('eslint').Rule.RuleFixer} fixer */
* fix(fixer) {
if (
!isOnSameLine(awaitToken, valueNode)
&& !isParenthesized(node, sourceCode)
) {
yield * addParenthesizesToReturnOrThrowExpression(fixer, node.parent, sourceCode);
}

yield fixer.remove(awaitToken);
yield removeSpacesAfter(awaitToken, sourceCode, fixer);

const nextToken = sourceCode.getTokenAfter(awaitToken);
const tokenBefore = sourceCode.getTokenBefore(awaitToken);
if (needsSemicolon(tokenBefore, sourceCode, nextToken.value)) {
yield fixer.insertTextBefore(nextToken, ';');
}
},
});
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Disallow awaiting non-promise values.',
},
fixable: 'code',

messages,
},
};
7 changes: 7 additions & 0 deletions rules/utils/is-on-same-line.js
@@ -0,0 +1,7 @@
'use strict';

function isOnSameLine(nodeOrTokenA, nodeOrTokenB) {
return nodeOrTokenA.loc.start.line === nodeOrTokenB.loc.start.line;
}

module.exports = isOnSameLine;
113 changes: 113 additions & 0 deletions test/no-unnecessary-await.mjs
@@ -0,0 +1,113 @@
import outdent from 'outdent';
import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

test.snapshot({
testerOptions: {
parserOptions: {
ecmaFeatures: {
jsx: true,
},
},
},
valid: [
'await {then}',
'await a ? b : c',
'await a || b',
'await a && b',
'await a ?? b',
'await new Foo()',
'await tagged``',
'class A { async foo() { await this }}',
'async function * foo() {await (yield bar);}',
'await (1, Promise.resolve())',
],
invalid: [
'await []',
'await [Promise.resolve()]',
'await (() => {})',
'await (() => Promise.resolve())',
'await (a === b)',
'await (a instanceof Promise)',
'await (a > b)',
'await class {}',
'await class extends Promise {}',
'await function() {}',
'await function name() {}',
'await function() { return Promise.resolve() }',
'await (<></>)',
'await (<a></a>)',
'await 0',
'await 1',
'await ""',
'await "string"',
'await true',
'await false',
'await null',
'await 0n',
'await 1n',
// eslint-disable-next-line no-template-curly-in-string
'await `${Promise.resolve()}`',
'await !Promise.resolve()',
'await void Promise.resolve()',
'await +Promise.resolve()',
'await ~1',
'await ++foo',
'await foo--',
'await (Promise.resolve(), 1)',
outdent`
async function foo() {
return await
// comment
1;
}
`,
outdent`
async function foo() {
return await
// comment
1
}
`,
outdent`
async function foo() {
return( await
// comment
1);
}
`,
outdent`
foo()
await []
`,
outdent`
foo()
await +1
`,
outdent`
async function foo() {
return await
// comment
[];
}
`,
outdent`
async function foo() {
throw await
// comment
1;
}
`,
outdent`
console.log(
await
// comment
[]
);
`,
'async function foo() {+await +1}',
'async function foo() {-await-1}',
'async function foo() {+await -1}',
],
});