fix(eslint-plugin): [class-literal-property-style] ignore property as…

…signed in constructor (#8412) * fix(eslint-plugin): [class-literal-property-style] ignore property assigned in constructor * Update packages/eslint-plugin/src/rules/class-literal-property-style.ts --------- Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
typescript-eslint · Mar 18, 2024 · 04e32d6 · 04e32d6
1 parent 612875b
commit 04e32d6
Show file tree

Hide file tree

Showing 5 changed files with 319 additions and 88 deletions.
diff --git a/packages/eslint-plugin/src/rules/class-literal-property-style.ts b/packages/eslint-plugin/src/rules/class-literal-property-style.ts
@@ -1,7 +1,13 @@
 import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
 
-import { createRule, getStaticStringValue } from '../util';
+import {
+  createRule,
+  getStaticStringValue,
+  isAssignee,
+  isFunction,
+  nullThrows,
+} from '../util';
 
 type Options = ['fields' | 'getters'];
 type MessageIds =
@@ -15,6 +21,11 @@ interface NodeWithModifiers {
   static: boolean;
 }
 
+interface PropertiesInfo {
+  properties: TSESTree.PropertyDefinition[];
+  excludeSet: Set<string>;
+}
+
 const printNodeModifiers = (
   node: NodeWithModifiers,
   final: 'get' | 'readonly',
@@ -65,10 +76,71 @@ export default createRule<Options, MessageIds>({
   },
   defaultOptions: ['fields'],
   create(context, [style]) {
-    function getMethodName(node: TSESTree.MethodDefinition): string {
-      return (
-        getStaticStringValue(node.key) ?? context.sourceCode.getText(node.key)
+    const propertiesInfoStack: PropertiesInfo[] = [];
+
+    function getStringValue(node: TSESTree.Node): string {
+      return getStaticStringValue(node) ?? context.sourceCode.getText(node);
+    }
+
+    function enterClassBody(): void {
+      propertiesInfoStack.push({
+        properties: [],
+        excludeSet: new Set(),
+      });
+    }
+
+    function exitClassBody(): void {
+      const { properties, excludeSet } = nullThrows(
+        propertiesInfoStack.pop(),
+        'Stack should exist on class exit',
       );
+
+      properties.forEach(node => {
+        const { value } = node;
+        if (!value || !isSupportedLiteral(value)) {
+          return;
+        }
+
+        const name = getStringValue(node.key);
+        if (excludeSet.has(name)) {
+          return;
+        }
+
+        context.report({
+          node: node.key,
+          messageId: 'preferGetterStyle',
+          suggest: [
+            {
+              messageId: 'preferGetterStyleSuggestion',
+              fix(fixer): TSESLint.RuleFix {
+                const name = context.sourceCode.getText(node.key);
+
+                let text = '';
+                text += printNodeModifiers(node, 'get');
+                text += node.computed ? `[${name}]` : name;
+                text += `() { return ${context.sourceCode.getText(value)}; }`;
+
+                return fixer.replaceText(node, text);
+              },
+            },
+          ],
+        });
+      });
+    }
+
+    function excludeAssignedProperty(node: TSESTree.MemberExpression): void {
+      if (isAssignee(node)) {
+        const { excludeSet } =
+          propertiesInfoStack[propertiesInfoStack.length - 1];
+
+        const name =
+          getStaticStringValue(node.property) ??
+          context.sourceCode.getText(node.property);
+
+        if (name) {
+          excludeSet.add(name);
+        }
+      }
     }
 
     return {
@@ -94,14 +166,14 @@ export default createRule<Options, MessageIds>({
             return;
           }
 
-          const name = getMethodName(node);
+          const name = getStringValue(node.key);
 
           if (node.parent.type === AST_NODE_TYPES.ClassBody) {
             const hasDuplicateKeySetter = node.parent.body.some(element => {
               return (
                 element.type === AST_NODE_TYPES.MethodDefinition &&
                 element.kind === 'set' &&
-                getMethodName(element) === name
+                getStringValue(element.key) === name
               );
             });
             if (hasDuplicateKeySetter) {
@@ -132,37 +204,33 @@ export default createRule<Options, MessageIds>({
         },
       }),
       ...(style === 'getters' && {
+        ClassBody: enterClassBody,
+        'ClassBody:exit': exitClassBody,
+        'MethodDefinition[kind="constructor"] ThisExpression'(
+          node: TSESTree.ThisExpression,
+        ): void {
+          if (node.parent.type === AST_NODE_TYPES.MemberExpression) {
+            let parent: TSESTree.Node | undefined = node.parent;
+
+            while (!isFunction(parent)) {
+              parent = parent.parent;
+            }
+
+            if (
+              parent.parent.type === AST_NODE_TYPES.MethodDefinition &&
+              parent.parent.kind === 'constructor'
+            ) {
+              excludeAssignedProperty(node.parent);
+            }
+          }
+        },
         PropertyDefinition(node): void {
           if (!node.readonly || node.declare) {
             return;
           }
-
-          const { value } = node;
-
-          if (!value || !isSupportedLiteral(value)) {
-            return;
-          }
-
-          context.report({
-            node: node.key,
-            messageId: 'preferGetterStyle',
-            suggest: [
-              {
-                messageId: 'preferGetterStyleSuggestion',
-                fix(fixer): TSESLint.RuleFix {
-                  const name = context.sourceCode.getText(node.key);
-
-                  let text = '';
-
-                  text += printNodeModifiers(node, 'get');
-                  text += node.computed ? `[${name}]` : name;
-                  text += `() { return ${context.sourceCode.getText(value)}; }`;
-
-                  return fixer.replaceText(node, text);
-                },
-              },
-            ],
-          });
+          const { properties } =
+            propertiesInfoStack[propertiesInfoStack.length - 1];
+          properties.push(node);
         },
       }),
     };

diff --git a/packages/eslint-plugin/src/rules/prefer-for-of.ts b/packages/eslint-plugin/src/rules/prefer-for-of.ts
@@ -1,7 +1,7 @@
 import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
 
-import { createRule } from '../util';
+import { createRule, isAssignee } from '../util';
 
 export default createRule({
   name: 'prefer-for-of',
@@ -102,60 +102,6 @@ export default createRule({
       );
     }
 
-    function isAssignee(node: TSESTree.Node): boolean {
-      const parent = node.parent;
-      if (!parent) {
-        return false;
-      }
-
-      // a[i] = 1, a[i] += 1, etc.
-      if (
-        parent.type === AST_NODE_TYPES.AssignmentExpression &&
-        parent.left === node
-      ) {
-        return true;
-      }
-
-      // delete a[i]
-      if (
-        parent.type === AST_NODE_TYPES.UnaryExpression &&
-        parent.operator === 'delete' &&
-        parent.argument === node
-      ) {
-        return true;
-      }
-
-      // a[i]++, --a[i], etc.
-      if (
-        parent.type === AST_NODE_TYPES.UpdateExpression &&
-        parent.argument === node
-      ) {
-        return true;
-      }
-
-      // [a[i]] = [0]
-      if (parent.type === AST_NODE_TYPES.ArrayPattern) {
-        return true;
-      }
-
-      // [...a[i]] = [0]
-      if (parent.type === AST_NODE_TYPES.RestElement) {
-        return true;
-      }
-
-      // ({ foo: a[i] }) = { foo: 0 }
-      if (
-        parent.type === AST_NODE_TYPES.Property &&
-        parent.value === node &&
-        parent.parent.type === AST_NODE_TYPES.ObjectExpression &&
-        isAssignee(parent.parent)
-      ) {
-        return true;
-      }
-
-      return false;
-    }
-
     function isIndexOnlyUsedWithArray(
       body: TSESTree.Statement,
       indexVar: TSESLint.Scope.Variable,

diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts
@@ -15,6 +15,7 @@ export * from './isUndefinedIdentifier';
 export * from './misc';
 export * from './objectIterators';
 export * from './types';
+export * from './isAssignee';
 
 // this is done for convenience - saves migrating all of the old rules
 export * from '@typescript-eslint/type-utils';

diff --git a/packages/eslint-plugin/src/util/isAssignee.ts b/packages/eslint-plugin/src/util/isAssignee.ts
@@ -0,0 +1,56 @@
+import type { TSESTree } from '@typescript-eslint/utils';
+import { AST_NODE_TYPES } from '@typescript-eslint/utils';
+
+export function isAssignee(node: TSESTree.Node): boolean {
+  const parent = node.parent;
+  if (!parent) {
+    return false;
+  }
+
+  // a[i] = 1, a[i] += 1, etc.
+  if (
+    parent.type === AST_NODE_TYPES.AssignmentExpression &&
+    parent.left === node
+  ) {
+    return true;
+  }
+
+  // delete a[i]
+  if (
+    parent.type === AST_NODE_TYPES.UnaryExpression &&
+    parent.operator === 'delete' &&
+    parent.argument === node
+  ) {
+    return true;
+  }
+
+  // a[i]++, --a[i], etc.
+  if (
+    parent.type === AST_NODE_TYPES.UpdateExpression &&
+    parent.argument === node
+  ) {
+    return true;
+  }
+
+  // [a[i]] = [0]
+  if (parent.type === AST_NODE_TYPES.ArrayPattern) {
+    return true;
+  }
+
+  // [...a[i]] = [0]
+  if (parent.type === AST_NODE_TYPES.RestElement) {
+    return true;
+  }
+
+  // ({ foo: a[i] }) = { foo: 0 }
+  if (
+    parent.type === AST_NODE_TYPES.Property &&
+    parent.value === node &&
+    parent.parent.type === AST_NODE_TYPES.ObjectExpression &&
+    isAssignee(parent.parent)
+  ) {
+    return true;
+  }
+
+  return false;
+}