apply suggestions

Author: gurgunday

__tests__/always-return.js

@@ -105,18 +105,15 @@ ruleTester.run('always-return', rule, {
         .finally(() => console.error('end'))`,
       options: [{ ignoreLastCallback: true }],
     },
-    `hey.then(x => { window[x] = x })`,
-    `hey.then(x => { window.a = x })`,
-    `hey.then(x => { window.a.n = x })`,
-    `hey.then(x => { window[12] = x })`,
-    `hey.then(x => { window['12']["test"] = x })`,
-    {
-      code: `hey.then(x => { globalThis[x] = x })`,
-      options: [{ ignoreAssignmentVariable: ['globalThis'] }],
-    },
-    {
-      code: `hey.then(x => { globalThis.a.x = x })`,
-      options: [{ ignoreAssignmentVariable: ['globalThis'] }],
+    `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: ['window'] }],
     },
   ],
 
@@ -249,7 +246,25 @@ ruleTester.run('always-return', rule, {
       errors: [{ message }],
     },
     {
-      code: `hey.then(x => { windows[x] = x })`,
+      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 => { windows['x'] = x })`,
+      options: [{ ignoreAssignmentVariable: ['window'] }],
+      errors: [{ message }],
+    },
+    {
+      code: `hey.then(x => { x() })`,
       options: [{ ignoreAssignmentVariable: ['window'] }],
       errors: [{ message }],
     },

docs/rules/always-return.md

@@ -98,33 +98,37 @@ function foo() {
 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
-`["window"]`.
+`["globalThis"]`.
 
 ```js
-/* eslint promise/always-return: ["error", { ignoreAssignmentVariable: ["window", "globalThis"] }] */
+/* eslint promise/always-return: ["error", { ignoreAssignmentVariable: ["globalThis"] }] */
 
 // OK
 promise.then((x) => {
-  window.x = x
+  globalThis = x
 })
 
-// OK
 promise.then((x) => {
-  window.x.y = x
+  globalThis.x = x
 })
 
 // OK
 promise.then((x) => {
-  globalThis.x = x
+  globalThis.x.y = x
 })
 
-// OK
+// NG
+promise.then((x) => {
+  anyOtherVariable = x
+})
+
+// NG
 promise.then((x) => {
-  globalThis.modules.x = x
+  anyOtherVariable.x = x
 })
 
 // NG
 promise.then((x) => {
-  let a = x
+  x()
 })
 ```

rules/always-return.js

@@ -125,37 +125,22 @@ function peek(arr) {
 }
 
 /**
- * Gets the full object name for a MemberExpression or Identifier
- * @param {import('estree').MemberExpression | import('estree').Identifier} node
- * @returns {string}
+ * Gets the root object name for a MemberExpression or Identifier.
+ * @param {Node} node
+ * @returns {string | undefined}
  */
-function getFullObjectName(node) {
+function getRootObjectName(node) {
   if (node.type === 'Identifier') {
     return node.name
   }
-  const objectName = getFullObjectName(node.object)
-  const propertyName = node.property.name
-  return `${objectName}${node.computed ? '[' : '.'}${propertyName}${node.computed ? ']' : ''}`
-}
-
-/**
- * Checks if the given name starts with an ignored variable
- * @param {string} name
- * @param {string[]} ignoredVars
- * @returns {boolean}
- */
-function startsWithIgnoredVar(name, ignoredVars) {
-  return ignoredVars.some(
-    (ignoredVar) =>
-      name === ignoredVar ||
-      (name.startsWith(ignoredVar) &&
-        (name.charAt(ignoredVar.length) === '.' ||
-          name.charAt(ignoredVar.length) === '[')),
-  )
+  // istanbul ignore else (fallback)
+  if (node.type === 'MemberExpression') {
+    return getRootObjectName(node.object)
+  }
 }
 
 /**
- * Checks if the node is an assignment to an ignored variable
+ * Checks if the node is an assignment to an ignored variable. Use getRootObjectName to get the variable name.
  * @param {Node} node
  * @param {string[]} ignoredVars
  * @returns {boolean}
@@ -165,14 +150,8 @@ function isIgnoredAssignment(node, ignoredVars) {
   const expr = node.expression
   if (expr.type !== 'AssignmentExpression') return false
   const left = expr.left
-  // istanbul ignore else
-  if (left.type === 'MemberExpression') {
-    const fullName = getFullObjectName(left.object)
-    return startsWithIgnoredVar(fullName, ignoredVars)
-  }
-  // istanbul ignore next
-  // fallback
-  return false
+  const rootName = getRootObjectName(left)
+  return ignoredVars.includes(rootName)
 }
 
 module.exports = {
@@ -210,8 +189,9 @@ module.exports = {
     const options = context.options[0] || {}
     const ignoreLastCallback = !!options.ignoreLastCallback
     const ignoreAssignmentVariable = options.ignoreAssignmentVariable || [
-      'window',
+      'globalThis',
     ]
+
     /**
      * @typedef {object} FuncInfo
      * @property {string[]} branchIDStack This is a stack representing the currently
@@ -306,13 +286,14 @@ module.exports = {
           return
         }
 
-        if (ignoreAssignmentVariable.length && isLastCallback(node)) {
-          // istanbul ignore else
-          if (node.body?.type === 'BlockStatement') {
-            for (const statement of node.body.body) {
-              if (isIgnoredAssignment(statement, ignoreAssignmentVariable)) {
-                return
-              }
+        if (
+          ignoreAssignmentVariable.length &&
+          isLastCallback(node) &&
+          node.body?.type === 'BlockStatement'
+        ) {
+          for (const statement of node.body.body) {
+            if (isIgnoredAssignment(statement, ignoreAssignmentVariable)) {
+              return
             }
           }
         }