Skip to content

Commit

Permalink
Add no-await-expression-member rule (#1586)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker committed Nov 10, 2021
1 parent 5ee67a3 commit 0485924
Show file tree
Hide file tree
Showing 11 changed files with 478 additions and 15 deletions.
1 change: 1 addition & 0 deletions configs/recommended.js
Expand Up @@ -22,6 +22,7 @@ module.exports = {
'unicorn/no-array-method-this-argument': 'error',
'unicorn/no-array-push-push': 'error',
'unicorn/no-array-reduce': 'error',
'unicorn/no-await-expression-member': 'error',
'unicorn/no-console-spaces': 'error',
'unicorn/no-document-cookie': 'error',
'unicorn/no-empty-file': 'error',
Expand Down
42 changes: 42 additions & 0 deletions docs/rules/no-await-expression-member.md
@@ -0,0 +1,42 @@
# Forbid member access from await expression

When accessing a member from an await expression, the await expression has to be parenthesized, which is not readable.

This rule is fixable for simple member access.

## Fail

```js
const foo = (await import('./foo.js')).default;
```

```js
const secondElement = (await getArray())[1];
```

```js
const property = (await getObject()).property;
```

```js
const data = await (await fetch('/foo')).json();
```

## Pass

```js
const {default: foo} = await import('./foo.js');
```

```js
const [, secondElement] = await getArray();
```

```js
const {property} = await getObject();
```

```js
const response = await fetch('/foo');
const data = await response.json();
```
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -55,6 +55,7 @@ Configure it in `package.json`.
"unicorn/no-array-method-this-argument": "error",
"unicorn/no-array-push-push": "error",
"unicorn/no-array-reduce": "error",
"unicorn/no-await-expression-member": "error",
"unicorn/no-console-spaces": "error",
"unicorn/no-document-cookie": "error",
"unicorn/no-empty-file": "error",
Expand Down Expand Up @@ -169,6 +170,7 @@ Each rule has emojis denoting:
| [no-array-method-this-argument](docs/rules/no-array-method-this-argument.md) | Disallow using the `this` argument in array methods. || 🔧 | 💡 |
| [no-array-push-push](docs/rules/no-array-push-push.md) | Enforce combining multiple `Array#push()` into one call. || 🔧 | 💡 |
| [no-array-reduce](docs/rules/no-array-reduce.md) | Disallow `Array#reduce()` and `Array#reduceRight()`. || | |
| [no-await-expression-member](docs/rules/no-await-expression-member.md) | Forbid member access from await expression. || 🔧 | |
| [no-console-spaces](docs/rules/no-console-spaces.md) | Do not use leading/trailing space between `console.log` parameters. || 🔧 | |
| [no-document-cookie](docs/rules/no-document-cookie.md) | Do not use `document.cookie` directly. || | |
| [no-empty-file](docs/rules/no-empty-file.md) | Disallow empty files. || | |
Expand Down
1 change: 1 addition & 0 deletions rules/fix/index.js
Expand Up @@ -3,6 +3,7 @@
module.exports = {
// Utilities
extendFixRange: require('./extend-fix-range.js'),
removeParentheses: require('./remove-parentheses.js'),

appendArgument: require('./append-argument.js'),
removeArgument: require('./remove-argument.js'),
Expand Down
11 changes: 11 additions & 0 deletions rules/fix/remove-parentheses.js
@@ -0,0 +1,11 @@
'use strict';
const {getParentheses} = require('../utils/parentheses.js');

function * removeParentheses(node, fixer, sourceCode) {
const parentheses = getParentheses(node, sourceCode);
for (const token of parentheses) {
yield fixer.remove(token);
}
}

module.exports = removeParentheses;
84 changes: 84 additions & 0 deletions rules/no-await-expression-member.js
@@ -0,0 +1,84 @@
'use strict';
const {
removeParentheses,
removeMemberExpressionProperty,
} = require('./fix/index.js');

const MESSAGE_ID = 'no-await-expression-member';
const messages = {
[MESSAGE_ID]: 'Do not access a member directly from an await expression.',
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => {
const sourceCode = context.getSourceCode();

return {
'MemberExpression[object.type="AwaitExpression"]'(memberExpression) {
const {property} = memberExpression;
const problem = {
node: property,
messageId: MESSAGE_ID,
};

// `const foo = (await bar)[0]`
if (
memberExpression.computed
&& !memberExpression.optional
&& property.type === 'Literal'
&& (property.value === 0 || property.value === 1)
&& memberExpression.parent.type === 'VariableDeclarator'
&& memberExpression.parent.init === memberExpression
&& memberExpression.parent.id.type === 'Identifier'
) {
problem.fix = function * (fixer) {
const variable = memberExpression.parent.id;
yield fixer.insertTextBefore(variable, property.value === 0 ? '[' : '[, ');
yield fixer.insertTextAfter(variable, ']');

yield removeMemberExpressionProperty(fixer, memberExpression, sourceCode);
yield * removeParentheses(memberExpression.object, fixer, sourceCode);
};

return problem;
}

// `const foo = (await bar).foo`
if (
!memberExpression.computed
&& !memberExpression.optional
&& property.type === 'Identifier'
&& memberExpression.parent.type === 'VariableDeclarator'
&& memberExpression.parent.init === memberExpression
&& memberExpression.parent.id.type === 'Identifier'
&& memberExpression.parent.id.name === property.name
) {
problem.fix = function * (fixer) {
const variable = memberExpression.parent.id;
yield fixer.insertTextBefore(variable, '{');
yield fixer.insertTextAfter(variable, '}');

yield removeMemberExpressionProperty(fixer, memberExpression, sourceCode);
yield * removeParentheses(memberExpression.object, fixer, sourceCode);
};

return problem;
}

return problem;
},
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
description: 'Forbid member access from await expression.',
},
fixable: 'code',
schema: [],
messages,
},
};
29 changes: 15 additions & 14 deletions rules/prefer-module.js
Expand Up @@ -2,10 +2,13 @@
const {isOpeningParenToken} = require('eslint-utils');
const isShadowed = require('./utils/is-shadowed.js');
const isStaticRequire = require('./utils/is-static-require.js');
const {getParentheses} = require('./utils/parentheses.js');
const assertToken = require('./utils/assert-token.js');
const {referenceIdentifierSelector} = require('./selectors/index.js');
const {replaceReferenceIdentifier, removeSpacesAfter} = require('./fix/index.js');
const {
removeParentheses,
replaceReferenceIdentifier,
removeSpacesAfter,
} = require('./fix/index.js');

const ERROR_USE_STRICT_DIRECTIVE = 'error/use-strict-directive';
const ERROR_GLOBAL_RETURN = 'error/global-return';
Expand Down Expand Up @@ -34,15 +37,6 @@ const identifierSelector = referenceIdentifierSelector([
'__dirname',
]);

function * removeParentheses(nodeOrNodes, fixer, sourceCode) {
for (const node of Array.isArray(nodeOrNodes) ? nodeOrNodes : [nodeOrNodes]) {
const parentheses = getParentheses(node, sourceCode);
for (const token of parentheses) {
yield fixer.remove(token);
}
}
}

function fixRequireCall(node, sourceCode) {
if (!isStaticRequire(node.parent) || node.parent.callee !== node) {
return;
Expand All @@ -66,7 +60,10 @@ function fixRequireCall(node, sourceCode) {
yield fixer.replaceText(openingParenthesisToken, ' ');
const closingParenthesisToken = sourceCode.getLastToken(requireCall);
yield fixer.remove(closingParenthesisToken);
yield * removeParentheses([callee, requireCall, source], fixer, sourceCode);

for (const node of [callee, requireCall, source]) {
yield * removeParentheses(node, fixer, sourceCode);
}
};
}

Expand Down Expand Up @@ -124,7 +121,9 @@ function fixRequireCall(node, sourceCode) {
const closingParenthesisToken = sourceCode.getLastToken(requireCall);
yield fixer.remove(closingParenthesisToken);

yield * removeParentheses([callee, requireCall, source], fixer, sourceCode);
for (const node of [callee, requireCall, source]) {
yield * removeParentheses(node, fixer, sourceCode);
}

if (id.type === 'Identifier') {
return;
Expand Down Expand Up @@ -180,7 +179,9 @@ function fixDefaultExport(node, sourceCode) {
yield fixer.remove(equalToken);
yield removeSpacesAfter(equalToken, sourceCode, fixer);

yield * removeParentheses([node.parent, node], fixer, sourceCode);
for (const currentNode of [node.parent, node]) {
yield * removeParentheses(currentNode, fixer, sourceCode);
}
};
}

Expand Down
5 changes: 4 additions & 1 deletion test/integration/test.mjs
Expand Up @@ -88,7 +88,10 @@ const makeEslintTask = (project, destination) => {
});
};

const getBranch = mem(async dirname => (await execa('git', ['branch', '--show-current'], {cwd: dirname})).stdout);
const getBranch = mem(async dirname => {
const {stdout} = await execa('git', ['branch', '--show-current'], {cwd: dirname});
return stdout;
});

const execute = project => {
const destination = project.location || path.join(dirname, 'fixtures', project.name);
Expand Down
53 changes: 53 additions & 0 deletions test/no-await-expression-member.mjs
@@ -0,0 +1,53 @@
import {getTester} from './utils/test.mjs';

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

test.snapshot({
valid: [
'const foo = await promise',
'const {foo: bar} = await promise',
'const foo = !await promise',
'const foo = typeof await promise',
'const foo = await notPromise.method()',
'const foo = foo[await promise]',

// These await expression need parenthesized, but rarely used
'new (await promiseReturnsAClass)',
'(await promiseReturnsAFunction)()',
],
invalid: [
'(await promise)[0]',
'(await promise).property',
'const foo = (await promise).bar()',
'const foo = (await promise).bar?.()',
'const foo = (await promise)?.bar()',

'const firstElement = (await getArray())[0]',
'const secondElement = (await getArray())[1]',
'const thirdElement = (await getArray())[2]',
'const optionalFirstElement = (await getArray())?.[0]',
'const {propertyOfFirstElement} = (await getArray())[0]',
'const [firstElementOfFirstElement] = (await getArray())[0]',
'let foo, firstElement = (await getArray())[0]',
'var firstElement = (await getArray())[0], bar',

'const property = (await getObject()).property',
'const renamed = (await getObject()).property',
'const property = (await getObject())[property]',
'const property = (await getObject())?.property',
'const {propertyOfProperty} = (await getObject()).property',
'const {propertyOfProperty} = (await getObject()).propertyOfProperty',
'const [firstElementOfProperty] = (await getObject()).property',
'const [firstElementOfProperty] = (await getObject()).firstElementOfProperty',

'firstElement = (await getArray())[0]',
'property = (await getArray()).property',
],
});

test.typescript({
valid: [
'function foo () {return (await promise) as string;}',
],
invalid: [],
});

0 comments on commit 0485924

Please sign in to comment.