feat(always-return): add ignoreAssignmentVariable option (#518)

Co-authored-by: Sebastian Good <[email protected]>
Co-authored-by: Brett Zamir <[email protected]>

Author: 3 people

__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 }],
+    },
   ],
 })

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()
+})
+```

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]