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-await-expression-member rule #1586

Merged
merged 9 commits into from Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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: [],
});