From 0485924f519b2a92d0f1fa842bf46ab50c6dab2d Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Wed, 10 Nov 2021 13:59:18 +0800 Subject: [PATCH] Add `no-await-expression-member` rule (#1586) --- configs/recommended.js | 1 + docs/rules/no-await-expression-member.md | 42 +++ readme.md | 2 + rules/fix/index.js | 1 + rules/fix/remove-parentheses.js | 11 + rules/no-await-expression-member.js | 84 ++++++ rules/prefer-module.js | 29 +- test/integration/test.mjs | 5 +- test/no-await-expression-member.mjs | 53 ++++ .../no-await-expression-member.mjs.md | 265 ++++++++++++++++++ .../no-await-expression-member.mjs.snap | Bin 0 -> 1142 bytes 11 files changed, 478 insertions(+), 15 deletions(-) create mode 100644 docs/rules/no-await-expression-member.md create mode 100644 rules/fix/remove-parentheses.js create mode 100644 rules/no-await-expression-member.js create mode 100644 test/no-await-expression-member.mjs create mode 100644 test/snapshots/no-await-expression-member.mjs.md create mode 100644 test/snapshots/no-await-expression-member.mjs.snap diff --git a/configs/recommended.js b/configs/recommended.js index 16034c6370..0b8b752bb9 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -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', diff --git a/docs/rules/no-await-expression-member.md b/docs/rules/no-await-expression-member.md new file mode 100644 index 0000000000..cf151bd49f --- /dev/null +++ b/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(); +``` diff --git a/readme.md b/readme.md index 1a03ef847b..559a14351d 100644 --- a/readme.md +++ b/readme.md @@ -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", @@ -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. | ✅ | | | diff --git a/rules/fix/index.js b/rules/fix/index.js index eaf982eb9a..5ce2853202 100644 --- a/rules/fix/index.js +++ b/rules/fix/index.js @@ -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'), diff --git a/rules/fix/remove-parentheses.js b/rules/fix/remove-parentheses.js new file mode 100644 index 0000000000..42d68f2cd2 --- /dev/null +++ b/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; diff --git a/rules/no-await-expression-member.js b/rules/no-await-expression-member.js new file mode 100644 index 0000000000..096763574d --- /dev/null +++ b/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, + }, +}; diff --git a/rules/prefer-module.js b/rules/prefer-module.js index 935bf951c2..f7269773b6 100644 --- a/rules/prefer-module.js +++ b/rules/prefer-module.js @@ -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'; @@ -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; @@ -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); + } }; } @@ -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; @@ -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); + } }; } diff --git a/test/integration/test.mjs b/test/integration/test.mjs index 48a52a1885..6e46e77299 100644 --- a/test/integration/test.mjs +++ b/test/integration/test.mjs @@ -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); diff --git a/test/no-await-expression-member.mjs b/test/no-await-expression-member.mjs new file mode 100644 index 0000000000..231894ed39 --- /dev/null +++ b/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: [], +}); diff --git a/test/snapshots/no-await-expression-member.mjs.md b/test/snapshots/no-await-expression-member.mjs.md new file mode 100644 index 0000000000..22e3be9791 --- /dev/null +++ b/test/snapshots/no-await-expression-member.mjs.md @@ -0,0 +1,265 @@ +# Snapshot report for `test/no-await-expression-member.mjs` + +The actual snapshot is saved in `no-await-expression-member.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | (await promise)[0] + +> Error 1/1 + + `␊ + > 1 | (await promise)[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #2 + 1 | (await promise).property + +> Error 1/1 + + `␊ + > 1 | (await promise).property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #3 + 1 | const foo = (await promise).bar() + +> Error 1/1 + + `␊ + > 1 | const foo = (await promise).bar()␊ + | ^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #4 + 1 | const foo = (await promise).bar?.() + +> Error 1/1 + + `␊ + > 1 | const foo = (await promise).bar?.()␊ + | ^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #5 + 1 | const foo = (await promise)?.bar() + +> Error 1/1 + + `␊ + > 1 | const foo = (await promise)?.bar()␊ + | ^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #6 + 1 | const firstElement = (await getArray())[0] + +> Output + + `␊ + 1 | const [firstElement] = await getArray()␊ + ` + +> Error 1/1 + + `␊ + > 1 | const firstElement = (await getArray())[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #7 + 1 | const secondElement = (await getArray())[1] + +> Output + + `␊ + 1 | const [, secondElement] = await getArray()␊ + ` + +> Error 1/1 + + `␊ + > 1 | const secondElement = (await getArray())[1]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #8 + 1 | const thirdElement = (await getArray())[2] + +> Error 1/1 + + `␊ + > 1 | const thirdElement = (await getArray())[2]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #9 + 1 | const optionalFirstElement = (await getArray())?.[0] + +> Error 1/1 + + `␊ + > 1 | const optionalFirstElement = (await getArray())?.[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #10 + 1 | const {propertyOfFirstElement} = (await getArray())[0] + +> Error 1/1 + + `␊ + > 1 | const {propertyOfFirstElement} = (await getArray())[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #11 + 1 | const [firstElementOfFirstElement] = (await getArray())[0] + +> Error 1/1 + + `␊ + > 1 | const [firstElementOfFirstElement] = (await getArray())[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #12 + 1 | let foo, firstElement = (await getArray())[0] + +> Output + + `␊ + 1 | let foo, [firstElement] = await getArray()␊ + ` + +> Error 1/1 + + `␊ + > 1 | let foo, firstElement = (await getArray())[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #13 + 1 | var firstElement = (await getArray())[0], bar + +> Output + + `␊ + 1 | var [firstElement] = await getArray(), bar␊ + ` + +> Error 1/1 + + `␊ + > 1 | var firstElement = (await getArray())[0], bar␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #14 + 1 | const property = (await getObject()).property + +> Output + + `␊ + 1 | const {property} = await getObject()␊ + ` + +> Error 1/1 + + `␊ + > 1 | const property = (await getObject()).property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #15 + 1 | const renamed = (await getObject()).property + +> Error 1/1 + + `␊ + > 1 | const renamed = (await getObject()).property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #16 + 1 | const property = (await getObject())[property] + +> Error 1/1 + + `␊ + > 1 | const property = (await getObject())[property]␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #17 + 1 | const property = (await getObject())?.property + +> Error 1/1 + + `␊ + > 1 | const property = (await getObject())?.property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #18 + 1 | const {propertyOfProperty} = (await getObject()).property + +> Error 1/1 + + `␊ + > 1 | const {propertyOfProperty} = (await getObject()).property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #19 + 1 | const {propertyOfProperty} = (await getObject()).propertyOfProperty + +> Error 1/1 + + `␊ + > 1 | const {propertyOfProperty} = (await getObject()).propertyOfProperty␊ + | ^^^^^^^^^^^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #20 + 1 | const [firstElementOfProperty] = (await getObject()).property + +> Error 1/1 + + `␊ + > 1 | const [firstElementOfProperty] = (await getObject()).property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #21 + 1 | const [firstElementOfProperty] = (await getObject()).firstElementOfProperty + +> Error 1/1 + + `␊ + > 1 | const [firstElementOfProperty] = (await getObject()).firstElementOfProperty␊ + | ^^^^^^^^^^^^^^^^^^^^^^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #22 + 1 | firstElement = (await getArray())[0] + +> Error 1/1 + + `␊ + > 1 | firstElement = (await getArray())[0]␊ + | ^ Do not access a member directly from an await expression.␊ + ` + +## Invalid #23 + 1 | property = (await getArray()).property + +> Error 1/1 + + `␊ + > 1 | property = (await getArray()).property␊ + | ^^^^^^^^ Do not access a member directly from an await expression.␊ + ` diff --git a/test/snapshots/no-await-expression-member.mjs.snap b/test/snapshots/no-await-expression-member.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..0133fdfe747b09ec6b1cfd7756166931909fb403 GIT binary patch literal 1142 zcmV-+1d01WRzVfs znZDrl4ed?yom!=D?c9>Z2p07KnkTdVLsRbL^zg^;!!B*;{j!J=ENaHgz#wbibN3#f zk#o}QPiB1_m;N$=MGK%;@lUNk0(3r<0uxo#0VBW3&gsg)k|L*%@mAu zbG&VP_1-#0uxKqS1A|Gjc>KIKjmZuofro?^?|8%r78Pe>U~sQgjGM9SK*G5$g@0^E zFNrXLMZ=+(nVo?lq|DI%<8RIl?`niL_fHqP#RwLy1>&Srktpvp5(SvKMR(-1hFionYV3bFa+)H#34oJvbQ{%4Y9fbuo65zZvU7<}+T&b&O!q zKR{f5E~qcDc3$y{r5PqlN%_5uV9_aDKk7ooU@n}K1j z*}8p~UIfWb7Jv3aN<1ip5iIJ*!@#h1f>+e5hcg`{_u6k@>Smb42o}8x#M^cY#r2Da zZ5Y++GP@v$F zuaK8tqL7%JoLXG0kf@NGnwykbq>z$X1T;RUQX#Dno}2|I<+V@FEKYYg?eF2?V#;Ob#GdJKEka) z=jCP=r)ugYB^KH10h1A7XHqeN8$lCzSz-}tUXDdh;?OA9Q2@FT)n{{IJ_8wmtRGi7 z1~G$_bVD`2=0Kf_7Vx+WSE`3Hw9M2|C{6`3Q}6^eYADY`O+P5Qu?Msv$rT3G68m|I zt8L0_2Ab6fOiM{A*X_w`iBQ|kokSKt^e%k#QL_c1;s