fix repeated-member detection; (TODO) need to add fixer; also current implementatio made some assumption about constant and enums

Author: meowbmw

.github/copilot-instructions.md

@@ -0,0 +1 @@
+代码部分请仅使用英文，其他部分使用中文，尽量在说明的同时也给一些实例方便理解

plugins/perf-plugin.ts

@@ -1,4 +1,8 @@
-import { ESLintUtils, TSESTree } from "@typescript-eslint/utils";
+import {
+  AST_NODE_TYPES,
+  ESLintUtils,
+  TSESTree,
+} from "@typescript-eslint/utils";
 
 /**
  * ESlint plugin to detect possible issues that can have impact on performance
@@ -10,7 +14,7 @@ const createRule = ESLintUtils.RuleCreator(
 
 /**
  * Rule: Array Initializer
- * avoid to use [] to init variable
+ * Avoid using [] to initialize variables
  * [] will create a temporary object in data section.
  * BAD
  * let v: i32[] = [];
@@ -77,8 +81,8 @@ const arrayInitStyle: ESLintUtils.RuleModule<
 
 /**
  * Rule: Member Variable / Array Element Get
- * avoid to get member variable multiple-times in the same context.
- * it can significantly increase code size (8% in some proxy like usecase) and waste CPU!!!
+ * Avoid accessing member variables multiple times in the same context.
+ * This can significantly increase code size (8% in some proxy-like usecases) and waste CPU!
  *
  * Implementation overview:
  * - For each outermost MemberExpression (e.g. ctx.data.v1), extract the "object chain" part (e.g. ctx.data).
@@ -134,35 +138,114 @@ const noRepeatedMemberAccess: ESLintUtils.RuleModule<
       let current = node;
       const path: string[] = [];
 
+      // Helper function to skip TSNonNullExpression nodes
+      function skipTSNonNullExpression(
+        node: TSESTree.Node
+      ): TSESTree.Expression {
+        if (node.type === "TSNonNullExpression") {
+          return skipTSNonNullExpression(node.expression);
+        }
+        return node as TSESTree.Expression;
+      }
+
+      // First check if this is part of a switch-case statement
+      let parent = node;
+      while (parent && parent.parent) {
+        parent = parent.parent;
+        if (
+          parent.type === AST_NODE_TYPES.SwitchCase &&
+          parent.test &&
+          (node === parent.test || isDescendant(node, parent.test))
+        ) {
+          return null; // Skip members used in switch-case statements
+        }
+      }
+
+      // Helper function to check if a node is a descendant of another node
+      function isDescendant(
+        node: TSESTree.Node,
+        possibleAncestor: TSESTree.Node
+      ): boolean {
+        let current = node.parent;
+        while (current) {
+          if (current === possibleAncestor) return true;
+          current = current.parent;
+        }
+        return false;
+      }
+
       // Traverse up to the second last member (object chain)
-      while (current && current.type === "MemberExpression") {
+      while (
+        (current && current.type === "MemberExpression") ||
+        current.type === "TSNonNullExpression"
+      ) {
+        // Skip non-null assertions
+        if (current.type === "TSNonNullExpression") {
+          current = current.expression;
+          continue;
+        }
+
         // Only handle static property or static index
         if (current.computed) {
+          // true means access with "[]"
+          // false means access with "."
           if (current.property.type === "Literal") {
-            path.unshift(`[${current.property.raw}]`);
+            // e.g. obj[1], obj["name"]
+            path.unshift(`[${current.property.value}]`);
           } else {
             // Ignore dynamic property access
+            // e.g. obj[var], obj[func()]
             return null;
           }
         } else {
+          // e.g. obj.prop
           path.unshift(`.${current.property.name}`);
         }
+
+        // Check if we've reached the base object
+        let objExpr = current.object;
+        if (objExpr?.type === "TSNonNullExpression") {
+          objExpr = skipTSNonNullExpression(objExpr);
+        }
+
         // If object is not MemberExpression, we've reached the base object
-        if (!current.object || current.object.type !== "MemberExpression") {
+        if (!objExpr || objExpr.type !== "MemberExpression") {
+          // Handle "this" expressions
+          if (objExpr && objExpr.type === "ThisExpression") {
+            path.unshift("this");
+
+            // Skip reporting if the chain is just 'this.property'
+            if (path.length <= 2) {
+              return null;
+            }
+
+            path.pop(); // Remove the last property
+            return path.join("").replace(/^\./, "");
+          }
+
           // If object is Identifier, add it to the path
-          if (current.object && current.object.type === "Identifier") {
-            path.unshift(current.object.name);
+          if (objExpr && objExpr.type === "Identifier") {
+            const baseName = objExpr.name;
+
+            // Skip if the base looks like an enum/constant (starts with capital letter)
+            if (
+              baseName.length > 0 &&
+              baseName[0] === baseName[0].toUpperCase()
+            ) {
+              return null; // Likely an enum or static class
+            }
+
+            path.unshift(baseName);
             // Remove the last property (keep only the object chain)
             path.pop();
             return path.join("").replace(/^\./, "");
           }
           return null;
         }
-        current = current.object;
+        current = objExpr;
       }
       return null;
     }
-
     const occurrences = new Map();
     const minOccurrences = context.options[0]?.minOccurrences || 2;
 
@@ -174,8 +257,47 @@ const noRepeatedMemberAccess: ESLintUtils.RuleModule<
         const objectChain = getObjectChain(node);
         if (!objectChain) return;
 
+        const baseObjectName = objectChain.split(/[.[]/)[0];
         // Use scope range as part of the key
         const scope = context.sourceCode.getScope(node);
+        if (!scope || !scope.block || !scope.block.range) return;
+
+        let variable = null;
+        let currentScope = scope;
+
+        while (currentScope) {
+          variable = currentScope.variables.find(
+            (v) => v.name === baseObjectName
+          );
+          if (variable) break;
+
+          const upperScope = currentScope.upper;
+          if (!upperScope) break;
+          currentScope = upperScope;
+        }
+
+        if (variable) {
+          // check for const/enum/import
+          const isConst = variable.defs.every(
+            (def) => def.node && "kind" in def.node && def.node.kind === "const"
+          );
+          const isEnum = variable.defs.some(
+            (def) =>
+              (def.parent as TSESTree.Node)?.type ===
+              AST_NODE_TYPES.TSEnumDeclaration
+          );
+          const isImport = variable.defs.some(
+            (def) =>
+              def.type === "ImportBinding" ||
+              (def.node &&
+                "type" in def.node &&
+                def.node.type === AST_NODE_TYPES.ImportDeclaration)
+          );
+          if (isConst || isEnum || isImport) {
+            return; // skip these types as extracting them won't be helpful
+          }
+        }
+
         const key = `${scope.block.range.join("-")}-${objectChain}`;
 
         const count = (occurrences.get(key) || 0) + 1;

tests/perf-plugin.test.ts

@@ -10,6 +10,46 @@ RuleTester.itOnly = test.it.only;
 
 const ruleTester = new RuleTester();
 
+// ruleTester.run(
+//   "no-repeated-member-access",
+//   perfPlugin.rules["no-repeated-member-access"],
+//   {
+//     valid: [
+//       `
+//         switch (reason) {
+//             case Test.STARTUP: {
+//                 return "STARTUP"
+//             }
+//             case Test.DEBOUNCE: {
+//                 return "DEBOUNCE"
+//             }
+//             case Test.INSTANT: {
+//                 return "INSTANT"
+//             }
+//             case Test.SHUTDOWN: {
+//                 return "SHUTDOWN"
+//             }
+//             default: {
+//                 return "UNKNOWN"
+//             }
+//         }
+//         `,
+//     ],
+
+//     invalid: [
+//       {
+//         code: `
+//             this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint());
+//             this.vehicleSys!.automobile!.underframe = new ChassisAssembly(new ChassisSchema());
+//             this.vehicleSys!.automobile!.underframe!.propulsionCover = new EngineEnclosure(new EnclosureSpec());
+//             this.vehicleSys!.automobile!.underframe!.logisticsBay = new CargoModule(new ModuleTemplate());
+//             `,
+//         errors: [{ messageId: "repeatedAccess" }],
+//       },
+//     ],
+//   }
+// );
+
 // Throws error if the tests in ruleTester.run() do not pass
 ruleTester.run(
   "array-init-style", // rule name
@@ -154,7 +194,11 @@ ruleTester.run(
             this.vehicleSys!.automobile!.underframe!.propulsionCover = new EngineEnclosure(new EnclosureSpec());
             this.vehicleSys!.automobile!.underframe!.logisticsBay = new CargoModule(new ModuleTemplate());
             `,
-        errors: [{ messageId: "repeatedAccess" }],
+        errors: [
+          { messageId: "repeatedAccess" },
+          { messageId: "repeatedAccess" },
+          { messageId: "repeatedAccess" },
+        ],
       },
     ],
   }