From 701279c573437598e86873f48b4f5cf6432ae38e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 16 Oct 2024 22:07:40 +0200 Subject: [PATCH] feat(always-return): add `ignoreAssignmentVariable` option (#518) Co-authored-by: Sebastian Good <2230835+scagood@users.noreply.github.com> Co-authored-by: Brett Zamir --- __tests__/always-return.js | 38 ++++++++++++++++++++++++++ docs/rules/always-return.md | 46 ++++++++++++++++++++++++++++--- rules/always-return.js | 54 +++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/__tests__/always-return.js b/__tests__/always-return.js index cb888fd6..7cc74de1 100644 --- a/__tests__/always-return.js +++ b/__tests__/always-return.js @@ -105,6 +105,16 @@ ruleTester.run('always-return', rule, { .finally(() => console.error('end'))`, options: [{ ignoreLastCallback: true }], }, + `hey.then(x => { globalThis = x })`, + `hey.then(x => { globalThis[a] = x })`, + `hey.then(x => { globalThis.a = x })`, + `hey.then(x => { globalThis.a.n = x })`, + `hey.then(x => { globalThis[12] = x })`, + `hey.then(x => { globalThis['12']["test"] = x })`, + { + code: `hey.then(x => { window['x'] = x })`, + options: [{ ignoreAssignmentVariable: ['globalThis', 'window'] }], + }, ], invalid: [ @@ -230,5 +240,33 @@ ruleTester.run('always-return', rule, { options: [{ ignoreLastCallback: true }], errors: [{ message }], }, + { + code: `hey.then(x => { invalid = x })`, + errors: [{ message }], + }, + { + code: `hey.then(x => { invalid['x'] = x })`, + errors: [{ message }], + }, + { + code: `hey.then(x => { windo[x] = x })`, + options: [{ ignoreAssignmentVariable: ['window'] }], + errors: [{ message }], + }, + { + code: `hey.then(x => { windo['x'] = x })`, + options: [{ ignoreAssignmentVariable: ['window'] }], + errors: [{ message }], + }, + { + code: `hey.then(x => { windows['x'] = x })`, + options: [{ ignoreAssignmentVariable: ['window'] }], + errors: [{ message }], + }, + { + code: `hey.then(x => { x() })`, + options: [{ ignoreAssignmentVariable: ['window'] }], + errors: [{ message }], + }, ], }) diff --git a/docs/rules/always-return.md b/docs/rules/always-return.md index d778771b..81899d96 100644 --- a/docs/rules/always-return.md +++ b/docs/rules/always-return.md @@ -49,9 +49,9 @@ myPromise.then((b) => { ##### `ignoreLastCallback` -You can pass an `{ ignoreLastCallback: true }` as an option to this rule to the -last `then()` callback in a promise chain does not warn if it does not have a -`return`. Default is `false`. +You can pass an `{ ignoreLastCallback: true }` as an option to this rule so that +the last `then()` callback in a promise chain does not warn if it does not have +a `return`. Default is `false`. ```js // OK @@ -92,3 +92,43 @@ function foo() { }) } ``` + +##### `ignoreAssignmentVariable` + +You can pass an `{ ignoreAssignmentVariable: [] }` as an option to this rule +with a list of variable names so that the last `then()` callback in a promise +chain does not warn if it does an assignment to a global variable. Default is +`["globalThis"]`. + +```js +/* eslint promise/always-return: ["error", { ignoreAssignmentVariable: ["globalThis"] }] */ + +// OK +promise.then((x) => { + globalThis = x +}) + +promise.then((x) => { + globalThis.x = x +}) + +// OK +promise.then((x) => { + globalThis.x.y = x +}) + +// NG +promise.then((x) => { + anyOtherVariable = x +}) + +// NG +promise.then((x) => { + anyOtherVariable.x = x +}) + +// NG +promise.then((x) => { + x() +}) +``` diff --git a/rules/always-return.js b/rules/always-return.js index 093ea33a..7d9784b8 100644 --- a/rules/always-return.js +++ b/rules/always-return.js @@ -124,6 +124,36 @@ function peek(arr) { return arr[arr.length - 1] } +/** + * Gets the root object name for a MemberExpression or Identifier. + * @param {Node} node + * @returns {string | undefined} + */ +function getRootObjectName(node) { + if (node.type === 'Identifier') { + return node.name + } + // istanbul ignore else (fallback) + if (node.type === 'MemberExpression') { + return getRootObjectName(node.object) + } +} + +/** + * Checks if the node is an assignment to an ignored variable. + * @param {Node} node + * @param {string[]} ignoredVars + * @returns {boolean} + */ +function isIgnoredAssignment(node, ignoredVars) { + if (node.type !== 'ExpressionStatement') return false + const expr = node.expression + if (expr.type !== 'AssignmentExpression') return false + const left = expr.left + const rootName = getRootObjectName(left) + return ignoredVars.includes(rootName) +} + module.exports = { meta: { type: 'problem', @@ -139,6 +169,14 @@ module.exports = { ignoreLastCallback: { type: 'boolean', }, + ignoreAssignmentVariable: { + type: 'array', + items: { + type: 'string', + pattern: '^[\\w$]+$', + }, + uniqueItems: true, + }, }, additionalProperties: false, }, @@ -150,6 +188,10 @@ module.exports = { create(context) { const options = context.options[0] || {} const ignoreLastCallback = !!options.ignoreLastCallback + const ignoreAssignmentVariable = options.ignoreAssignmentVariable || [ + 'globalThis', + ] + /** * @typedef {object} FuncInfo * @property {string[]} branchIDStack This is a stack representing the currently @@ -244,6 +286,18 @@ module.exports = { return } + if ( + ignoreAssignmentVariable.length && + isLastCallback(node) && + node.body?.type === 'BlockStatement' + ) { + for (const statement of node.body.body) { + if (isIgnoredAssignment(statement, ignoreAssignmentVariable)) { + return + } + } + } + path.finalSegments.forEach((segment) => { const id = segment.id const branch = funcInfo.branchInfoMap[id]