From 2a05148cdfb472e2abede8255b388910cdb254d4 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Fri, 23 May 2025 15:43:13 +0800 Subject: [PATCH 01/23] initial work on no-repeated-member-access rule --- plugins/perfPlugin.ts | 2 + plugins/rules/incorrectReference.ts | 251 ++++++++++++++++++ plugins/rules/memberAccess.ts | 42 +++ roadmap.md | 149 +++++++++++ tests/perfPlugin.test.ts | 1 + tests/rules/noRepeatedMemberAccess.test.ts | 292 +++++++++++++++++++++ 6 files changed, 737 insertions(+) create mode 100644 plugins/rules/incorrectReference.ts create mode 100644 plugins/rules/memberAccess.ts create mode 100644 roadmap.md create mode 100644 tests/rules/noRepeatedMemberAccess.test.ts diff --git a/plugins/perfPlugin.ts b/plugins/perfPlugin.ts index 119db76..f65fdc1 100644 --- a/plugins/perfPlugin.ts +++ b/plugins/perfPlugin.ts @@ -5,9 +5,11 @@ * in AssemblyScript code. */ import arrayInitStyle from "./rules/arrayInitStyle.js"; +import noRepeatedMemberAccess from "./rules/memberAccess.js"; export default { rules: { "array-init-style": arrayInitStyle, + "no-repeated-member-access": noRepeatedMemberAccess, }, }; diff --git a/plugins/rules/incorrectReference.ts b/plugins/rules/incorrectReference.ts new file mode 100644 index 0000000..a59f81c --- /dev/null +++ b/plugins/rules/incorrectReference.ts @@ -0,0 +1,251 @@ +import { + AST_NODE_TYPES, + ESLintUtils, + TSESTree, +} from "@typescript-eslint/utils"; +import { RuleListener, RuleModule } from "@typescript-eslint/utils/ts-eslint"; +import createRule from "../utils/createRule.js"; + +// Define the type for the rule options +type NoRepeatedMemberAccessOptions = [ + { + minOccurrences?: number; + }? +]; + +const noRepeatedMemberAccess: ESLintUtils.RuleModule< + "repeatedAccess", // Message ID type + NoRepeatedMemberAccessOptions, // Options type + unknown, // This parameter is often unused or unknown + ESLintUtils.RuleListener // Listener type +> = createRule({ + name: "no-repeated-member-access", + defaultOptions: [{ minOccurrences: 2 }], // Provide a default object matching the options structure + meta: { + type: "suggestion", + docs: { + description: + "Avoid getting member variable multiple-times in the same context", + }, + fixable: "code", + messages: { + repeatedAccess: + "Try refactor member access to a variable (e.g. 'const temp = {{ path }};') to avoid possible performance loss", + }, + schema: [ + { + type: "object", + properties: { + minOccurrences: { type: "number", minimum: 2 }, + }, + }, + ], + }, + create(context) { + function getObjectChain(node: TSESTree.Node) { + // node is the outermost MemberExpression, e.g. ctx.data.v1 + 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") || + 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") { + // 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 (!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 (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 = objExpr; + } + return null; + } + + // Store nodes for each object chain in each scope for auto-fixing + const chainNodesMap = new Map(); + + const occurrences = new Map(); + const minOccurrences = context.options[0]?.minOccurrences || 2; + + return { + MemberExpression(node) { + // Only check the outermost member expression + if (node.parent && node.parent.type === "MemberExpression") return; + + const objectChain = getObjectChain(node); + if (!objectChain) return; + + const baseObjectName = objectChain.split(/[.[]/)[0]; + // no need to continue if what we extract is the same as the base object + if (objectChain === baseObjectName) return; + + // Use scope range as part of the key + const scope = context.sourceCode.getScope(node); + if (!scope || !scope.block || !scope.block.range) return; + + const key = `${scope.block.range.join("-")}-${objectChain}`; + + // Store node for auto-fixing + if (!chainNodesMap.has(key)) { + chainNodesMap.set(key, []); + } + chainNodesMap.get(key)?.push(node as TSESTree.MemberExpression); + + const count = (occurrences.get(key) || 0) + 1; + occurrences.set(key, count); + + if (count >= minOccurrences) { + context.report({ + node, + messageId: "repeatedAccess", + data: { + path: objectChain, + count: count, + }, + *fix(fixer) { + const nodes = chainNodesMap.get(key); + if (!nodes || nodes.length < minOccurrences) return; + + // Create a safe variable name based on the object chain + const safeVarName = `_${objectChain.replace( + /[^a-zA-Z0-9_]/g, + "_" + )}`; + + // Find the first statement containing the first instance + let statement: TSESTree.Node = nodes[0]; + while ( + statement.parent && + ![ + "Program", + "BlockStatement", + "StaticBlock", + "SwitchCase", + ].includes(statement.parent.type) + ) { + statement = statement.parent; + } + + // Check if the variable already exists in this scope + const scope = context.sourceCode.getScope(nodes[0]); + const variableExists = scope.variables.some( + (v) => v.name === safeVarName + ); + + // Only insert declaration if variable doesn't exist + if (!variableExists) { + yield fixer.insertTextBefore( + statement, + `const ${safeVarName} = ${objectChain};\n` + ); + } + + // Replace ALL occurrences, not just the current node + for (const memberNode of nodes) { + const objText = context.sourceCode.getText(memberNode.object); + if (objText === objectChain) { + yield fixer.replaceText(memberNode.object, safeVarName); + } + } + }, + }); + } + }, + }; + }, +}); \ No newline at end of file diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts new file mode 100644 index 0000000..405882f --- /dev/null +++ b/plugins/rules/memberAccess.ts @@ -0,0 +1,42 @@ +import { + AST_NODE_TYPES, + ESLintUtils, + TSESTree, +} from "@typescript-eslint/utils"; +import { RuleListener, RuleModule } from "@typescript-eslint/utils/ts-eslint"; +import createRule from "../utils/createRule.js"; + +const noRepeatedMemberAccess: ESLintUtils.RuleModule< + "repeatedAccess", // Message ID type + unknown[], + unknown, + ESLintUtils.RuleListener // Listener type +> = createRule({ + name: "no-repeated-member-access", + defaultOptions: [{ minOccurrences: 2 }], // Provide a default object matching the options structure + meta: { + type: "suggestion", + docs: { + description: + "Avoid getting member variable multiple-times in the same context", + }, + fixable: "code", + messages: { + repeatedAccess: + "Try refactor member access to a variable (e.g. 'const temp = {{ path }};') to avoid possible performance loss", + }, + schema: [ + { + type: "object", + properties: { + minOccurrences: { type: "number", minimum: 2 }, + }, + }, + ], + }, + create(context) { + return {}; + }, +}); + +export default noRepeatedMemberAccess; diff --git a/roadmap.md b/roadmap.md new file mode 100644 index 0000000..13455f2 --- /dev/null +++ b/roadmap.md @@ -0,0 +1,149 @@ +# MemberAccess Rule + +前按：本文档聚焦于实现和设计MemberAccess rule，目前已有一版实现，但存在诸多问题，我们将会先给出rule的定义，算法实现，并讨论在生产环境中将会面临的问题以及一些可能的解决思路。 + +实现目的： +首先需要明确我们的规则在尝试解决什么问题，这里将其简要定义如下： +对于a.b.c.d这一类调用，我们需要对于a.b在多次调用情况下将其抽成变量x，并能够实现对于a.b的自动替换，避免在字节码层面多次求值带来的性能影响。 + +算法设计： +基于上述规则，这里简要给出一个实现的算法思路，但实际生产环境中还会有许多其他需要考虑的问题，仅靠此算法不能完全覆盖所有场景。这些问题会在后面进行讨论。 +我们将算法总结为三个部分： +（1）路径统计与最长公共路径检测 +对于a.b.c.d这类结构，我们需要逐层对其解析，形成一个对象链（或称object-chain），得到a-b-c-d +如何实现这个存储结构还有待商榷 +（2）作用域分析 +需要确保我们替换的变量存在于同一个作用域中 +（3）数据流分析 +需要确保替换的变量不存在写入或更改的情况， +检查是否作为赋值目标 +检查是否用于自增/自减操作 +检查是否作为函数参数并被修改 + +```ts +array[i].prop = 1; +i++; // 索引变化 +array[i].prop = 2; // 此处访问了不同元素，不能简单提取 + +// 如果错误地提取为变量: +const temp = array[i]; // 提取初始索引处的元素引用 +temp.prop = 1; +i++; // 索引变化 +temp.prop = 2; // 错误：仍然修改的是原始索引处的元素 +``` + +在生产环境下实际需要解决的问题： +Rule本身是十分容易理解的，但是将其用于生产环境下就有可能会面临许多corner case需要解决，如果不将其全面的考虑进去，必然会导致最后实现的rule在不应该发生替换的地方进行替换，甚至改变程序的原有实现逻辑。 +这类问题可能较多，且较复杂 +我们对一些必须要考虑的case做归纳如下： +（1）需跳过原型方法访问 + +```ts +const arr = [1, 2, 3]; +arr.push(4); // 不应将 arr 提取为变量 +arr.length; // 同上 +``` + +解决方案 + +```ts +function isPrototypeAccess(node: TSESTree.MemberExpression): boolean { + const props = new Set(["__proto__", "constructor", "prototype"]); + return node.property.type === "Identifier" && props.has(node.property.name); +} +``` + +（2）处理解构场景 + +```ts +const { c, d } = a.b; +``` + +解决方案 + +```ts +function isPrototypeAccess(node: TSESTree.MemberExpression): boolean { + const props = new Set(["__proto__", "constructor", "prototype"]); + return node.property.type === "Identifier" && props.has(node.property.name); +} +``` + +（3）对于长链情况的处理 +对于a.b.c.d.e.f.g，如果是a.b.c.d.e被多次使用，那么就直接将其抽取为一个变量y，而不是分别先抽取a.b，再抽取a.b.c,以此类推 +但也有可能会存在a.b.c被多次使用，而被抽取为变量的情况 + +1. 长链提取方式的探讨 + +```ts +// 原始代码 +a.b.c.d.e.f.g(); +a.b.c.d.e.h(); +a.b.c.i(); +a.b.c.j(); + +// 可能的提取方案（贪婪提取） +const temp1 = a.b.c; +const temp2 = temp1.d.e; +temp2.f.g(); +temp2.h(); +temp1.i(); +temp1.j(); + +// 或者更保守的方案（只提取最频繁的） +const temp = a.b.c; +temp.d.e.f.g(); +temp.d.e.h(); +temp.i(); +temp.j(); +``` + +2. 重叠长链优先级问题 + +```ts +// 当有多个重叠的长链时，应该如何选择？ +a.b.c.d.e.f(); +a.b.c.d.e.g(); +a.b.c.h(); +a.b.c.i(); + +// 应该优先提取 a.b.c 还是 a.b.c.d.e? +// 这可能需要基于使用频率和长度的权衡算法 +``` + +3. 链中存在副作用 + +```ts +// 如果链中某部分有副作用，应避免多次提取 +a.getB().c.d(); // getB()可能有副作用 +a.getB().c.e(); + +// 错误提取 +const temp = a.getB().c; // getB()只执行一次！ +temp.d(); +temp.e(); +``` + +4. 链的中间部分被重用但两端不同 + +```ts +a.b.c.d.e.f; +g.h.c.d.e.i; + +// 这种情况 c.d.e 被重用，但前后部分不同 +// 可能需要特殊处理或放弃提取 +``` + +5. 多条链交错使用 + +```ts +a.b.c.d(); +x.y.z(); +a.b.c.e(); +x.y.w(); + +// 由于代码交错，提取变量可能会改变执行顺序 +// 需要确保提取后不会影响程序行为 +``` + +（4）作用域及闭包可能带来的问题 +（5）变量名发生冲突时的情况（对于autofixer） diff --git a/tests/perfPlugin.test.ts b/tests/perfPlugin.test.ts index becd280..143227a 100644 --- a/tests/perfPlugin.test.ts +++ b/tests/perfPlugin.test.ts @@ -7,6 +7,7 @@ import { describe } from "mocha"; // Import individual rule tests to run them as part of the test suite import "./rules/arrayInitStyle.test.js"; +import "./rules/noRepeatedMemberAccess.test.js"; describe("AssemblyScript Performance ESLint Plugin", () => { // Test suite is composed of individual rule tests imported above diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts new file mode 100644 index 0000000..ee6e730 --- /dev/null +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -0,0 +1,292 @@ +import { describe, it } from "mocha"; +import { createRuleTester } from "../utils/testUtils.js"; +import noRepeatedMemberAccess from "../../plugins/rules/memberAccess.js"; + +describe("Rule: no-spread", () => { + const ruleTester = createRuleTester(); + + it("validates all test cases for no-repeated-member-access rule", () => { + ruleTester.run("no-repeated-member-access", noRepeatedMemberAccess, { + valid: [ + // Basic valid case + ` + const data = ctx.data[0]; + const v1 = data.v1; + `, + + // Different scopes + ` + function test() { + const a = obj.foo.bar; + } + function test2() { + const b = obj.foo.bar; + } + `, + // Dynamic property access (should be ignored) + ` + const v1 = ctx[method()].value; + const v2 = ctx[method()].value; + `, + ` + 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" + } + } + `, + ` + import { Juice } from "@applejuice" + export const apple = Juice.D31 + export const banana = Juice.D32 + export const cat = Juice.D33 + `, + ` +export namespace Constants { + export const defaultDebounceTimeRegular_s: u32 = SendTime.NEXT_REGULAR + export const debounceTimeGnssInnerCity_s = SendTime.S30 + export const debounceTimeGnssMotorway_s = SendTime.S120 +} + `, + + // some more advanced cases + ` + /** + * Distance-based emission strategy + */ + export class DistanceBasedDeliveryStrategy implements EmissionStrategy { + private checkStateChange(attr: NumericProperty): bool { + return ( + attr.getCurrent().isUnstable() && + attr.getPrevious() != null && + attr.getPrevious()!.isStable() + ); + } + + public isEqual(other: GenericValueContainer): bool { + return ( + this.isStable() == other.isStable() && + this.isActive() == other.isActive() && + (!this.isActive() || this.getValue() == other.getValue()) + ); + } + + public copy(): GenericValueContainer { + const clone = new LongValueWrapper(this.initializer); + clone.status = this.status; + clone.error = this.error; + return clone; + } + }`, + ` + + let activeConfig: BaseConfiguration; + const configData = AppContext.loadSettings(); + + if (configData) { + activeConfig = configData; + } else { + activeConfig = new BaseConfiguration(); + activeConfig.initializationDelay = Constants.DEFAULT_INIT_DELAY; + activeConfig.fastPollingInterval = Constants.DEFAULT_FAST_INTERVAL; + activeConfig.normalPollingInterval = Constants.DEFAULT_NORMAL_INTERVAL; + } + + if (runtimeEnv && eventController) { + SystemHookManager.instance = this; + this.eventController = eventController; + runtimeEnv.registerStateChangeListener(SystemHookManager.handleStateChange); + this.lastKnownState = runtimeEnv.getCurrentEnvironmentState(); + } + `, + ` + if ( + currentSession.authType == AuthType.PRIVILEGED && + currentSession.status == SessionStatus.ACTIVE && + this.runtimeEnv.getCurrentEnvironmentState().isPresent() + ) { + const timestamp = getTimestamp(); + serviceInstance.lastSyncTime = timestamp; + } + + `, + ], + + invalid: [ + // Basic invalid case + { + code: ` + const v1 = ctx.data.v1; + const v2 = ctx.data.v2; + `, + errors: [{ messageId: "repeatedAccess" }], + output: ` + const _ctx_data = ctx.data; +const v1 = _ctx_data.v1; + const v2 = _ctx_data.v2; + `, + }, + { + code: ` + class User { + constructor() { + this.profile = service.user.profile + this.log = service.user.logger + } + }`, + errors: [{ messageId: "repeatedAccess" }], + output: + "\n" + + " class User {\n" + + " constructor() {\n" + + " const _service_user = service.user;\n" + + "this.profile = _service_user.profile\n" + + " this.log = _service_user.logger\n" + + " }\n" + + " }", + }, + // Nested scope case + { + code: ` + function demo() { + console.log(obj.a.b.c); + return obj.a.b.d; + } + `, + errors: [{ messageId: "repeatedAccess" }], + output: + "\n" + + " function demo() {\n" + + " const _obj_a_b = obj.a.b;\n" + + "console.log(_obj_a_b.c);\n" + + " return _obj_a_b.d;\n" + + " }\n" + + " ", + }, + + // Array index case + { + code: ` + const x = data[0].value; + data[0].count++; + send(data[0].id); + `, + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], + output: + "\n" + + " const _data_0_ = data[0];\n" + + "const x = _data_0_.value;\n" + + " _data_0_.count++;\n" + + " send(_data_0_.id);\n" + + " ", + }, + { + code: ` + const x = data[0][1].value; + data[0][1].count++; + send(data[0][1].id); + `, + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], + output: + "\n" + + " const _data_0__1_ = data[0][1];\n" + + "const x = _data_0__1_.value;\n" + + " _data_0__1_.count++;\n" + + " send(_data__0_1_.id);\n" + + " ", + }, + { + code: ` + const a = dataset[0][1].x + dataset[0][1].y; + dataset[0][1].update(); + const b = dataset[0][1].z * 2; + notify(dataset[0][1].timestamp); + `, + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], + output: ` + const _dataset_0_1_ = dataset[0][1]; + const a = _dataset_0_1_.x + _dataset_0_1_.y; + _dataset_0_1_.update(); + const b = _dataset_0_1_.z * 2; + notify(_dataset_0_1_.timestamp); + `, + }, + { + code: ` + const first = data.items[0].config['security'].rules[2].level; + data.items[0].config['security'].rules[2].enabled = true; + validate(data.items[0].config['security'].rules[2].level); + `, + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], + output: ` + const _data_items_0_config_security_rules_2_ = data.items[0].config['security'].rules[2]; + const first = _data_items_0_config_security_rules_2_.level; + _data_items_0_config_security_rules_2_.enabled = true; + validate(_data_items_0_config_security_rules_2_.level); + `, + }, + { + 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" }, + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], + output: [ + "\n" + + " const _this_vehicleSys = this.vehicleSys;\n" + + "this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint());\n" + + " const _this_vehicleSys_automobile = this.vehicleSys.automobile;\n" + + "this.vehicleSys!.automobile!.underframe = new ChassisAssembly(new ChassisSchema());\n" + + " const _this_vehicleSys_automobile_underframe = this.vehicleSys.automobile.underframe;\n" + + "this.vehicleSys!.automobile!.underframe!.propulsionCover = new EngineEnclosure(new EnclosureSpec());\n" + + " this.vehicleSys!.automobile!.underframe!.logisticsBay = new CargoModule(new ModuleTemplate());\n" + + " ", + "\n" + + " const _this_vehicleSys = this.vehicleSys;\n" + + "this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint());\n" + + " const _this_vehicleSys_automobile = _this_vehicleSys.automobile;\n" + + "this.vehicleSys!.automobile!.underframe = new ChassisAssembly(new ChassisSchema());\n" + + " const _this_vehicleSys_automobile_underframe = _this_vehicleSys_automobile.underframe;\n" + + "this.vehicleSys!.automobile!.underframe!.propulsionCover = new EngineEnclosure(new EnclosureSpec());\n" + + " this.vehicleSys!.automobile!.underframe!.logisticsBay = new CargoModule(new ModuleTemplate());\n" + + " ", + ], + }, + ], + }); + }); +}); From 00f054990dd07e66728130149b175d2a812b829c Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 26 May 2025 20:41:41 +0800 Subject: [PATCH 02/23] second attempt; still trying to figure this out --- plugins/rules/memberAccess.ts | 322 +++++++++++++++++++++++++++++++--- 1 file changed, 301 insertions(+), 21 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 405882f..d791438 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -1,41 +1,321 @@ -import { - AST_NODE_TYPES, - ESLintUtils, - TSESTree, -} from "@typescript-eslint/utils"; -import { RuleListener, RuleModule } from "@typescript-eslint/utils/ts-eslint"; +import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/utils"; +import { Scope } from "@typescript-eslint/utils/ts-eslint"; import createRule from "../utils/createRule.js"; -const noRepeatedMemberAccess: ESLintUtils.RuleModule< - "repeatedAccess", // Message ID type - unknown[], - unknown, - ESLintUtils.RuleListener // Listener type -> = createRule({ +const noRepeatedMemberAccess = createRule({ name: "no-repeated-member-access", - defaultOptions: [{ minOccurrences: 2 }], // Provide a default object matching the options structure meta: { type: "suggestion", docs: { description: - "Avoid getting member variable multiple-times in the same context", + "Optimize repeated member access patterns by extracting variables", }, fixable: "code", - messages: { - repeatedAccess: - "Try refactor member access to a variable (e.g. 'const temp = {{ path }};') to avoid possible performance loss", - }, schema: [ { type: "object", properties: { - minOccurrences: { type: "number", minimum: 2 }, + minOccurrences: { type: "number", minimum: 2, default: 3 }, + maxChainDepth: { type: "number", minimum: 1, maximum: 5, default: 3 }, }, }, ], + messages: { + repeatedAccess: + "Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.", + modifiedChain: + "Cannot optimize '{{ chain }}' due to potential modification.", + }, }, - create(context) { - return {}; + defaultOptions: [{ minOccurrences: 3, maxChainDepth: 3 }], + + create(context, [options]) { + const sourceCode = context.sourceCode; + const minOccurrences = options?.minOccurrences ?? 3; + const maxChainDepth = options?.maxChainDepth ?? 3; + + // ====================== + // Scope Management System + // ====================== + type ScopeData = { + chains: Map< + string, + { + count: number; + nodes: TSESTree.MemberExpression[]; + modified: boolean; + } + >; + variables: Set; + }; + + const scopeDataMap = new WeakMap(); + + function getScopeData(scope: Scope.Scope): ScopeData { + if (!scopeDataMap.has(scope)) { + scopeDataMap.set(scope, { + chains: new Map(), + variables: new Set(scope.variables.map((v) => v.name)), + }); + } + return scopeDataMap.get(scope)!; + } + + // ====================== + // Path Analysis System + // ====================== + interface ChainInfo { + hierarchy: string[]; // Chain hierarchy (e.g., ["a", "a.b", "a.b.c"]) + fullChain: string; // Complete path (e.g., "a.b.c") + } + + const chainCache = new WeakMap< + TSESTree.MemberExpression, + ChainInfo | null + >(); + + function analyzeChain(node: TSESTree.MemberExpression): ChainInfo | null { + if (chainCache.has(node)) return chainCache.get(node)!; + + const parts: string[] = []; + let current: TSESTree.Node = node; + let isValid = true; + + // Collect property chain (reverse order) + while (current.type === AST_NODE_TYPES.MemberExpression) { + if (current.computed) { + if (current.property.type === AST_NODE_TYPES.Literal) { + parts.push(`[${current.property.value}]`); + } else { + isValid = false; + break; + } + } else { + parts.push(current.property.name); + } + + current = current.object; + } + + // Handle base object + if (current.type === AST_NODE_TYPES.Identifier) { + parts.push(current.name); + } else { + isValid = false; + } + + if (!isValid || parts.length > maxChainDepth + 1) { + chainCache.set(node, null); + return null; + } + + // Generate hierarchy chain (forward order) + parts.reverse(); + const reversedParts = parts; + const hierarchy = reversedParts.reduce((acc, part, index) => { + const chain = index === 0 ? part : `${acc[index - 1]}.${part}`; + return [...acc, chain]; + }, [] as string[]); + + const result = { + hierarchy: hierarchy.slice(0, maxChainDepth + 1), + fullChain: hierarchy.at(-1) ?? "", // Add default empty string + }; + + chainCache.set(node, result); + return result; + } + + // ====================== + // Modification Tracking System + // ====================== + const modificationRegistry = new Map(); + + function trackModification(chain: string, node: TSESTree.Node) { + if (!modificationRegistry.has(chain)) { + modificationRegistry.set(chain, []); + } + modificationRegistry.get(chain)!.push(node); + } + + // ====================== + // Variable Naming System + // ====================== + function generateVarName(parts: string[], scope: Scope.Scope): string { + const base = parts + .map((p, i) => (i === 0 ? p : p[0].toUpperCase() + p.slice(1))) + .join("") + .replace(/[^a-zA-Z0-9]/g, "_"); + + let name = base; + let suffix = 1; + const scopeData = getScopeData(scope); + + while (scopeData.variables.has(name)) { + name = `${base}${suffix++}`; + } + + scopeData.variables.add(name); + return name; + } + + // ====================== + // Core Processing Logic (Improved Hierarchy Tracking) + // ====================== + function processMemberExpression(node: TSESTree.MemberExpression) { + if (node.parent?.type === AST_NODE_TYPES.MemberExpression) return; + + const chainInfo = analyzeChain(node); + if (!chainInfo) return; + + const scope = sourceCode.getScope(node); + const scopeData = getScopeData(scope); + + // Update hierarchy chain statistics + for (const chain of chainInfo.hierarchy) { + const record = scopeData.chains.get(chain) || { + count: 0, + nodes: [], + modified: false, + }; + record.count++; + record.nodes.push(node); + scopeData.chains.set(chain, record); + } + + // Check for optimal chain immediately + const optimalChain = findOptimalChain(chainInfo.hierarchy, scopeData); + if ( + optimalChain && + scopeData.chains.get(optimalChain)?.count === minOccurrences + ) { + reportOptimization(optimalChain, scopeData, scope); + } + } + + function findOptimalChain( + hierarchy: string[], + scopeData: ScopeData + ): string | null { + for (let i = hierarchy.length - 1; i >= 0; i--) { + const chain = hierarchy[i]; + const record = scopeData.chains.get(chain); + if (record && record.count >= minOccurrences) { + return chain; + } + } + return null; + } + + // ====================== + // Optimization Reporting Logic + // ====================== + function reportOptimization( + chain: string, + scopeData: ScopeData, + scope: Scope.Scope + ) { + const record = scopeData.chains.get(chain)!; + const modifications = modificationRegistry.get(chain) || []; + const lastModification = modifications.at(-1); + + context.report({ + node: record.nodes[0], + messageId: "repeatedAccess", + data: { + chain: chain, + count: record.count, + }, + *fix(fixer) { + // Check for subsequent modifications + if ( + lastModification && + lastModification.range[0] < record.nodes[0].range[0] + ) { + return; + } + + // Generate variable name (based on chain hierarchy) + const parts = chain.split("."); + const varName = generateVarName(parts, scope); + + // Insert variable declaration + const insertPosition = findInsertPosition(scope); + yield fixer.insertTextBefore( + insertPosition.node, + `const ${varName} = ${chain};\n` + ); + + // Replace all matching nodes + for (const memberNode of record.nodes) { + const fullChain = analyzeChain(memberNode)?.fullChain; + if (fullChain?.startsWith(chain)) { + const remaining = fullChain.slice(chain.length + 1); + yield fixer.replaceText( + memberNode, + `${varName}${remaining ? "." + remaining : ""}` + ); + } + } + + // Update scope information + scopeData.variables.add(varName); + }, + }); + } + + // ====================== + // Helper Functions + // ====================== + function findInsertPosition(scope: Scope.Scope): { + node: TSESTree.Node; + isGlobal: boolean; + } { + if (scope.block.type === AST_NODE_TYPES.Program) { + return { node: scope.block.body[0], isGlobal: true }; + } + return { + node: (scope.block as TSESTree.BlockStatement).body[0], + isGlobal: false, + }; + } + + // ====================== + // Rule Listeners + // ====================== + return { + MemberExpression: (node) => processMemberExpression(node), + + AssignmentExpression: (node) => { + if (node.left.type === AST_NODE_TYPES.MemberExpression) { + const chainInfo = analyzeChain(node.left); + if (chainInfo) { + for (const chain of chainInfo.hierarchy) + trackModification(chain, node); + } + } + }, + + UpdateExpression: (node) => { + if (node.argument.type === AST_NODE_TYPES.MemberExpression) { + const chainInfo = analyzeChain(node.argument); + if (chainInfo) { + for (const chain of chainInfo.hierarchy) + trackModification(chain, node); + } + } + }, + + CallExpression: (node) => { + if (node.callee.type === AST_NODE_TYPES.MemberExpression) { + const chainInfo = analyzeChain(node.callee); + if (chainInfo) { + for (const chain of chainInfo.hierarchy) + trackModification(chain, node); + } + } + }, + }; }, }); From 434d4e8e75fc8fa0f90455b101b1918e04f94988 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Tue, 3 Jun 2025 17:40:33 +0800 Subject: [PATCH 03/23] Add no repeated member access rule --- cspell/project-words.txt | 1 + docs/rules/no-repeated-member-access.md | 107 +++++++ plugins/rules/incorrectReference.ts | 251 --------------- plugins/rules/memberAccess.ts | 350 +++++++++++---------- roadmap.md | 149 --------- tests/rules/noRepeatedMemberAccess.test.ts | 207 +++++------- 6 files changed, 370 insertions(+), 695 deletions(-) create mode 100644 docs/rules/no-repeated-member-access.md delete mode 100644 plugins/rules/incorrectReference.ts delete mode 100644 roadmap.md diff --git a/cspell/project-words.txt b/cspell/project-words.txt index 81e6df9..154e4cb 100644 --- a/cspell/project-words.txt +++ b/cspell/project-words.txt @@ -17,6 +17,7 @@ typechecker tsconfig tseslint tses +sonarjs // AssemblyScript types and keywords i8 diff --git a/docs/rules/no-repeated-member-access.md b/docs/rules/no-repeated-member-access.md new file mode 100644 index 0000000..3138d46 --- /dev/null +++ b/docs/rules/no-repeated-member-access.md @@ -0,0 +1,107 @@ +# no-repeated-member-access + +> Optimize repeated member access patterns by extracting variables + +## Rule Details + +This rule identifies repeated member access patterns in your code and suggests extracting them to variables for better performance and readability. In AssemblyScript, repeated property access can have performance implications (due to when they are compiled to WASM bytecode, they will induce more instructions), especially in loops or frequently called functions. + +This rule doesn't extract computed properties/array index. These can change unexpectedly and therefore should be avoid for extraction. Examples include: + +```ts +arr[0]; +arr[0][1]; +arr[0].property; +obj.arr[0].value; +data.items[0].config; +obj["prop"]; +obj[getKey()]; +``` + +The rule will also avoid to warn when functions are invoked upon properties, as this could have implications that alter the extracted value. +Examples include: + +```ts +x = a.b.c; +a.b.doSomething(); // this line will prevent a.b.c from being warned although it is used multiple times, as doSomething() could potentially change the value of a.b +y = a.b.c; +z = a.b.c; +``` + +## Rule Options + +This rule accepts an options object with the following properties: + +```json +{ + "minOccurrences": 3 +} +``` + +- `minOccurrences` (default: 3): Minimum number of times a member chain must be accessed before triggering the rule + +## Examples + +### Incorrect + +```ts +// Repeated access to the same property chain (3+ times) +function processData(obj: MyObject): void { + if (obj.config.settings.enabled) { + obj.config.settings.value = 10; + console.log(obj.config.settings.name); + obj.config.settings.timestamp = Date.now(); + } +} + +// Deep property chains accessed multiple times +function renderUI(app: Application): void { + app.ui.layout.header.title.text = "New Title"; + app.ui.layout.header.title.fontSize = 16; + app.ui.layout.header.title.color = "blue"; +} +``` + +### Correct + +```ts +// Extract repeated property access to variables +function processData(obj: MyObject): void { + const settings = obj.config.settings; + if (settings.enabled) { + settings.value = 10; + console.log(settings.name); + settings.timestamp = Date.now(); + } +} + +// Extract deep property chains +function renderUI(app: Application): void { + const title = app.ui.layout.header.title; + title.text = "New Title"; + title.fontSize = 16; + title.color = "blue"; +} + +// Single or infrequent access is allowed +function singleAccess(obj: MyObject): void { + console.log(obj.config.settings.enabled); // Only accessed once +} +``` + +## Benefits + +- **Performance**: Reduces redundant property lookups, especially in tight loops +- **Readability**: Makes code more readable by giving meaningful names to complex property chains +- **Maintainability**: Easier to update property references when extracted to variables +- **Memory Efficiency**: Can reduce memory pressure in performance-critical AssemblyScript code + +## When Not To Use + +- If the property chains are very short (single level) and performance is not critical +- When the object properties are frequently modified, making extraction less beneficial +- In very simple functions where the overhead of variable extraction outweighs the benefits + +## Related Rules + +- Consider using this rule alongside other performance-focused rules for optimal AssemblyScript code generation diff --git a/plugins/rules/incorrectReference.ts b/plugins/rules/incorrectReference.ts deleted file mode 100644 index a59f81c..0000000 --- a/plugins/rules/incorrectReference.ts +++ /dev/null @@ -1,251 +0,0 @@ -import { - AST_NODE_TYPES, - ESLintUtils, - TSESTree, -} from "@typescript-eslint/utils"; -import { RuleListener, RuleModule } from "@typescript-eslint/utils/ts-eslint"; -import createRule from "../utils/createRule.js"; - -// Define the type for the rule options -type NoRepeatedMemberAccessOptions = [ - { - minOccurrences?: number; - }? -]; - -const noRepeatedMemberAccess: ESLintUtils.RuleModule< - "repeatedAccess", // Message ID type - NoRepeatedMemberAccessOptions, // Options type - unknown, // This parameter is often unused or unknown - ESLintUtils.RuleListener // Listener type -> = createRule({ - name: "no-repeated-member-access", - defaultOptions: [{ minOccurrences: 2 }], // Provide a default object matching the options structure - meta: { - type: "suggestion", - docs: { - description: - "Avoid getting member variable multiple-times in the same context", - }, - fixable: "code", - messages: { - repeatedAccess: - "Try refactor member access to a variable (e.g. 'const temp = {{ path }};') to avoid possible performance loss", - }, - schema: [ - { - type: "object", - properties: { - minOccurrences: { type: "number", minimum: 2 }, - }, - }, - ], - }, - create(context) { - function getObjectChain(node: TSESTree.Node) { - // node is the outermost MemberExpression, e.g. ctx.data.v1 - 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") || - 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") { - // 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 (!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 (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 = objExpr; - } - return null; - } - - // Store nodes for each object chain in each scope for auto-fixing - const chainNodesMap = new Map(); - - const occurrences = new Map(); - const minOccurrences = context.options[0]?.minOccurrences || 2; - - return { - MemberExpression(node) { - // Only check the outermost member expression - if (node.parent && node.parent.type === "MemberExpression") return; - - const objectChain = getObjectChain(node); - if (!objectChain) return; - - const baseObjectName = objectChain.split(/[.[]/)[0]; - // no need to continue if what we extract is the same as the base object - if (objectChain === baseObjectName) return; - - // Use scope range as part of the key - const scope = context.sourceCode.getScope(node); - if (!scope || !scope.block || !scope.block.range) return; - - const key = `${scope.block.range.join("-")}-${objectChain}`; - - // Store node for auto-fixing - if (!chainNodesMap.has(key)) { - chainNodesMap.set(key, []); - } - chainNodesMap.get(key)?.push(node as TSESTree.MemberExpression); - - const count = (occurrences.get(key) || 0) + 1; - occurrences.set(key, count); - - if (count >= minOccurrences) { - context.report({ - node, - messageId: "repeatedAccess", - data: { - path: objectChain, - count: count, - }, - *fix(fixer) { - const nodes = chainNodesMap.get(key); - if (!nodes || nodes.length < minOccurrences) return; - - // Create a safe variable name based on the object chain - const safeVarName = `_${objectChain.replace( - /[^a-zA-Z0-9_]/g, - "_" - )}`; - - // Find the first statement containing the first instance - let statement: TSESTree.Node = nodes[0]; - while ( - statement.parent && - ![ - "Program", - "BlockStatement", - "StaticBlock", - "SwitchCase", - ].includes(statement.parent.type) - ) { - statement = statement.parent; - } - - // Check if the variable already exists in this scope - const scope = context.sourceCode.getScope(nodes[0]); - const variableExists = scope.variables.some( - (v) => v.name === safeVarName - ); - - // Only insert declaration if variable doesn't exist - if (!variableExists) { - yield fixer.insertTextBefore( - statement, - `const ${safeVarName} = ${objectChain};\n` - ); - } - - // Replace ALL occurrences, not just the current node - for (const memberNode of nodes) { - const objText = context.sourceCode.getText(memberNode.object); - if (objText === objectChain) { - yield fixer.replaceText(memberNode.object, safeVarName); - } - } - }, - }); - } - }, - }; - }, -}); \ No newline at end of file diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index d791438..1f1c69b 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -16,153 +16,215 @@ const noRepeatedMemberAccess = createRule({ type: "object", properties: { minOccurrences: { type: "number", minimum: 2, default: 3 }, - maxChainDepth: { type: "number", minimum: 1, maximum: 5, default: 3 }, }, }, ], messages: { repeatedAccess: "Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.", - modifiedChain: - "Cannot optimize '{{ chain }}' due to potential modification.", }, }, - defaultOptions: [{ minOccurrences: 3, maxChainDepth: 3 }], + defaultOptions: [{ minOccurrences: 2 }], create(context, [options]) { const sourceCode = context.sourceCode; - const minOccurrences = options?.minOccurrences ?? 3; - const maxChainDepth = options?.maxChainDepth ?? 3; + const minOccurrences = options?.minOccurrences ?? 2; + + // Track which chains have already been reported to avoid duplicate reports + const reportedChains = new Set(); - // ====================== - // Scope Management System - // ====================== type ScopeData = { chains: Map< string, { - count: number; - nodes: TSESTree.MemberExpression[]; - modified: boolean; + count: number; // Number of times this chain is accessed + nodes: TSESTree.MemberExpression[]; // AST nodes where this chain appears + modified: boolean; // Whether this chain is modified (written to) } >; - variables: Set; + variables: Set; // Variables already declared in this scope }; + // Stores data for each scope using WeakMap to avoid memory leaks const scopeDataMap = new WeakMap(); function getScopeData(scope: Scope.Scope): ScopeData { + // Creates new scope data if none exists + // Example: First time seeing the function foo() { obj.prop.val; } + // we create a new ScopeData for this function if (!scopeDataMap.has(scope)) { - scopeDataMap.set(scope, { - chains: new Map(), - variables: new Set(scope.variables.map((v) => v.name)), - }); + // Create new scope data + const newScopeData = { + chains: new Map< + string, + { + count: number; + nodes: TSESTree.MemberExpression[]; + modified: boolean; + } + >(), + variables: new Set(), + }; + + // Add existing variable names to the set + for (let i = 0; i < scope.variables.length; i++) { + const variable = scope.variables[i]; + newScopeData.variables.add(variable.name); + } + + scopeDataMap.set(scope, newScopeData); } + return scopeDataMap.get(scope)!; } - // ====================== - // Path Analysis System - // ====================== + // This part analyzes and extracts member access chains from AST nodes + // + // Examples of chains: + // - a.b.c → hierarchy: ["a", "a.b", "a.b.c"], fullChain: "a.b.c" + // - foo["bar"].baz → hierarchy: ["foo", "foo[bar]", "foo[bar].baz"], fullChain: "foo[bar].baz" + // - user.profile.settings.theme → hierarchy: ["user", "user.profile", "user.profile.settings"], fullChain: "user.profile.settings" interface ChainInfo { hierarchy: string[]; // Chain hierarchy (e.g., ["a", "a.b", "a.b.c"]) fullChain: string; // Complete path (e.g., "a.b.c") } - + // Cache analyzed expressions to improve performance const chainCache = new WeakMap< TSESTree.MemberExpression, ChainInfo | null >(); function analyzeChain(node: TSESTree.MemberExpression): ChainInfo | null { + // Check cache first for performance + // Example: If we've already analyzed a.b.c in a previous call, + // we return the cached result immediately if (chainCache.has(node)) return chainCache.get(node)!; - const parts: string[] = []; - let current: TSESTree.Node = node; - let isValid = true; + const parts: string[] = []; // Stores parts of the chain in reverse order + let current: TSESTree.Node = node; // Current node in traversal + let isValid = true; // Whether this chain is valid for optimization // Collect property chain (reverse order) + // Example: For a.b.c, we'd collect ["c", "b", "a"] initially while (current.type === AST_NODE_TYPES.MemberExpression) { if (current.computed) { - if (current.property.type === AST_NODE_TYPES.Literal) { - parts.push(`[${current.property.value}]`); - } else { - isValid = false; - break; - } + // skip computed properties like obj["prop"] or arr[0] or obj[getKey()] + isValid = false; + break; } else { + // Handle dot notation like obj.prop parts.push(current.property.name); } - current = current.object; + current = current.object; // Move to parent object + + // Handle TSNonNullExpression (the ! operator) + while (current.type === AST_NODE_TYPES.TSNonNullExpression) { + current = current.expression; + } } - // Handle base object + // Handle base object (the root of the chain) + // Example: For a.b.c, the base object is "a" if (current.type === AST_NODE_TYPES.Identifier) { - parts.push(current.name); + parts.push(current.name); // Add base object name + } else if (current.type === AST_NODE_TYPES.ThisExpression) { + parts.push("this"); } else { + // Skip chains with non-identifier base objects + // Example: (getObject()).prop is not optimized because + // function call results shouldn't be cached isValid = false; } - if (!isValid || parts.length > maxChainDepth + 1) { + // Validate chain + if (!isValid || parts.length < 2) { chainCache.set(node, null); return null; } // Generate hierarchy chain (forward order) + // Example: For parts ["c", "b", "a"], we reverse to ["a", "b", "c"] + // and build hierarchy ["a", "a.b", "a.b.c"] parts.reverse(); - const reversedParts = parts; - const hierarchy = reversedParts.reduce((acc, part, index) => { - const chain = index === 0 ? part : `${acc[index - 1]}.${part}`; - return [...acc, chain]; - }, [] as string[]); + + const hierarchy: string[] = []; + for (let i = 0; i < parts.length; i++) { + let chain; + // Create chain for each level + // eslint-disable-next-line unicorn/prefer-ternary + if (i === 0) { + // First element is used directly + // Example: For a.b.c, first element is "a" + chain = parts[0]; + } else { + // Build based on previous element + // Example: "a" + "." + "b" = "a.b" + chain = hierarchy[i - 1] + "." + parts[i]; + } + hierarchy.push(chain); + } const result = { - hierarchy: hierarchy.slice(0, maxChainDepth + 1), - fullChain: hierarchy.at(-1) ?? "", // Add default empty string + hierarchy: hierarchy, + fullChain: hierarchy.at(-1) ?? "", // Use last element or empty string }; + // Cache and return the result chainCache.set(node, result); return result; } - // ====================== - // Modification Tracking System - // ====================== - const modificationRegistry = new Map(); + // Tracks which chains are modified in code to avoid incorrect optimizations + // + // Examples of modifications: + // 1. obj.prop = value; // Direct assignment + // 2. obj.prop++; // Increment/decrement + // 3. updateValues(obj.prop); // Potential modification through function call function trackModification(chain: string, node: TSESTree.Node) { - if (!modificationRegistry.has(chain)) { - modificationRegistry.set(chain, []); - } - modificationRegistry.get(chain)!.push(node); - } - - // ====================== - // Variable Naming System - // ====================== - function generateVarName(parts: string[], scope: Scope.Scope): string { - const base = parts - .map((p, i) => (i === 0 ? p : p[0].toUpperCase() + p.slice(1))) - .join("") - .replace(/[^a-zA-Z0-9]/g, "_"); - - let name = base; - let suffix = 1; + const scope = sourceCode.getScope(node); const scopeData = getScopeData(scope); - while (scopeData.variables.has(name)) { - name = `${base}${suffix++}`; + // Mark the modified chain and all its sub-chains as modified + // Example: If "a.b" is modified, then "a.b.c", "a.b.c.d" etc. should also be considered invalid + for (const [existingChain, record] of scopeData.chains) { + // Check if the existing chain starts with the modified chain followed by a dot or bracket + // This handles cases where modifying "a.b" should invalidate "a.b.c", "a.b.d", etc. + if ( + existingChain === chain || + existingChain.startsWith(chain + ".") || + existingChain.startsWith(chain + "[") + ) { + record.modified = true; + } + } + // Mark the chain as modified regardless of it has been created or not!! Otherwise properties that get written will be reported in the first time, but they should not be reported. + // Examples: + // "this.vehicleSys!" should not be extracted as it is written later + // this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint()); // THIS line will get reported if we don't mark the chain as modified + // this.vehicleSys!.automobile!.apple = new ChassisAssembly(new ChassisSchema()); + // this.vehicleSys!.automobile!.apple!.propulsionCover = new EngineEnclosure(new EnclosureSpec()); + if (scopeData.chains.has(chain)) { + scopeData.chains.get(chain)!.modified = true; + } else { + scopeData.chains.set(chain, { + count: 0, + nodes: [], + modified: true, + }); } - - scopeData.variables.add(name); - return name; } - // ====================== - // Core Processing Logic (Improved Hierarchy Tracking) - // ====================== + // Processing member expressions and identifying optimization opportunities + // Examples: + // - obj.prop.val accessed 3+ times → extract to variable + // - obj.prop.val modified → don't extract + // - obj.prop.val used in different scopes → extract separately in each scope function processMemberExpression(node: TSESTree.MemberExpression) { + // Skip nodes that are part of larger member expressions + // Example: In a.b.c, we process the top-level MemberExpression only, + // not the sub-expressions a.b or a if (node.parent?.type === AST_NODE_TYPES.MemberExpression) return; const chainInfo = analyzeChain(node); @@ -171,150 +233,108 @@ const noRepeatedMemberAccess = createRule({ const scope = sourceCode.getScope(node); const scopeData = getScopeData(scope); - // Update hierarchy chain statistics + // keeps record of the longest valid chain, and only report it instead of shorter ones (to avoid repeated reports) + let longestValidChain = ""; + + // Update chain statistics for each part of the hierarchy for (const chain of chainInfo.hierarchy) { + // Skip single-level chains + if (!chain.includes(".")) continue; + + // Skip any chain that contains array access + if (chain.includes("[") && chain.includes("]")) continue; + const record = scopeData.chains.get(chain) || { count: 0, nodes: [], modified: false, }; + if (record.modified) continue; + record.count++; record.nodes.push(node); scopeData.chains.set(chain, record); - } - - // Check for optimal chain immediately - const optimalChain = findOptimalChain(chainInfo.hierarchy, scopeData); - if ( - optimalChain && - scopeData.chains.get(optimalChain)?.count === minOccurrences - ) { - reportOptimization(optimalChain, scopeData, scope); - } - } - function findOptimalChain( - hierarchy: string[], - scopeData: ScopeData - ): string | null { - for (let i = hierarchy.length - 1; i >= 0; i--) { - const chain = hierarchy[i]; - const record = scopeData.chains.get(chain); - if (record && record.count >= minOccurrences) { - return chain; + // record longest chain + if ( + record.count >= minOccurrences && + chain.length > longestValidChain.length + ) { + longestValidChain = chain; } } - return null; - } - - // ====================== - // Optimization Reporting Logic - // ====================== - function reportOptimization( - chain: string, - scopeData: ScopeData, - scope: Scope.Scope - ) { - const record = scopeData.chains.get(chain)!; - const modifications = modificationRegistry.get(chain) || []; - const lastModification = modifications.at(-1); - - context.report({ - node: record.nodes[0], - messageId: "repeatedAccess", - data: { - chain: chain, - count: record.count, - }, - *fix(fixer) { - // Check for subsequent modifications - if ( - lastModification && - lastModification.range[0] < record.nodes[0].range[0] - ) { - return; - } - - // Generate variable name (based on chain hierarchy) - const parts = chain.split("."); - const varName = generateVarName(parts, scope); - - // Insert variable declaration - const insertPosition = findInsertPosition(scope); - yield fixer.insertTextBefore( - insertPosition.node, - `const ${varName} = ${chain};\n` - ); - - // Replace all matching nodes - for (const memberNode of record.nodes) { - const fullChain = analyzeChain(memberNode)?.fullChain; - if (fullChain?.startsWith(chain)) { - const remaining = fullChain.slice(chain.length + 1); - yield fixer.replaceText( - memberNode, - `${varName}${remaining ? "." + remaining : ""}` - ); - } - } - - // Update scope information - scopeData.variables.add(varName); - }, - }); - } - // ====================== - // Helper Functions - // ====================== - function findInsertPosition(scope: Scope.Scope): { - node: TSESTree.Node; - isGlobal: boolean; - } { - if (scope.block.type === AST_NODE_TYPES.Program) { - return { node: scope.block.body[0], isGlobal: true }; + // report the longest chain + if (longestValidChain && !reportedChains.has(longestValidChain)) { + const record = scopeData.chains.get(longestValidChain)!; + context.report({ + node: record.nodes[0], + messageId: "repeatedAccess", + data: { chain: longestValidChain, count: record.count }, + }); + reportedChains.add(longestValidChain); } - return { - node: (scope.block as TSESTree.BlockStatement).body[0], - isGlobal: false, - }; } // ====================== // Rule Listeners // ====================== + // These event handlers process different AST node types and track chain usage + // + // Examples of what each listener detects: + // - MemberExpression: obj.prop.val + // - AssignmentExpression: obj.prop.val = 5 + // - UpdateExpression: obj.prop.val++ + // - CallExpression: obj.prop.method() return { - MemberExpression: (node) => processMemberExpression(node), - + // Track assignments that modify member chains + // Example: obj.prop.val = 5 modifies the "obj.prop.val" chain + // This prevents us from extracting chains that are modified AssignmentExpression: (node) => { if (node.left.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.left); if (chainInfo) { + // Mark all parts of the chain as modified + // Example: For obj.prop.val = 5, we mark "obj", "obj.prop", + // and "obj.prop.val" as modified for (const chain of chainInfo.hierarchy) trackModification(chain, node); } } }, + // Track increment/decrement operations + // Example: obj.prop.counter++ modifies "obj.prop.counter" UpdateExpression: (node) => { if (node.argument.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.argument); if (chainInfo) { + // Mark all parts of the chain as modified + // Example: For obj.prop.val++, we mark "obj", "obj.prop", + // and "obj.prop.val" as modified for (const chain of chainInfo.hierarchy) trackModification(chain, node); } } }, + // Track function calls that might modify their arguments + // Example: obj.methods.update() might modify the "obj.methods" chain CallExpression: (node) => { if (node.callee.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.callee); if (chainInfo) { + // Mark all parts of the chain as potentially modified + // Example: For obj.methods.update(), we mark "obj", "obj.methods", and "obj.methods.update" as potentially modified for (const chain of chainInfo.hierarchy) trackModification(chain, node); } } }, + + // Process member expressions to identify repeated patterns + // Example: Catches obj.prop.val, user.settings.theme, etc. + MemberExpression: (node) => processMemberExpression(node), }; }, }); diff --git a/roadmap.md b/roadmap.md deleted file mode 100644 index 13455f2..0000000 --- a/roadmap.md +++ /dev/null @@ -1,149 +0,0 @@ -# MemberAccess Rule - -前按：本文档聚焦于实现和设计MemberAccess rule，目前已有一版实现，但存在诸多问题，我们将会先给出rule的定义，算法实现，并讨论在生产环境中将会面临的问题以及一些可能的解决思路。 - -实现目的： -首先需要明确我们的规则在尝试解决什么问题，这里将其简要定义如下： -对于a.b.c.d这一类调用，我们需要对于a.b在多次调用情况下将其抽成变量x，并能够实现对于a.b的自动替换，避免在字节码层面多次求值带来的性能影响。 - -算法设计： -基于上述规则，这里简要给出一个实现的算法思路，但实际生产环境中还会有许多其他需要考虑的问题，仅靠此算法不能完全覆盖所有场景。这些问题会在后面进行讨论。 -我们将算法总结为三个部分： -（1）路径统计与最长公共路径检测 -对于a.b.c.d这类结构，我们需要逐层对其解析，形成一个对象链（或称object-chain），得到a-b-c-d -如何实现这个存储结构还有待商榷 -（2）作用域分析 -需要确保我们替换的变量存在于同一个作用域中 -（3）数据流分析 -需要确保替换的变量不存在写入或更改的情况， -检查是否作为赋值目标 -检查是否用于自增/自减操作 -检查是否作为函数参数并被修改 - -```ts -array[i].prop = 1; -i++; // 索引变化 -array[i].prop = 2; // 此处访问了不同元素，不能简单提取 - -// 如果错误地提取为变量: -const temp = array[i]; // 提取初始索引处的元素引用 -temp.prop = 1; -i++; // 索引变化 -temp.prop = 2; // 错误：仍然修改的是原始索引处的元素 -``` - -在生产环境下实际需要解决的问题： -Rule本身是十分容易理解的，但是将其用于生产环境下就有可能会面临许多corner case需要解决，如果不将其全面的考虑进去，必然会导致最后实现的rule在不应该发生替换的地方进行替换，甚至改变程序的原有实现逻辑。 -这类问题可能较多，且较复杂 -我们对一些必须要考虑的case做归纳如下： -（1）需跳过原型方法访问 - -```ts -const arr = [1, 2, 3]; -arr.push(4); // 不应将 arr 提取为变量 -arr.length; // 同上 -``` - -解决方案 - -```ts -function isPrototypeAccess(node: TSESTree.MemberExpression): boolean { - const props = new Set(["__proto__", "constructor", "prototype"]); - return node.property.type === "Identifier" && props.has(node.property.name); -} -``` - -（2）处理解构场景 - -```ts -const { c, d } = a.b; -``` - -解决方案 - -```ts -function isPrototypeAccess(node: TSESTree.MemberExpression): boolean { - const props = new Set(["__proto__", "constructor", "prototype"]); - return node.property.type === "Identifier" && props.has(node.property.name); -} -``` - -（3）对于长链情况的处理 -对于a.b.c.d.e.f.g，如果是a.b.c.d.e被多次使用，那么就直接将其抽取为一个变量y，而不是分别先抽取a.b，再抽取a.b.c,以此类推 -但也有可能会存在a.b.c被多次使用，而被抽取为变量的情况 - -1. 长链提取方式的探讨 - -```ts -// 原始代码 -a.b.c.d.e.f.g(); -a.b.c.d.e.h(); -a.b.c.i(); -a.b.c.j(); - -// 可能的提取方案（贪婪提取） -const temp1 = a.b.c; -const temp2 = temp1.d.e; -temp2.f.g(); -temp2.h(); -temp1.i(); -temp1.j(); - -// 或者更保守的方案（只提取最频繁的） -const temp = a.b.c; -temp.d.e.f.g(); -temp.d.e.h(); -temp.i(); -temp.j(); -``` - -2. 重叠长链优先级问题 - -```ts -// 当有多个重叠的长链时，应该如何选择？ -a.b.c.d.e.f(); -a.b.c.d.e.g(); -a.b.c.h(); -a.b.c.i(); - -// 应该优先提取 a.b.c 还是 a.b.c.d.e? -// 这可能需要基于使用频率和长度的权衡算法 -``` - -3. 链中存在副作用 - -```ts -// 如果链中某部分有副作用，应避免多次提取 -a.getB().c.d(); // getB()可能有副作用 -a.getB().c.e(); - -// 错误提取 -const temp = a.getB().c; // getB()只执行一次！ -temp.d(); -temp.e(); -``` - -4. 链的中间部分被重用但两端不同 - -```ts -a.b.c.d.e.f; -g.h.c.d.e.i; - -// 这种情况 c.d.e 被重用，但前后部分不同 -// 可能需要特殊处理或放弃提取 -``` - -5. 多条链交错使用 - -```ts -a.b.c.d(); -x.y.z(); -a.b.c.e(); -x.y.w(); - -// 由于代码交错，提取变量可能会改变执行顺序 -// 需要确保提取后不会影响程序行为 -``` - -（4）作用域及闭包可能带来的问题 -（5）变量名发生冲突时的情况（对于autofixer） diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index ee6e730..fb2d51e 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -23,6 +23,12 @@ describe("Rule: no-spread", () => { const b = obj.foo.bar; } `, + // ignore array access + ` + const x = data[0].value; + data[0].count++; + send(data[0].id); + `, // Dynamic property access (should be ignored) ` const v1 = ctx[method()].value; @@ -60,9 +66,47 @@ export namespace Constants { export const debounceTimeGnssMotorway_s = SendTime.S120 } `, - - // some more advanced cases + /** + * WARN: should NOT extract [] elements as they can get modified easily + * This is implemented by detecting brackets "[]" in chains + * Examples include: + * arr[0]; + * arr[0][1]; + * arr[0].property; + * obj.arr[0].value; + * data.items[0].config; + */ + ` + const x = data[0][1].value; + data[0][1].count++; + send(data[0][1].id); + `, + ` + const a = dataset[0][1].x + dataset[0][1].y; + dataset[0][1].update(); + const b = dataset[0][1].z * 2; + notify(dataset[0][1].timestamp); + `, + // WARN: DONT extract when function with possible side effect is called upon + ` + const a = data.x + data.y; + data.update(); + const b = data.x * 2; + notify(data.x); + `, ` + const first = data.items[0].config['security'].rules[2].level; + data.items[0].config['security'].rules[2].enabled = true; + validate(data.items[0].config['security'].rules[2].level); + `, + ` + const v1 = obj[123].value; + const v2 = obj[123].value; + const v3 = obj[123].value; + `, + // some more complex cases + ` + /** * Distance-based emission strategy */ @@ -122,6 +166,25 @@ export namespace Constants { } `, + // shouldn't report when modified + ` + const v1 = a.b.c; + a.b = {}; + const v2 = a.b.c; + const v3 = a.b.c; + `, + ` + const v1 = a.b.c; + a.b++; + const v2 = a.b.c; + const v3 = a.b.c; + `, + ` + 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()); + `, ], invalid: [ @@ -132,11 +195,14 @@ export namespace Constants { const v2 = ctx.data.v2; `, errors: [{ messageId: "repeatedAccess" }], - output: ` - const _ctx_data = ctx.data; -const v1 = _ctx_data.v1; - const v2 = _ctx_data.v2; + }, + { + code: ` + const v1 = a.b.c; + const v2 = a.b.c; + const v3 = a.b.c; `, + errors: [{ messageId: "repeatedAccess" }], }, { code: ` @@ -147,15 +213,6 @@ const v1 = _ctx_data.v1; } }`, errors: [{ messageId: "repeatedAccess" }], - output: - "\n" + - " class User {\n" + - " constructor() {\n" + - " const _service_user = service.user;\n" + - "this.profile = _service_user.profile\n" + - " this.log = _service_user.logger\n" + - " }\n" + - " }", }, // Nested scope case { @@ -166,125 +223,15 @@ const v1 = _ctx_data.v1; } `, errors: [{ messageId: "repeatedAccess" }], - output: - "\n" + - " function demo() {\n" + - " const _obj_a_b = obj.a.b;\n" + - "console.log(_obj_a_b.c);\n" + - " return _obj_a_b.d;\n" + - " }\n" + - " ", - }, - - // Array index case - { - code: ` - const x = data[0].value; - data[0].count++; - send(data[0].id); - `, - errors: [ - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - ], - output: - "\n" + - " const _data_0_ = data[0];\n" + - "const x = _data_0_.value;\n" + - " _data_0_.count++;\n" + - " send(_data_0_.id);\n" + - " ", - }, - { - code: ` - const x = data[0][1].value; - data[0][1].count++; - send(data[0][1].id); - `, - errors: [ - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - ], - output: - "\n" + - " const _data_0__1_ = data[0][1];\n" + - "const x = _data_0__1_.value;\n" + - " _data_0__1_.count++;\n" + - " send(_data__0_1_.id);\n" + - " ", - }, - { - code: ` - const a = dataset[0][1].x + dataset[0][1].y; - dataset[0][1].update(); - const b = dataset[0][1].z * 2; - notify(dataset[0][1].timestamp); - `, - errors: [ - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - ], - output: ` - const _dataset_0_1_ = dataset[0][1]; - const a = _dataset_0_1_.x + _dataset_0_1_.y; - _dataset_0_1_.update(); - const b = _dataset_0_1_.z * 2; - notify(_dataset_0_1_.timestamp); - `, }, { code: ` - const first = data.items[0].config['security'].rules[2].level; - data.items[0].config['security'].rules[2].enabled = true; - validate(data.items[0].config['security'].rules[2].level); - `, - errors: [ - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - ], - output: ` - const _data_items_0_config_security_rules_2_ = data.items[0].config['security'].rules[2]; - const first = _data_items_0_config_security_rules_2_.level; - _data_items_0_config_security_rules_2_.enabled = true; - validate(_data_items_0_config_security_rules_2_.level); + const a = data.x + data.y; + // data.update(); + const b = data.x * 2; + notify(data.x); `, - }, - { - 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" }, - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - { messageId: "repeatedAccess" }, - ], - output: [ - "\n" + - " const _this_vehicleSys = this.vehicleSys;\n" + - "this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint());\n" + - " const _this_vehicleSys_automobile = this.vehicleSys.automobile;\n" + - "this.vehicleSys!.automobile!.underframe = new ChassisAssembly(new ChassisSchema());\n" + - " const _this_vehicleSys_automobile_underframe = this.vehicleSys.automobile.underframe;\n" + - "this.vehicleSys!.automobile!.underframe!.propulsionCover = new EngineEnclosure(new EnclosureSpec());\n" + - " this.vehicleSys!.automobile!.underframe!.logisticsBay = new CargoModule(new ModuleTemplate());\n" + - " ", - "\n" + - " const _this_vehicleSys = this.vehicleSys;\n" + - "this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint());\n" + - " const _this_vehicleSys_automobile = _this_vehicleSys.automobile;\n" + - "this.vehicleSys!.automobile!.underframe = new ChassisAssembly(new ChassisSchema());\n" + - " const _this_vehicleSys_automobile_underframe = _this_vehicleSys_automobile.underframe;\n" + - "this.vehicleSys!.automobile!.underframe!.propulsionCover = new EngineEnclosure(new EnclosureSpec());\n" + - " this.vehicleSys!.automobile!.underframe!.logisticsBay = new CargoModule(new ModuleTemplate());\n" + - " ", - ], + errors: [{ messageId: "repeatedAccess" }], }, ], }); From 3fac4ff67ea6c14e2ea5b898eda174fe85142e91 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Thu, 5 Jun 2025 17:26:24 +0800 Subject: [PATCH 04/23] Add more documentation on how this rule works --- docs/rules/no-repeated-member-access.md | 1 - plugins/rules/memberAccess.ts | 40 ++++++++++++++++++------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/docs/rules/no-repeated-member-access.md b/docs/rules/no-repeated-member-access.md index 3138d46..d51e938 100644 --- a/docs/rules/no-repeated-member-access.md +++ b/docs/rules/no-repeated-member-access.md @@ -94,7 +94,6 @@ function singleAccess(obj: MyObject): void { - **Performance**: Reduces redundant property lookups, especially in tight loops - **Readability**: Makes code more readable by giving meaningful names to complex property chains - **Maintainability**: Easier to update property references when extracted to variables -- **Memory Efficiency**: Can reduce memory pressure in performance-critical AssemblyScript code ## When Not To Use diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 1f1c69b..d0e9141 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -2,6 +2,33 @@ import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/utils"; import { Scope } from "@typescript-eslint/utils/ts-eslint"; import createRule from "../utils/createRule.js"; +/** + * Rule to optimize repeated member access patterns by extracting variables + * For more rule details refer to docs/rules/no-repeated-member-access.md + * + * The following material is an overview of implementation details + * It is divided into several phases + * + * 1. Analysis Phase: + * - Traverse AST to identify member access chains (e.g., obj.prop.val) + * - Store chains into hierarchical structures (e.g., ["obj", "obj.prop", "obj.prop.val"]) + * - Cache analysis results to avoid repeatedly processing + * + * 2. Tracking Phase: + * - Count usage frequency of each chain within current scope + * - Identify modified chains (assignments, increments, function calls, etc.) + * - Mark all parts alongside the chain as modified + * + * 3. Reporting Phase: + * - For chains that meet usage threshold and are not modified, suggest variable extraction + * - Report only the longest valid chains + * + * Things to note: + * - Only process chains starting with identifiers or "this" (avoid function call results) + * - Skip computed property access (e.g., obj[key]) + * - Mark modified chains as un-extractable + * - Support TypeScript non-null assertion operator (!) (minor bugs might still persist in some cases) + */ const noRepeatedMemberAccess = createRule({ name: "no-repeated-member-access", meta: { @@ -42,7 +69,6 @@ const noRepeatedMemberAccess = createRule({ modified: boolean; // Whether this chain is modified (written to) } >; - variables: Set; // Variables already declared in this scope }; // Stores data for each scope using WeakMap to avoid memory leaks @@ -63,15 +89,8 @@ const noRepeatedMemberAccess = createRule({ modified: boolean; } >(), - variables: new Set(), }; - // Add existing variable names to the set - for (let i = 0; i < scope.variables.length; i++) { - const variable = scope.variables[i]; - newScopeData.variables.add(variable.name); - } - scopeDataMap.set(scope, newScopeData); } @@ -132,8 +151,7 @@ const noRepeatedMemberAccess = createRule({ parts.push("this"); } else { // Skip chains with non-identifier base objects - // Example: (getObject()).prop is not optimized because - // function call results shouldn't be cached + // Example: (getObject()).prop is not optimized because function call results shouldn't be cached isValid = false; } @@ -200,7 +218,7 @@ const noRepeatedMemberAccess = createRule({ } } // Mark the chain as modified regardless of it has been created or not!! Otherwise properties that get written will be reported in the first time, but they should not be reported. - // Examples: + // Here is a more concrete example: // "this.vehicleSys!" should not be extracted as it is written later // this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint()); // THIS line will get reported if we don't mark the chain as modified // this.vehicleSys!.automobile!.apple = new ChassisAssembly(new ChassisSchema()); From 7667f9d606b67f2cb917298013ec0ce456f278df Mon Sep 17 00:00:00 2001 From: meowbmw Date: Fri, 6 Jun 2025 15:50:16 +0800 Subject: [PATCH 05/23] change default minOcurrences to 3; small code clean up --- plugins/rules/memberAccess.ts | 9 +++------ tests/rules/noRepeatedMemberAccess.test.ts | 3 +++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index d0e9141..afeacc9 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -51,11 +51,11 @@ const noRepeatedMemberAccess = createRule({ "Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.", }, }, - defaultOptions: [{ minOccurrences: 2 }], + defaultOptions: [{ minOccurrences: 3 }], create(context, [options]) { const sourceCode = context.sourceCode; - const minOccurrences = options?.minOccurrences ?? 2; + const minOccurrences = options.minOccurrences; // Track which chains have already been reported to avoid duplicate reports const reportedChains = new Set(); @@ -193,7 +193,7 @@ const noRepeatedMemberAccess = createRule({ return result; } - // Tracks which chains are modified in code to avoid incorrect optimizations + // Tracks which chains are modified in code // // Examples of modifications: // 1. obj.prop = value; // Direct assignment @@ -259,9 +259,6 @@ const noRepeatedMemberAccess = createRule({ // Skip single-level chains if (!chain.includes(".")) continue; - // Skip any chain that contains array access - if (chain.includes("[") && chain.includes("]")) continue; - const record = scopeData.chains.get(chain) || { count: 0, nodes: [], diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index fb2d51e..4264f0c 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -193,6 +193,7 @@ export namespace Constants { code: ` const v1 = ctx.data.v1; const v2 = ctx.data.v2; + const v3 = ctx.data.v3; `, errors: [{ messageId: "repeatedAccess" }], }, @@ -210,6 +211,7 @@ export namespace Constants { constructor() { this.profile = service.user.profile this.log = service.user.logger + this.cat = service.user.cat } }`, errors: [{ messageId: "repeatedAccess" }], @@ -219,6 +221,7 @@ export namespace Constants { code: ` function demo() { console.log(obj.a.b.c); + let x = obj.a.b; return obj.a.b.d; } `, From 21732a11e0bd9ab69b8c2a5815bea7597e0b8014 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Fri, 6 Jun 2025 17:40:22 +0800 Subject: [PATCH 06/23] clean up test case --- tests/rules/noRepeatedMemberAccess.test.ts | 86 ---------------------- 1 file changed, 86 deletions(-) diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index 4264f0c..041b808 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -58,13 +58,6 @@ describe("Rule: no-spread", () => { export const apple = Juice.D31 export const banana = Juice.D32 export const cat = Juice.D33 - `, - ` -export namespace Constants { - export const defaultDebounceTimeRegular_s: u32 = SendTime.NEXT_REGULAR - export const debounceTimeGnssInnerCity_s = SendTime.S30 - export const debounceTimeGnssMotorway_s = SendTime.S120 -} `, /** * WARN: should NOT extract [] elements as they can get modified easily @@ -81,12 +74,6 @@ export namespace Constants { data[0][1].count++; send(data[0][1].id); `, - ` - const a = dataset[0][1].x + dataset[0][1].y; - dataset[0][1].update(); - const b = dataset[0][1].z * 2; - notify(dataset[0][1].timestamp); - `, // WARN: DONT extract when function with possible side effect is called upon ` const a = data.x + data.y; @@ -95,77 +82,10 @@ export namespace Constants { notify(data.x); `, ` - const first = data.items[0].config['security'].rules[2].level; - data.items[0].config['security'].rules[2].enabled = true; - validate(data.items[0].config['security'].rules[2].level); - `, - ` const v1 = obj[123].value; const v2 = obj[123].value; const v3 = obj[123].value; `, - // some more complex cases - ` - - /** - * Distance-based emission strategy - */ - export class DistanceBasedDeliveryStrategy implements EmissionStrategy { - private checkStateChange(attr: NumericProperty): bool { - return ( - attr.getCurrent().isUnstable() && - attr.getPrevious() != null && - attr.getPrevious()!.isStable() - ); - } - - public isEqual(other: GenericValueContainer): bool { - return ( - this.isStable() == other.isStable() && - this.isActive() == other.isActive() && - (!this.isActive() || this.getValue() == other.getValue()) - ); - } - - public copy(): GenericValueContainer { - const clone = new LongValueWrapper(this.initializer); - clone.status = this.status; - clone.error = this.error; - return clone; - } - }`, - ` - - let activeConfig: BaseConfiguration; - const configData = AppContext.loadSettings(); - - if (configData) { - activeConfig = configData; - } else { - activeConfig = new BaseConfiguration(); - activeConfig.initializationDelay = Constants.DEFAULT_INIT_DELAY; - activeConfig.fastPollingInterval = Constants.DEFAULT_FAST_INTERVAL; - activeConfig.normalPollingInterval = Constants.DEFAULT_NORMAL_INTERVAL; - } - - if (runtimeEnv && eventController) { - SystemHookManager.instance = this; - this.eventController = eventController; - runtimeEnv.registerStateChangeListener(SystemHookManager.handleStateChange); - this.lastKnownState = runtimeEnv.getCurrentEnvironmentState(); - } - `, - ` - if ( - currentSession.authType == AuthType.PRIVILEGED && - currentSession.status == SessionStatus.ACTIVE && - this.runtimeEnv.getCurrentEnvironmentState().isPresent() - ) { - const timestamp = getTimestamp(); - serviceInstance.lastSyncTime = timestamp; - } - - `, // shouldn't report when modified ` const v1 = a.b.c; @@ -179,12 +99,6 @@ export namespace Constants { const v2 = a.b.c; const v3 = a.b.c; `, - ` - 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()); - `, ], invalid: [ From 6221adda062a7cca34af30f6ad1efe831b3dcfb3 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Fri, 13 Jun 2025 15:31:30 +0800 Subject: [PATCH 07/23] enable debug support --- .gitignore | 1 - .vscode/launch.json | 28 +++++++ .vscode/settings.json | 3 + loader.mjs | 13 ++++ package-lock.json | 159 +++++++++++++++++++++++++++++++++++++- package.json | 1 + tests/rules/playground.ts | 19 +++++ todo.txt | 11 +++ 8 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 .vscode/settings.json create mode 100644 loader.mjs create mode 100644 tests/rules/playground.ts create mode 100644 todo.txt diff --git a/.gitignore b/.gitignore index a7e6d18..c0c5b24 100644 --- a/.gitignore +++ b/.gitignore @@ -3,5 +3,4 @@ dist/ sample_cases/ wasm-toolchain coverage -.vscode reference \ No newline at end of file diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000..63ae31b --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,28 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "args": ["--no-timeouts", "--colors", "--inspect-brk", "tests/rules/playground.ts"], + "runtimeArgs": [ + "--experimental-specifier-resolution=node", + "--experimental-loader", + "ts-node/esm", + "--experimental-loader", + "./loader.mjs", + "--no-warnings" + ], + "internalConsoleOptions": "openOnSessionStart", + "name": "Mocha Tests", + "program": "${workspaceRoot}/node_modules/.bin/_mocha", + "request": "launch", + "skipFiles": [ + "${workspaceFolder}/node_modules/**/*.js", + "/**" + ], + "type": "node" + } + ] +} diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..19696db --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "mochaExplorer.files": "dist/tests/**/*.test.js", +} \ No newline at end of file diff --git a/loader.mjs b/loader.mjs new file mode 100644 index 0000000..e7c58fe --- /dev/null +++ b/loader.mjs @@ -0,0 +1,13 @@ +let is_main = true; + +export const load = (url, context, loadNext) => { + if (is_main) { + is_main = false; + + if (!context.format) { + context.format = "commonjs"; + } + } + + return loadNext(url, context, loadNext); +}; \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 696d08d..2e41607 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,6 +24,7 @@ "mocha": "^11.2.2", "npm-run-all": "^4.1.5", "prettier": "^3.5.3", + "ts-node": "^10.9.2", "tsx": "^4.19.3" }, "engines": { @@ -658,6 +659,19 @@ "node": ">=20" } }, + "node_modules/@cspotcode/source-map-support": { + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/@cspotcode/source-map-support/-/source-map-support-0.8.1.tgz", + "integrity": "sha512-IchNf6dN4tHoMFIn/7OE8LWZ19Y6q/67Bmf6vnGREv8RSbBVb9LPJxEcnwrcwX6ixSvaiGoomAUvu4YSxXrVgw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@jridgewell/trace-mapping": "0.3.9" + }, + "engines": { + "node": ">=12" + } + }, "node_modules/@emnapi/core": { "version": "1.4.3", "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.4.3.tgz", @@ -1412,6 +1426,17 @@ "dev": true, "license": "MIT" }, + "node_modules/@jridgewell/trace-mapping": { + "version": "0.3.9", + "resolved": "https://registry.npmjs.org/@jridgewell/trace-mapping/-/trace-mapping-0.3.9.tgz", + "integrity": "sha512-3Belt6tdc8bPgAtbcmdtNJlirVoTmEb5e2gC94PnkwEW9jI6CAHUeoG85tjWP5WquqfavoMtMwiG4P926ZKKuQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@jridgewell/resolve-uri": "^3.0.3", + "@jridgewell/sourcemap-codec": "^1.4.10" + } + }, "node_modules/@modelcontextprotocol/sdk": { "version": "1.11.2", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.11.2.tgz", @@ -1538,6 +1563,34 @@ "typescript-eslint": "^8.29.0" } }, + "node_modules/@tsconfig/node10": { + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/@tsconfig/node10/-/node10-1.0.11.tgz", + "integrity": "sha512-DcRjDCujK/kCk/cUe8Xz8ZSpm8mS3mNNpta+jGCA6USEDfktlNvm1+IuZ9eTcDbNk41BHwpHHeW+N1lKCz4zOw==", + "dev": true, + "license": "MIT" + }, + "node_modules/@tsconfig/node12": { + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/@tsconfig/node12/-/node12-1.0.11.tgz", + "integrity": "sha512-cqefuRsh12pWyGsIoBKJA9luFu3mRxCA+ORZvA4ktLSzIuCUtWVxGIuXigEwO5/ywWFMZ2QEGKWvkZG1zDMTag==", + "dev": true, + "license": "MIT" + }, + "node_modules/@tsconfig/node14": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/@tsconfig/node14/-/node14-1.0.3.tgz", + "integrity": "sha512-ysT8mhdixWK6Hw3i1V2AeRqZ5WfXg1G43mqoYlM2nc6388Fq5jcXyr5mRsqViLx/GJYdoL0bfXD8nmF+Zn/Iow==", + "dev": true, + "license": "MIT" + }, + "node_modules/@tsconfig/node16": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/@tsconfig/node16/-/node16-1.0.4.tgz", + "integrity": "sha512-vxhUy4J8lyeyinH7Azl1pdd43GJhZH/tP2weN8TntQblOY+A0XbT8DJk1/oCPuOOyg/Ja757rG0CgHcWC8OfMA==", + "dev": true, + "license": "MIT" + }, "node_modules/@tybys/wasm-util": { "version": "0.9.0", "resolved": "https://registry.npmjs.org/@tybys/wasm-util/-/wasm-util-0.9.0.tgz", @@ -2100,7 +2153,6 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.14.1.tgz", "integrity": "sha512-OvQ/2pUDKmgfCg++xsTX1wGxfTaszcHVcTctW4UJB4hibJx2HXxxO5UmVgyjMa+ZDsiaf5wWLXYpRWMmBI0QHg==", "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2118,6 +2170,19 @@ "acorn": "^6.0.0 || ^7.0.0 || ^8.0.0" } }, + "node_modules/acorn-walk": { + "version": "8.3.4", + "resolved": "https://registry.npmjs.org/acorn-walk/-/acorn-walk-8.3.4.tgz", + "integrity": "sha512-ueEepnujpqee2o5aIYnvHU6C0A42MNdsIDeqy5BydrkuC5R1ZuUFnm27EeFJGoEHJQgn3uleRvmTXaJgfXbt4g==", + "dev": true, + "license": "MIT", + "dependencies": { + "acorn": "^8.11.0" + }, + "engines": { + "node": ">=0.4.0" + } + }, "node_modules/ajv": { "version": "6.12.6", "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", @@ -2160,6 +2225,13 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, + "node_modules/arg": { + "version": "4.1.3", + "resolved": "https://registry.npmjs.org/arg/-/arg-4.1.3.tgz", + "integrity": "sha512-58S9QDqG0Xx27YwPSt9fJxivjYl432YCwfDMfZ+71RAqUrZef7LrKQZ3LHLOwCS4FLNBplP533Zx895SeOCHvA==", + "dev": true, + "license": "MIT" + }, "node_modules/argparse": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", @@ -2897,6 +2969,13 @@ "node": ">= 0.10" } }, + "node_modules/create-require": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/create-require/-/create-require-1.1.1.tgz", + "integrity": "sha512-dcKFX3jn0MpIaXjisoRvexIJVEKzaq7z2rZKxf+MSr9TkdmHmsU4m2lcLojrj/FHl8mk5VxMmYA+ftRkP/3oKQ==", + "dev": true, + "license": "MIT" + }, "node_modules/cross-spawn": { "version": "7.0.6", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", @@ -5942,6 +6021,13 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/make-error": { + "version": "1.3.6", + "resolved": "https://registry.npmjs.org/make-error/-/make-error-1.3.6.tgz", + "integrity": "sha512-s8UhlNe7vPKomQhC1qFelMokr/Sc3AgNbso3n74mVPA5LTZwkB9NlXf4XPamLxJE8h0gh73rM94xvwRT2CVInw==", + "dev": true, + "license": "ISC" + }, "node_modules/math-intrinsics": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/math-intrinsics/-/math-intrinsics-1.1.0.tgz", @@ -7985,6 +8071,60 @@ "typescript": ">=4.8.4" } }, + "node_modules/ts-node": { + "version": "10.9.2", + "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-10.9.2.tgz", + "integrity": "sha512-f0FFpIdcHgn8zcPSbf1dRevwt047YMnaiJM3u2w2RewrB+fob/zePZcrOyQoLMMO7aBIddLcQIEK5dYjkLnGrQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@cspotcode/source-map-support": "^0.8.0", + "@tsconfig/node10": "^1.0.7", + "@tsconfig/node12": "^1.0.7", + "@tsconfig/node14": "^1.0.0", + "@tsconfig/node16": "^1.0.2", + "acorn": "^8.4.1", + "acorn-walk": "^8.1.1", + "arg": "^4.1.0", + "create-require": "^1.1.0", + "diff": "^4.0.1", + "make-error": "^1.1.1", + "v8-compile-cache-lib": "^3.0.1", + "yn": "3.1.1" + }, + "bin": { + "ts-node": "dist/bin.js", + "ts-node-cwd": "dist/bin-cwd.js", + "ts-node-esm": "dist/bin-esm.js", + "ts-node-script": "dist/bin-script.js", + "ts-node-transpile-only": "dist/bin-transpile.js", + "ts-script": "dist/bin-script-deprecated.js" + }, + "peerDependencies": { + "@swc/core": ">=1.2.50", + "@swc/wasm": ">=1.2.50", + "@types/node": "*", + "typescript": ">=2.7" + }, + "peerDependenciesMeta": { + "@swc/core": { + "optional": true + }, + "@swc/wasm": { + "optional": true + } + } + }, + "node_modules/ts-node/node_modules/diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/tsconfig-paths": { "version": "3.15.0", "resolved": "https://registry.npmjs.org/tsconfig-paths/-/tsconfig-paths-3.15.0.tgz", @@ -8307,6 +8447,13 @@ "punycode": "^2.1.0" } }, + "node_modules/v8-compile-cache-lib": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/v8-compile-cache-lib/-/v8-compile-cache-lib-3.0.1.tgz", + "integrity": "sha512-wa7YjyUGfNZngI/vtK0UHAN+lgDCxBPCylVXGp0zu59Fz5aiGtNXaq3DhIov063MorB+VfufLh3JlF2KdTK3xg==", + "dev": true, + "license": "MIT" + }, "node_modules/v8-to-istanbul": { "version": "9.3.0", "resolved": "https://registry.npmjs.org/v8-to-istanbul/-/v8-to-istanbul-9.3.0.tgz", @@ -8720,6 +8867,16 @@ "node": ">=8" } }, + "node_modules/yn": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/yn/-/yn-3.1.1.tgz", + "integrity": "sha512-Ux4ygGWsu2c7isFWe8Yu1YluJmqVhxqK2cLXNQA5AcC3QfbGNpM7fu0Y8b/z16pXLnFxZYvWhd3fhBY9DLmC6Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=6" + } + }, "node_modules/yocto-queue": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", diff --git a/package.json b/package.json index dc762ed..e00413b 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "mocha": "^11.2.2", "npm-run-all": "^4.1.5", "prettier": "^3.5.3", + "ts-node": "^10.9.2", "tsx": "^4.19.3" }, "dependencies": { diff --git a/tests/rules/playground.ts b/tests/rules/playground.ts new file mode 100644 index 0000000..3aa2b28 --- /dev/null +++ b/tests/rules/playground.ts @@ -0,0 +1,19 @@ +import { describe, it } from "mocha"; +import { createRuleTester } from "../utils/testUtils.js"; +import noRepeatedMemberAccess from "../../plugins/rules/memberAccess.js"; + +describe("Rule: no-spread", () => { + const ruleTester = createRuleTester(); + + it("validates all test cases for no-repeated-member-access rule", () => { + ruleTester.run("no-repeated-member-access", noRepeatedMemberAccess, { + valid: [ + ` + const x=a.b.c; + `, + ], + + invalid: [], + }); + }); +}); diff --git a/todo.txt b/todo.txt new file mode 100644 index 0000000..cf5ed5b --- /dev/null +++ b/todo.txt @@ -0,0 +1,11 @@ +a.b.c.d.e 从e开始往上遍历，看看触发了多少次 +明确AST和node的遍历过程 + +考虑用树来存储 + +从输入到输出，对数据做加工，考虑用什么数据结构合适 + +明确库函数的作用 + +例子： +get().a.b From 4f44e12fea6b416057dd5eba53b44394005d21be Mon Sep 17 00:00:00 2001 From: meowbmw Date: Fri, 13 Jun 2025 17:34:35 +0800 Subject: [PATCH 08/23] add barebone rewrite --- plugins/rules/rewriteMember.ts | 72 ++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 plugins/rules/rewriteMember.ts diff --git a/plugins/rules/rewriteMember.ts b/plugins/rules/rewriteMember.ts new file mode 100644 index 0000000..6064b29 --- /dev/null +++ b/plugins/rules/rewriteMember.ts @@ -0,0 +1,72 @@ +import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/utils"; +import { Scope } from "@typescript-eslint/utils/ts-eslint"; +import createRule from "../utils/createRule.js"; + +const noRepeatedMemberAccess = createRule({ + name: "no-repeated-member-access", + meta: { + type: "suggestion", + docs: { + description: + "Optimize repeated member access patterns by extracting variables", + }, + fixable: "code", + schema: [ + { + type: "object", + properties: { + minOccurrences: { type: "number", minimum: 2, default: 3 }, + }, + }, + ], + messages: { + repeatedAccess: + "Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.", + }, + }, + defaultOptions: [{ minOccurrences: 3 }], + + create(context, [options]) { + const sourceCode = context.sourceCode; + const minOccurrences = options.minOccurrences; + + type scopeKey = [Scope.Scope, TSESTree.MemberExpression[]]; + type scopeValue = { count: number; modified: boolean }; + const scopeMap = new WeakMap(); + // ====================== + // Rule Listeners + // ====================== + // These event handlers process different AST node types and track chain usage + // + // Examples of what each listener detects: + // - MemberExpression: obj.prop.val + // - AssignmentExpression: obj.prop.val = 5 + // - UpdateExpression: obj.prop.val++ + // - CallExpression: obj.prop.method() + return { + // Track assignment expression + // Example: obj.prop.val = 5 + AssignmentExpression: (node) => { + // track left expression and mark all of them to be modified + }, + + // Track increment/decrement operations + // Example: obj.prop.counter++ modifies "obj.prop.counter" + UpdateExpression: (node) => { + // track expression and mark all of them to be modified + }, + + // Track function calls that might modify their arguments + // Example: obj.methods.update() might modify the "obj.methods" chain + CallExpression: (node) => { + // track expression and mark all of them to be modified + }, + + // Process member expressions to identify repeated patterns + // Example: Catches obj.prop.val, user.settings.theme, etc. + MemberExpression: (node) => processMemberExpression(node), + }; + }, +}); + +export default noRepeatedMemberAccess; From defaea0db83293956c604109c0703c51cc3363f5 Mon Sep 17 00:00:00 2001 From: xpirad Date: Sun, 15 Jun 2025 21:07:07 +0800 Subject: [PATCH 09/23] still wip; --- plugins/rules/rewriteMember.ts | 28 ++++++++++++++++++++++------ tests/rules/playground.ts | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/plugins/rules/rewriteMember.ts b/plugins/rules/rewriteMember.ts index 6064b29..ba6a161 100644 --- a/plugins/rules/rewriteMember.ts +++ b/plugins/rules/rewriteMember.ts @@ -27,39 +27,55 @@ const noRepeatedMemberAccess = createRule({ defaultOptions: [{ minOccurrences: 3 }], create(context, [options]) { + // consider: + // 1. a map to store [[scope, memberExpression[]], { count: number, modified: boolean }] + // 2. process every statement we have + // if it is a memberExpression, go through all its nodes and increase their count in the map + // in case of assignment, update the modified flag for all nodes in the chain + // 3. at the end of the scope, check if any of the chains have count >= minOccurrences + // if so, report the issue and provide a fix to extract the chain into a variable const sourceCode = context.sourceCode; const minOccurrences = options.minOccurrences; - type scopeKey = [Scope.Scope, TSESTree.MemberExpression[]]; + // type scopeKey = [Scope.Scope, TSESTree.MemberExpression[]]; type scopeValue = { count: number; modified: boolean }; - const scopeMap = new WeakMap(); + const scopeMap = new WeakMap(); + + function trackModification(node: TSESTree.MemberExpression) { + scopeMap[node].modified = true; + } + function processMemberExpression(node: TSESTree.MemberExpression) {} + // ====================== // Rule Listeners // ====================== // These event handlers process different AST node types and track chain usage // // Examples of what each listener detects: - // - MemberExpression: obj.prop.val // - AssignmentExpression: obj.prop.val = 5 // - UpdateExpression: obj.prop.val++ // - CallExpression: obj.prop.method() + // - MemberExpression: obj.prop.val return { // Track assignment expression // Example: obj.prop.val = 5 AssignmentExpression: (node) => { - // track left expression and mark all of them to be modified + if (node.left.type === AST_NODE_TYPES.MemberExpression) { + } }, // Track increment/decrement operations // Example: obj.prop.counter++ modifies "obj.prop.counter" UpdateExpression: (node) => { - // track expression and mark all of them to be modified + if (node.argument.type === AST_NODE_TYPES.MemberExpression) { + } }, // Track function calls that might modify their arguments // Example: obj.methods.update() might modify the "obj.methods" chain CallExpression: (node) => { - // track expression and mark all of them to be modified + if (node.callee.type === AST_NODE_TYPES.MemberExpression) { + } }, // Process member expressions to identify repeated patterns diff --git a/tests/rules/playground.ts b/tests/rules/playground.ts index 3aa2b28..b166a2a 100644 --- a/tests/rules/playground.ts +++ b/tests/rules/playground.ts @@ -9,7 +9,7 @@ describe("Rule: no-spread", () => { ruleTester.run("no-repeated-member-access", noRepeatedMemberAccess, { valid: [ ` - const x=a.b.c; + x.y=a.b.c; `, ], From 500b0ca90cf1aa958adaf9fa26aedc72cc5876e9 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 16 Jun 2025 20:41:13 +0800 Subject: [PATCH 10/23] wip 2: refactor old version --- package.json | 2 +- plugins/rules/memberAccess.ts | 111 +++++---------------------------- plugins/rules/rewriteMember.ts | 48 +++++++------- 3 files changed, 42 insertions(+), 119 deletions(-) diff --git a/package.json b/package.json index e00413b..283336f 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ ], "scripts": { "build": "npx tsc --build ./tsconfig.json", - "test": "mocha --timeout 10000 'dist/tests/**/**.test.js'", + "test": "mocha --timeout 10000 'dist/tests/**/noRepeatedMemberAccess.test.js'", "test:coverage": "c8 npm test", "lint": "npx eslint plugins/ tests/", "watch": "npx tsc --build ./tsconfig.json --watch", diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index afeacc9..0fa20cf 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -97,38 +97,15 @@ const noRepeatedMemberAccess = createRule({ return scopeDataMap.get(scope)!; } - // This part analyzes and extracts member access chains from AST nodes - // - // Examples of chains: - // - a.b.c → hierarchy: ["a", "a.b", "a.b.c"], fullChain: "a.b.c" - // - foo["bar"].baz → hierarchy: ["foo", "foo[bar]", "foo[bar].baz"], fullChain: "foo[bar].baz" - // - user.profile.settings.theme → hierarchy: ["user", "user.profile", "user.profile.settings"], fullChain: "user.profile.settings" - interface ChainInfo { - hierarchy: string[]; // Chain hierarchy (e.g., ["a", "a.b", "a.b.c"]) - fullChain: string; // Complete path (e.g., "a.b.c") - } - // Cache analyzed expressions to improve performance - const chainCache = new WeakMap< - TSESTree.MemberExpression, - ChainInfo | null - >(); - - function analyzeChain(node: TSESTree.MemberExpression): ChainInfo | null { - // Check cache first for performance - // Example: If we've already analyzed a.b.c in a previous call, - // we return the cached result immediately - if (chainCache.has(node)) return chainCache.get(node)!; - - const parts: string[] = []; // Stores parts of the chain in reverse order + function analyzeChain(node: TSESTree.MemberExpression) { + const parts: string[] = []; // AST is iterated in reverse order let current: TSESTree.Node = node; // Current node in traversal - let isValid = true; // Whether this chain is valid for optimization // Collect property chain (reverse order) // Example: For a.b.c, we'd collect ["c", "b", "a"] initially while (current.type === AST_NODE_TYPES.MemberExpression) { if (current.computed) { // skip computed properties like obj["prop"] or arr[0] or obj[getKey()] - isValid = false; break; } else { // Handle dot notation like obj.prop @@ -149,57 +126,23 @@ const noRepeatedMemberAccess = createRule({ parts.push(current.name); // Add base object name } else if (current.type === AST_NODE_TYPES.ThisExpression) { parts.push("this"); - } else { - // Skip chains with non-identifier base objects - // Example: (getObject()).prop is not optimized because function call results shouldn't be cached - isValid = false; - } - - // Validate chain - if (!isValid || parts.length < 2) { - chainCache.set(node, null); - return null; - } + } // ignore other patterns // Generate hierarchy chain (forward order) // Example: For parts ["c", "b", "a"], we reverse to ["a", "b", "c"] // and build hierarchy ["a", "a.b", "a.b.c"] parts.reverse(); - const hierarchy: string[] = []; + const result: string[] = []; + let currentChain = ""; for (let i = 0; i < parts.length; i++) { - let chain; - // Create chain for each level - // eslint-disable-next-line unicorn/prefer-ternary - if (i === 0) { - // First element is used directly - // Example: For a.b.c, first element is "a" - chain = parts[0]; - } else { - // Build based on previous element - // Example: "a" + "." + "b" = "a.b" - chain = hierarchy[i - 1] + "." + parts[i]; - } - hierarchy.push(chain); + currentChain = i === 0 ? parts[0] : `${currentChain}.${parts[i]}`; + result.push(currentChain); } - const result = { - hierarchy: hierarchy, - fullChain: hierarchy.at(-1) ?? "", // Use last element or empty string - }; - - // Cache and return the result - chainCache.set(node, result); return result; } - // Tracks which chains are modified in code - // - // Examples of modifications: - // 1. obj.prop = value; // Direct assignment - // 2. obj.prop++; // Increment/decrement - // 3. updateValues(obj.prop); // Potential modification through function call - function trackModification(chain: string, node: TSESTree.Node) { const scope = sourceCode.getScope(node); const scopeData = getScopeData(scope); @@ -217,12 +160,6 @@ const noRepeatedMemberAccess = createRule({ record.modified = true; } } - // Mark the chain as modified regardless of it has been created or not!! Otherwise properties that get written will be reported in the first time, but they should not be reported. - // Here is a more concrete example: - // "this.vehicleSys!" should not be extracted as it is written later - // this.vehicleSys!.automobile = new TransportCore(new TransportBlueprint()); // THIS line will get reported if we don't mark the chain as modified - // this.vehicleSys!.automobile!.apple = new ChassisAssembly(new ChassisSchema()); - // this.vehicleSys!.automobile!.apple!.propulsionCover = new EngineEnclosure(new EnclosureSpec()); if (scopeData.chains.has(chain)) { scopeData.chains.get(chain)!.modified = true; } else { @@ -234,11 +171,6 @@ const noRepeatedMemberAccess = createRule({ } } - // Processing member expressions and identifying optimization opportunities - // Examples: - // - obj.prop.val accessed 3+ times → extract to variable - // - obj.prop.val modified → don't extract - // - obj.prop.val used in different scopes → extract separately in each scope function processMemberExpression(node: TSESTree.MemberExpression) { // Skip nodes that are part of larger member expressions // Example: In a.b.c, we process the top-level MemberExpression only, @@ -255,7 +187,7 @@ const noRepeatedMemberAccess = createRule({ let longestValidChain = ""; // Update chain statistics for each part of the hierarchy - for (const chain of chainInfo.hierarchy) { + for (const chain of chainInfo) { // Skip single-level chains if (!chain.includes(".")) continue; @@ -291,16 +223,6 @@ const noRepeatedMemberAccess = createRule({ } } - // ====================== - // Rule Listeners - // ====================== - // These event handlers process different AST node types and track chain usage - // - // Examples of what each listener detects: - // - MemberExpression: obj.prop.val - // - AssignmentExpression: obj.prop.val = 5 - // - UpdateExpression: obj.prop.val++ - // - CallExpression: obj.prop.method() return { // Track assignments that modify member chains // Example: obj.prop.val = 5 modifies the "obj.prop.val" chain @@ -309,11 +231,9 @@ const noRepeatedMemberAccess = createRule({ if (node.left.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.left); if (chainInfo) { - // Mark all parts of the chain as modified - // Example: For obj.prop.val = 5, we mark "obj", "obj.prop", - // and "obj.prop.val" as modified - for (const chain of chainInfo.hierarchy) + for (const chain of chainInfo) { trackModification(chain, node); + } } } }, @@ -324,11 +244,9 @@ const noRepeatedMemberAccess = createRule({ if (node.argument.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.argument); if (chainInfo) { - // Mark all parts of the chain as modified - // Example: For obj.prop.val++, we mark "obj", "obj.prop", - // and "obj.prop.val" as modified - for (const chain of chainInfo.hierarchy) + for (const chain of chainInfo) { trackModification(chain, node); + } } } }, @@ -339,10 +257,9 @@ const noRepeatedMemberAccess = createRule({ if (node.callee.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.callee); if (chainInfo) { - // Mark all parts of the chain as potentially modified - // Example: For obj.methods.update(), we mark "obj", "obj.methods", and "obj.methods.update" as potentially modified - for (const chain of chainInfo.hierarchy) + for (const chain of chainInfo) { trackModification(chain, node); + } } } }, diff --git a/plugins/rules/rewriteMember.ts b/plugins/rules/rewriteMember.ts index ba6a161..451ff11 100644 --- a/plugins/rules/rewriteMember.ts +++ b/plugins/rules/rewriteMember.ts @@ -27,35 +27,41 @@ const noRepeatedMemberAccess = createRule({ defaultOptions: [{ minOccurrences: 3 }], create(context, [options]) { - // consider: - // 1. a map to store [[scope, memberExpression[]], { count: number, modified: boolean }] - // 2. process every statement we have - // if it is a memberExpression, go through all its nodes and increase their count in the map - // in case of assignment, update the modified flag for all nodes in the chain - // 3. at the end of the scope, check if any of the chains have count >= minOccurrences - // if so, report the issue and provide a fix to extract the chain into a variable const sourceCode = context.sourceCode; const minOccurrences = options.minOccurrences; - // type scopeKey = [Scope.Scope, TSESTree.MemberExpression[]]; - type scopeValue = { count: number; modified: boolean }; - const scopeMap = new WeakMap(); + type modifiedMap = Map; + type countMap = Map; + type nodeMap = Map; + + const scopeToModifiedMap = new Map(); + const scopeToCountMap = new Map(); + const scopeToNodeMap = new Map(); + + function analyzeChain(node: TSESTree.MemberExpression) { + let currentNode = node; + let parts = []; + let valid = true; + while (currentNode.type === AST_NODE_TYPES.MemberExpression) { + parts.push + + } + + } function trackModification(node: TSESTree.MemberExpression) { - scopeMap[node].modified = true; + const currentScope = sourceCode.getScope(node); + if (!scopeToModifiedMap.has(currentScope)) { + const newModifiedMap = new Map(); + scopeToModifiedMap.set(currentScope, newModifiedMap); + } + const currentModifiedMap = scopeToModifiedMap.get(currentScope)!; + + // scopeMap[node].modified = true; } + function processMemberExpression(node: TSESTree.MemberExpression) {} - // ====================== - // Rule Listeners - // ====================== - // These event handlers process different AST node types and track chain usage - // - // Examples of what each listener detects: - // - AssignmentExpression: obj.prop.val = 5 - // - UpdateExpression: obj.prop.val++ - // - CallExpression: obj.prop.method() - // - MemberExpression: obj.prop.val return { // Track assignment expression // Example: obj.prop.val = 5 From edecf9f27f5fd61c56131445d7b7c90b755be55d Mon Sep 17 00:00:00 2001 From: xpirad Date: Tue, 17 Jun 2025 00:03:48 +0800 Subject: [PATCH 11/23] wip 3; near completion?? --- .vscode/launch.json | 7 +- .vscode/settings.json | 4 +- plugins/rules/memberAccess.ts | 128 ++++++++++++--------------------- plugins/rules/rewriteMember.ts | 94 ------------------------ 4 files changed, 54 insertions(+), 179 deletions(-) delete mode 100644 plugins/rules/rewriteMember.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index 63ae31b..2b62403 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -5,7 +5,12 @@ "version": "0.2.0", "configurations": [ { - "args": ["--no-timeouts", "--colors", "--inspect-brk", "tests/rules/playground.ts"], + "args": [ + "--no-timeouts", + "--colors", + "--inspect-brk", + "tests/rules/playground.ts" + ], "runtimeArgs": [ "--experimental-specifier-resolution=node", "--experimental-loader", diff --git a/.vscode/settings.json b/.vscode/settings.json index 19696db..229f4c9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,3 +1,3 @@ { - "mochaExplorer.files": "dist/tests/**/*.test.js", -} \ No newline at end of file + "mochaExplorer.files": "dist/tests/**/*.test.js" +} diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 0fa20cf..17cb45d 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -60,45 +60,35 @@ const noRepeatedMemberAccess = createRule({ // Track which chains have already been reported to avoid duplicate reports const reportedChains = new Set(); - type ScopeData = { - chains: Map< - string, - { - count: number; // Number of times this chain is accessed - nodes: TSESTree.MemberExpression[]; // AST nodes where this chain appears - modified: boolean; // Whether this chain is modified (written to) - } - >; - }; - + type ScopeData = Map< + string, + { + count: number; // Number of times this chain is accessed + node: TSESTree.MemberExpression; // AST nodes where this chain appears + modified: boolean; // Whether this chain is modified (written to) + } + >; // Stores data for each scope using WeakMap to avoid memory leaks const scopeDataMap = new WeakMap(); function getScopeData(scope: Scope.Scope): ScopeData { - // Creates new scope data if none exists - // Example: First time seeing the function foo() { obj.prop.val; } - // we create a new ScopeData for this function if (!scopeDataMap.has(scope)) { - // Create new scope data - const newScopeData = { - chains: new Map< - string, - { - count: number; - nodes: TSESTree.MemberExpression[]; - modified: boolean; - } - >(), - }; - + // Create new scope data if not already present + const newScopeData = new Map< + string, + { + count: number; + node: TSESTree.MemberExpression; + modified: boolean; + } + >(); scopeDataMap.set(scope, newScopeData); } - return scopeDataMap.get(scope)!; } function analyzeChain(node: TSESTree.MemberExpression) { - const parts: string[] = []; // AST is iterated in reverse order + const properties: string[] = []; // AST is iterated in reverse order let current: TSESTree.Node = node; // Current node in traversal // Collect property chain (reverse order) @@ -109,7 +99,7 @@ const noRepeatedMemberAccess = createRule({ break; } else { // Handle dot notation like obj.prop - parts.push(current.property.name); + properties.push(current.property.name); } current = current.object; // Move to parent object @@ -123,33 +113,34 @@ const noRepeatedMemberAccess = createRule({ // Handle base object (the root of the chain) // Example: For a.b.c, the base object is "a" if (current.type === AST_NODE_TYPES.Identifier) { - parts.push(current.name); // Add base object name + properties.push(current.name); // Add base object name } else if (current.type === AST_NODE_TYPES.ThisExpression) { - parts.push("this"); + properties.push("this"); } // ignore other patterns // Generate hierarchy chain (forward order) - // Example: For parts ["c", "b", "a"], we reverse to ["a", "b", "c"] - // and build hierarchy ["a", "a.b", "a.b.c"] - parts.reverse(); + // Example: + // Input is "a.b.c" + // For property ["c", "b", "a"], we reverse it to ["a", "b", "c"] + properties.reverse(); + // and build chain of object ["a", "a.b", "a.b.c"] const result: string[] = []; let currentChain = ""; - for (let i = 0; i < parts.length; i++) { - currentChain = i === 0 ? parts[0] : `${currentChain}.${parts[i]}`; + for (let i = 0; i < properties.length; i++) { + currentChain = + i === 0 ? properties[0] : `${currentChain}.${properties[i]}`; result.push(currentChain); } return result; } - function trackModification(chain: string, node: TSESTree.Node) { + function setModifiedFlag(chain: string, node: TSESTree.Node) { const scope = sourceCode.getScope(node); const scopeData = getScopeData(scope); - // Mark the modified chain and all its sub-chains as modified - // Example: If "a.b" is modified, then "a.b.c", "a.b.c.d" etc. should also be considered invalid - for (const [existingChain, record] of scopeData.chains) { + for (const [existingChain, record] of scopeData) { // Check if the existing chain starts with the modified chain followed by a dot or bracket // This handles cases where modifying "a.b" should invalidate "a.b.c", "a.b.d", etc. if ( @@ -160,12 +151,10 @@ const noRepeatedMemberAccess = createRule({ record.modified = true; } } - if (scopeData.chains.has(chain)) { - scopeData.chains.get(chain)!.modified = true; - } else { - scopeData.chains.set(chain, { + if (!scopeData.has(chain)) { + scopeData.set(chain, { count: 0, - nodes: [], + node: node as TSESTree.MemberExpression, // to do: check this conversion!! modified: true, }); } @@ -177,45 +166,20 @@ const noRepeatedMemberAccess = createRule({ // not the sub-expressions a.b or a if (node.parent?.type === AST_NODE_TYPES.MemberExpression) return; - const chainInfo = analyzeChain(node); - if (!chainInfo) return; - const scope = sourceCode.getScope(node); const scopeData = getScopeData(scope); - // keeps record of the longest valid chain, and only report it instead of shorter ones (to avoid repeated reports) - let longestValidChain = ""; - - // Update chain statistics for each part of the hierarchy - for (const chain of chainInfo) { - // Skip single-level chains - if (!chain.includes(".")) continue; - - const record = scopeData.chains.get(chain) || { - count: 0, - nodes: [], - modified: false, - }; - if (record.modified) continue; - - record.count++; - record.nodes.push(node); - scopeData.chains.set(chain, record); - - // record longest chain - if ( - record.count >= minOccurrences && - chain.length > longestValidChain.length - ) { - longestValidChain = chain; - } - } + const chainInfo = analyzeChain(node); + if (!chainInfo) return; - // report the longest chain - if (longestValidChain && !reportedChains.has(longestValidChain)) { - const record = scopeData.chains.get(longestValidChain)!; + const longestValidChain = chainInfo[-1]; + const record = scopeData.get(longestValidChain)!; + if ( + record.count >= minOccurrences && + !reportedChains.has(longestValidChain) + ) { context.report({ - node: record.nodes[0], + node: record.node, messageId: "repeatedAccess", data: { chain: longestValidChain, count: record.count }, }); @@ -232,7 +196,7 @@ const noRepeatedMemberAccess = createRule({ const chainInfo = analyzeChain(node.left); if (chainInfo) { for (const chain of chainInfo) { - trackModification(chain, node); + setModifiedFlag(chain, node); } } } @@ -245,7 +209,7 @@ const noRepeatedMemberAccess = createRule({ const chainInfo = analyzeChain(node.argument); if (chainInfo) { for (const chain of chainInfo) { - trackModification(chain, node); + setModifiedFlag(chain, node); } } } @@ -258,7 +222,7 @@ const noRepeatedMemberAccess = createRule({ const chainInfo = analyzeChain(node.callee); if (chainInfo) { for (const chain of chainInfo) { - trackModification(chain, node); + setModifiedFlag(chain, node); } } } diff --git a/plugins/rules/rewriteMember.ts b/plugins/rules/rewriteMember.ts deleted file mode 100644 index 451ff11..0000000 --- a/plugins/rules/rewriteMember.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/utils"; -import { Scope } from "@typescript-eslint/utils/ts-eslint"; -import createRule from "../utils/createRule.js"; - -const noRepeatedMemberAccess = createRule({ - name: "no-repeated-member-access", - meta: { - type: "suggestion", - docs: { - description: - "Optimize repeated member access patterns by extracting variables", - }, - fixable: "code", - schema: [ - { - type: "object", - properties: { - minOccurrences: { type: "number", minimum: 2, default: 3 }, - }, - }, - ], - messages: { - repeatedAccess: - "Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.", - }, - }, - defaultOptions: [{ minOccurrences: 3 }], - - create(context, [options]) { - const sourceCode = context.sourceCode; - const minOccurrences = options.minOccurrences; - - type modifiedMap = Map; - type countMap = Map; - type nodeMap = Map; - - const scopeToModifiedMap = new Map(); - const scopeToCountMap = new Map(); - const scopeToNodeMap = new Map(); - - function analyzeChain(node: TSESTree.MemberExpression) { - let currentNode = node; - let parts = []; - let valid = true; - while (currentNode.type === AST_NODE_TYPES.MemberExpression) { - parts.push - - } - - } - - function trackModification(node: TSESTree.MemberExpression) { - const currentScope = sourceCode.getScope(node); - if (!scopeToModifiedMap.has(currentScope)) { - const newModifiedMap = new Map(); - scopeToModifiedMap.set(currentScope, newModifiedMap); - } - const currentModifiedMap = scopeToModifiedMap.get(currentScope)!; - - // scopeMap[node].modified = true; - } - - function processMemberExpression(node: TSESTree.MemberExpression) {} - - return { - // Track assignment expression - // Example: obj.prop.val = 5 - AssignmentExpression: (node) => { - if (node.left.type === AST_NODE_TYPES.MemberExpression) { - } - }, - - // Track increment/decrement operations - // Example: obj.prop.counter++ modifies "obj.prop.counter" - UpdateExpression: (node) => { - if (node.argument.type === AST_NODE_TYPES.MemberExpression) { - } - }, - - // Track function calls that might modify their arguments - // Example: obj.methods.update() might modify the "obj.methods" chain - CallExpression: (node) => { - if (node.callee.type === AST_NODE_TYPES.MemberExpression) { - } - }, - - // Process member expressions to identify repeated patterns - // Example: Catches obj.prop.val, user.settings.theme, etc. - MemberExpression: (node) => processMemberExpression(node), - }; - }, -}); - -export default noRepeatedMemberAccess; From 00709fbae139f46c7cce1a50c746dd4b3cc6893e Mon Sep 17 00:00:00 2001 From: meowbmw Date: Tue, 17 Jun 2025 16:00:40 +0800 Subject: [PATCH 12/23] ready for review --- package.json | 2 +- plugins/rules/memberAccess.ts | 92 +++++++++++++--------- tests/rules/noRepeatedMemberAccess.test.ts | 29 ++++--- 3 files changed, 74 insertions(+), 49 deletions(-) diff --git a/package.json b/package.json index 283336f..e00413b 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ ], "scripts": { "build": "npx tsc --build ./tsconfig.json", - "test": "mocha --timeout 10000 'dist/tests/**/noRepeatedMemberAccess.test.js'", + "test": "mocha --timeout 10000 'dist/tests/**/**.test.js'", "test:coverage": "c8 npm test", "lint": "npx eslint plugins/ tests/", "watch": "npx tsc --build ./tsconfig.json --watch", diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 17cb45d..6e7e1d2 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -60,34 +60,39 @@ const noRepeatedMemberAccess = createRule({ // Track which chains have already been reported to avoid duplicate reports const reportedChains = new Set(); - type ScopeData = Map< + // We have got two map types, chainMap and scopeDataMap + // it works like: scopeDataMap -> chainMap -> chainInfo + + // Stores info to decide if a extraction is necessary + type ChainMap = Map< string, { count: number; // Number of times this chain is accessed - node: TSESTree.MemberExpression; // AST nodes where this chain appears modified: boolean; // Whether this chain is modified (written to) } >; - // Stores data for each scope using WeakMap to avoid memory leaks - const scopeDataMap = new WeakMap(); + // Stores mapping of scope to ChainMap + const scopeDataMap = new WeakMap(); - function getScopeData(scope: Scope.Scope): ScopeData { + function getChainMap(scope: Scope.Scope): ChainMap { if (!scopeDataMap.has(scope)) { - // Create new scope data if not already present - const newScopeData = new Map< + // Create new info map if not already present + const newChainMap = new Map< string, { count: number; - node: TSESTree.MemberExpression; modified: boolean; } >(); - scopeDataMap.set(scope, newScopeData); + scopeDataMap.set(scope, newChainMap); } return scopeDataMap.get(scope)!; } - function analyzeChain(node: TSESTree.MemberExpression) { + // This function generates ["a", "a.b", "a.b.c"] from a.b.c + // We will further add [count, modified] info to them in ChainMap, and use them as an indication for extraction + // eslint-disable-next-line unicorn/consistent-function-scoping + function analyzeChain(node: TSESTree.MemberExpression): string[] { const properties: string[] = []; // AST is iterated in reverse order let current: TSESTree.Node = node; // Current node in traversal @@ -138,11 +143,10 @@ const noRepeatedMemberAccess = createRule({ function setModifiedFlag(chain: string, node: TSESTree.Node) { const scope = sourceCode.getScope(node); - const scopeData = getScopeData(scope); + const scopeData = getChainMap(scope); for (const [existingChain, record] of scopeData) { - // Check if the existing chain starts with the modified chain followed by a dot or bracket - // This handles cases where modifying "a.b" should invalidate "a.b.c", "a.b.d", etc. + // Check if the existing chain starts with the modified chain followed by a dot or bracket, and if so, marks them as modified if ( existingChain === chain || existingChain.startsWith(chain + ".") || @@ -154,7 +158,6 @@ const noRepeatedMemberAccess = createRule({ if (!scopeData.has(chain)) { scopeData.set(chain, { count: 0, - node: node as TSESTree.MemberExpression, // to do: check this conversion!! modified: true, }); } @@ -166,20 +169,43 @@ const noRepeatedMemberAccess = createRule({ // not the sub-expressions a.b or a if (node.parent?.type === AST_NODE_TYPES.MemberExpression) return; - const scope = sourceCode.getScope(node); - const scopeData = getScopeData(scope); - const chainInfo = analyzeChain(node); if (!chainInfo) return; - const longestValidChain = chainInfo[-1]; - const record = scopeData.get(longestValidChain)!; - if ( - record.count >= minOccurrences && - !reportedChains.has(longestValidChain) - ) { + const scope = sourceCode.getScope(node); + const infoMap = getChainMap(scope); + + // keeps record of the longest valid chain, and only report it instead of shorter ones (to avoid repeated reports) + let longestValidChain = ""; + + // Update chain statistics for each part of the hierarchy + for (const chain of chainInfo) { + // Skip single-level chains + if (!chain.includes(".")) continue; + + const record = infoMap.get(chain) || { + count: 0, + modified: false, + }; + if (record.modified) break; + + record.count++; + infoMap.set(chain, record); + + // record longest extractable chain + if ( + record.count >= minOccurrences && + chain.length > longestValidChain.length + ) { + longestValidChain = chain; + } + } + + // report the longest chain + if (longestValidChain && !reportedChains.has(longestValidChain)) { + const record = infoMap.get(longestValidChain)!; context.report({ - node: record.node, + node: node, messageId: "repeatedAccess", data: { chain: longestValidChain, count: record.count }, }); @@ -194,10 +220,8 @@ const noRepeatedMemberAccess = createRule({ AssignmentExpression: (node) => { if (node.left.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.left); - if (chainInfo) { - for (const chain of chainInfo) { - setModifiedFlag(chain, node); - } + for (const chain of chainInfo) { + setModifiedFlag(chain, node); } } }, @@ -207,10 +231,8 @@ const noRepeatedMemberAccess = createRule({ UpdateExpression: (node) => { if (node.argument.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.argument); - if (chainInfo) { - for (const chain of chainInfo) { - setModifiedFlag(chain, node); - } + for (const chain of chainInfo) { + setModifiedFlag(chain, node); } } }, @@ -220,10 +242,8 @@ const noRepeatedMemberAccess = createRule({ CallExpression: (node) => { if (node.callee.type === AST_NODE_TYPES.MemberExpression) { const chainInfo = analyzeChain(node.callee); - if (chainInfo) { - for (const chain of chainInfo) { - setModifiedFlag(chain, node); - } + for (const chain of chainInfo) { + setModifiedFlag(chain, node); } } }, diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index 041b808..595f8b2 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -36,20 +36,14 @@ describe("Rule: no-spread", () => { `, ` switch (reason) { - case Test.STARTUP: { - return "STARTUP" + case Test.x: { + return "x" } - case Test.DEBOUNCE: { - return "DEBOUNCE" + case Test.y: { + return "y" } - case Test.INSTANT: { - return "INSTANT" - } - case Test.SHUTDOWN: { - return "SHUTDOWN" - } - default: { - return "UNKNOWN" + case Test.z: { + return "z" } } `, @@ -74,6 +68,12 @@ describe("Rule: no-spread", () => { data[0][1].count++; send(data[0][1].id); `, + ` + const a = dataset[0][1].x + dataset[0][1].y; + dataset[0][1].update(); + const b = dataset[0][1].z * 2; + notify(dataset[0][1].timestamp); + `, // WARN: DONT extract when function with possible side effect is called upon ` const a = data.x + data.y; @@ -82,6 +82,11 @@ describe("Rule: no-spread", () => { notify(data.x); `, ` + const first = data.items[0].config['security'].rules[2].level; + data.items[0].config['security'].rules[2].enabled = true; + validate(data.items[0].config['security'].rules[2].level); + `, + ` const v1 = obj[123].value; const v2 = obj[123].value; const v3 = obj[123].value; From a84bc607b4650d4c7eb7f3e3d0fba5a3af3d123a Mon Sep 17 00:00:00 2001 From: meowbmw Date: Tue, 17 Jun 2025 16:05:44 +0800 Subject: [PATCH 13/23] clearer naming --- plugins/rules/memberAccess.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 6e7e1d2..9713415 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -145,14 +145,14 @@ const noRepeatedMemberAccess = createRule({ const scope = sourceCode.getScope(node); const scopeData = getChainMap(scope); - for (const [existingChain, record] of scopeData) { + for (const [existingChain, chainInfo] of scopeData) { // Check if the existing chain starts with the modified chain followed by a dot or bracket, and if so, marks them as modified if ( existingChain === chain || existingChain.startsWith(chain + ".") || existingChain.startsWith(chain + "[") ) { - record.modified = true; + chainInfo.modified = true; } } if (!scopeData.has(chain)) { @@ -173,7 +173,7 @@ const noRepeatedMemberAccess = createRule({ if (!chainInfo) return; const scope = sourceCode.getScope(node); - const infoMap = getChainMap(scope); + const chainMap = getChainMap(scope); // keeps record of the longest valid chain, and only report it instead of shorter ones (to avoid repeated reports) let longestValidChain = ""; @@ -183,18 +183,18 @@ const noRepeatedMemberAccess = createRule({ // Skip single-level chains if (!chain.includes(".")) continue; - const record = infoMap.get(chain) || { + const chainInfo = chainMap.get(chain) || { count: 0, modified: false, }; - if (record.modified) break; + if (chainInfo.modified) break; - record.count++; - infoMap.set(chain, record); + chainInfo.count++; + chainMap.set(chain, chainInfo); // record longest extractable chain if ( - record.count >= minOccurrences && + chainInfo.count >= minOccurrences && chain.length > longestValidChain.length ) { longestValidChain = chain; @@ -203,11 +203,11 @@ const noRepeatedMemberAccess = createRule({ // report the longest chain if (longestValidChain && !reportedChains.has(longestValidChain)) { - const record = infoMap.get(longestValidChain)!; + const chainInfo = chainMap.get(longestValidChain)!; context.report({ node: node, messageId: "repeatedAccess", - data: { chain: longestValidChain, count: record.count }, + data: { chain: longestValidChain, count: chainInfo.count }, }); reportedChains.add(longestValidChain); } From af3a92e896946910e03f7be9cf07fe93e57a655f Mon Sep 17 00:00:00 2001 From: meowbmw Date: Tue, 17 Jun 2025 16:46:57 +0800 Subject: [PATCH 14/23] clean up comment --- plugins/rules/memberAccess.ts | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 9713415..3db7bec 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -2,33 +2,6 @@ import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/utils"; import { Scope } from "@typescript-eslint/utils/ts-eslint"; import createRule from "../utils/createRule.js"; -/** - * Rule to optimize repeated member access patterns by extracting variables - * For more rule details refer to docs/rules/no-repeated-member-access.md - * - * The following material is an overview of implementation details - * It is divided into several phases - * - * 1. Analysis Phase: - * - Traverse AST to identify member access chains (e.g., obj.prop.val) - * - Store chains into hierarchical structures (e.g., ["obj", "obj.prop", "obj.prop.val"]) - * - Cache analysis results to avoid repeatedly processing - * - * 2. Tracking Phase: - * - Count usage frequency of each chain within current scope - * - Identify modified chains (assignments, increments, function calls, etc.) - * - Mark all parts alongside the chain as modified - * - * 3. Reporting Phase: - * - For chains that meet usage threshold and are not modified, suggest variable extraction - * - Report only the longest valid chains - * - * Things to note: - * - Only process chains starting with identifiers or "this" (avoid function call results) - * - Skip computed property access (e.g., obj[key]) - * - Mark modified chains as un-extractable - * - Support TypeScript non-null assertion operator (!) (minor bugs might still persist in some cases) - */ const noRepeatedMemberAccess = createRule({ name: "no-repeated-member-access", meta: { From b6e7556e7c0a7a035361d04a2aa4e9adf7e538d9 Mon Sep 17 00:00:00 2001 From: xpirad Date: Tue, 17 Jun 2025 20:17:31 +0800 Subject: [PATCH 15/23] clean up draft file --- tests/rules/playground.ts | 19 ------------------- todo.txt | 11 ----------- 2 files changed, 30 deletions(-) delete mode 100644 tests/rules/playground.ts delete mode 100644 todo.txt diff --git a/tests/rules/playground.ts b/tests/rules/playground.ts deleted file mode 100644 index b166a2a..0000000 --- a/tests/rules/playground.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { describe, it } from "mocha"; -import { createRuleTester } from "../utils/testUtils.js"; -import noRepeatedMemberAccess from "../../plugins/rules/memberAccess.js"; - -describe("Rule: no-spread", () => { - const ruleTester = createRuleTester(); - - it("validates all test cases for no-repeated-member-access rule", () => { - ruleTester.run("no-repeated-member-access", noRepeatedMemberAccess, { - valid: [ - ` - x.y=a.b.c; - `, - ], - - invalid: [], - }); - }); -}); diff --git a/todo.txt b/todo.txt deleted file mode 100644 index cf5ed5b..0000000 --- a/todo.txt +++ /dev/null @@ -1,11 +0,0 @@ -a.b.c.d.e 从e开始往上遍历，看看触发了多少次 -明确AST和node的遍历过程 - -考虑用树来存储 - -从输入到输出，对数据做加工，考虑用什么数据结构合适 - -明确库函数的作用 - -例子： -get().a.b From dd0221a4fac37d24d47c7bf076b5c056a1fa39ab Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 30 Jun 2025 11:34:47 +0800 Subject: [PATCH 16/23] remove files to support ts debug; add force curly after if; minor adjustments to to test case --- .vscode/launch.json | 33 ---------------------- .vscode/settings.json | 3 -- eslint.config.mjs | 13 +++++---- loader.mjs | 13 --------- plugins/rules/memberAccess.ts | 16 ++++++++--- plugins/rules/noConcatString.ts | 4 ++- tests/rules/noRepeatedMemberAccess.test.ts | 5 ++-- 7 files changed, 25 insertions(+), 62 deletions(-) delete mode 100644 .vscode/launch.json delete mode 100644 .vscode/settings.json delete mode 100644 loader.mjs diff --git a/.vscode/launch.json b/.vscode/launch.json deleted file mode 100644 index 2b62403..0000000 --- a/.vscode/launch.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - // Use IntelliSense to learn about possible attributes. - // Hover to view descriptions of existing attributes. - // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 - "version": "0.2.0", - "configurations": [ - { - "args": [ - "--no-timeouts", - "--colors", - "--inspect-brk", - "tests/rules/playground.ts" - ], - "runtimeArgs": [ - "--experimental-specifier-resolution=node", - "--experimental-loader", - "ts-node/esm", - "--experimental-loader", - "./loader.mjs", - "--no-warnings" - ], - "internalConsoleOptions": "openOnSessionStart", - "name": "Mocha Tests", - "program": "${workspaceRoot}/node_modules/.bin/_mocha", - "request": "launch", - "skipFiles": [ - "${workspaceFolder}/node_modules/**/*.js", - "/**" - ], - "type": "node" - } - ] -} diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 229f4c9..0000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "mochaExplorer.files": "dist/tests/**/*.test.js" -} diff --git a/eslint.config.mjs b/eslint.config.mjs index 1899e6c..1cb467d 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -4,10 +4,13 @@ import { baseConfig } from "@schleifner/eslint-config-base/config.mjs"; export default tseslint.config( { - ignores: [ - "dist/**", - "**/*.mjs", - ], + ignores: ["dist/**", "**/*.mjs"], }, - ...baseConfig + ...baseConfig, + { + files: ["**/*.ts"], + rules: { + curly: ["error", "all"] + }, + } ); diff --git a/loader.mjs b/loader.mjs deleted file mode 100644 index e7c58fe..0000000 --- a/loader.mjs +++ /dev/null @@ -1,13 +0,0 @@ -let is_main = true; - -export const load = (url, context, loadNext) => { - if (is_main) { - is_main = false; - - if (!context.format) { - context.format = "commonjs"; - } - } - - return loadNext(url, context, loadNext); -}; \ No newline at end of file diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 3db7bec..43ceac7 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -140,10 +140,14 @@ const noRepeatedMemberAccess = createRule({ // Skip nodes that are part of larger member expressions // Example: In a.b.c, we process the top-level MemberExpression only, // not the sub-expressions a.b or a - if (node.parent?.type === AST_NODE_TYPES.MemberExpression) return; + if (node.parent?.type === AST_NODE_TYPES.MemberExpression) { + return; + } const chainInfo = analyzeChain(node); - if (!chainInfo) return; + if (!chainInfo) { + return; + } const scope = sourceCode.getScope(node); const chainMap = getChainMap(scope); @@ -154,13 +158,17 @@ const noRepeatedMemberAccess = createRule({ // Update chain statistics for each part of the hierarchy for (const chain of chainInfo) { // Skip single-level chains - if (!chain.includes(".")) continue; + if (!chain.includes(".")) { + continue; + } const chainInfo = chainMap.get(chain) || { count: 0, modified: false, }; - if (chainInfo.modified) break; + if (chainInfo.modified) { + break; + } chainInfo.count++; chainMap.set(chain, chainInfo); diff --git a/plugins/rules/noConcatString.ts b/plugins/rules/noConcatString.ts index 9e030a0..ecc304f 100644 --- a/plugins/rules/noConcatString.ts +++ b/plugins/rules/noConcatString.ts @@ -101,7 +101,9 @@ export default createRule({ // Check for string concatenation with + operator BinaryExpression(node) { // Only check inside loops - if (loopDepth === 0) return; + if (loopDepth === 0) { + return; + } const leftType: ts.Type = parserServices.getTypeAtLocation(node.left); const rightType: ts.Type = parserServices.getTypeAtLocation(node.right); diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index 595f8b2..2b50642 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -26,7 +26,7 @@ describe("Rule: no-spread", () => { // ignore array access ` const x = data[0].value; - data[0].count++; + data[0].count=data[0].count+1; send(data[0].id); `, // Dynamic property access (should be ignored) @@ -100,7 +100,7 @@ describe("Rule: no-spread", () => { `, ` const v1 = a.b.c; - a.b++; + a.b = a.b + 1; const v2 = a.b.c; const v3 = a.b.c; `, @@ -149,7 +149,6 @@ describe("Rule: no-spread", () => { { code: ` const a = data.x + data.y; - // data.update(); const b = data.x * 2; notify(data.x); `, From 2c569fa57df77f2c78daf0795858bcf5fe584c1a Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 30 Jun 2025 12:33:15 +0800 Subject: [PATCH 17/23] refactor memberaccess using tree structures --- plugins/rules/memberAccess.ts | 248 +++++++++++++++++++--------------- 1 file changed, 142 insertions(+), 106 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 43ceac7..1a90636 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -33,37 +33,135 @@ const noRepeatedMemberAccess = createRule({ // Track which chains have already been reported to avoid duplicate reports const reportedChains = new Set(); - // We have got two map types, chainMap and scopeDataMap - // it works like: scopeDataMap -> chainMap -> chainInfo + // Tree-based approach for storing member access chains + // Each node represents a property in the chain (e.g., a -> b -> c for a.b.c) + class ChainNode { + name: string; + count: number = 0; + modified: boolean = false; + parent?: ChainNode; + children: Map = new Map(); - // Stores info to decide if a extraction is necessary - type ChainMap = Map< - string, - { - count: number; // Number of times this chain is accessed - modified: boolean; // Whether this chain is modified (written to) + constructor(name: string) { + this.name = name; } - >; - // Stores mapping of scope to ChainMap - const scopeDataMap = new WeakMap(); - function getChainMap(scope: Scope.Scope): ChainMap { - if (!scopeDataMap.has(scope)) { - // Create new info map if not already present - const newChainMap = new Map< - string, - { - count: number; - modified: boolean; + // Get or create child node + getOrCreateChild(childName: string): ChainNode { + if (!this.children.has(childName)) { + this.children.set(childName, new ChainNode(childName)); + } + return this.children.get(childName)!; + } + + // Get the full chain path from root to this node + getChainPath(): string { + const path: string[] = []; + let current = this as ChainNode | undefined; + while (current && current.name !== "__root__") { + path.unshift(current.name); + current = current.parent; + } + return path.join("."); + } + + // Mark this node and all its descendants as modified + markAsModified(): void { + this.modified = true; + for (const child of this.children.values()) { + child.markAsModified(); + } + } + } + + // Root node for the tree (per scope) + class ChainTree { + root: ChainNode = new ChainNode("__root__"); + + // Insert a chain path into the tree and increment counts + insertChain(properties: string[]): void { + let current = this.root; + + // Navigate/create path in tree + for (const prop of properties) { + const child = current.getOrCreateChild(prop); + child.parent = current; + current = child; + + // Only increment count for non-single properties (chains with dots) + if (properties.length > 1) { + current.count++; + } + } + } + + // Mark a chain and its descendants as modified + markChainAsModified(properties: string[]): void { + let current = this.root; + + // Navigate to the target node + for (const prop of properties) { + const child = current.children.get(prop); + if (child) { + current = child; + } else { + // Create the chain if it doesn't exist + current = current.getOrCreateChild(prop); + current.parent = current; + current.modified = true; + } + } + + // Mark this node and all descendants as modified + current.markAsModified(); + } + + // Find the longest valid chain that meets the minimum occurrence threshold + findLongestValidChain( + minOccurrences: number + ): { chain: string; count: number } | null { + let bestChain: string | null = null; + let bestCount = 0; + + const traverse = (node: ChainNode, depth: number) => { + // Only consider chains with more than one segment (has dots) + if (depth > 1 && !node.modified && node.count >= minOccurrences) { + const chainPath = node.getChainPath(); + if (chainPath.length > (bestChain?.length || 0)) { + bestChain = chainPath; + bestCount = node.count; + } + } + + // Stop traversing if this node is modified + if (node.modified) { + return; + } + + // Recursively traverse children + for (const child of node.children.values()) { + traverse(child, depth + 1); } - >(); - scopeDataMap.set(scope, newChainMap); + }; + + traverse(this.root, 0); + + return bestChain ? { chain: bestChain, count: bestCount } : null; + } + } + + // Stores mapping of scope to ChainTree + const scopeDataMap = new WeakMap(); + + function getChainTree(scope: Scope.Scope): ChainTree { + if (!scopeDataMap.has(scope)) { + scopeDataMap.set(scope, new ChainTree()); } return scopeDataMap.get(scope)!; } - // This function generates ["a", "a.b", "a.b.c"] from a.b.c - // We will further add [count, modified] info to them in ChainMap, and use them as an indication for extraction + // This function generates ["a", "b", "c"] from a.b.c (just the property names) + // The tree structure will handle the hierarchy automatically // eslint-disable-next-line unicorn/consistent-function-scoping function analyzeChain(node: TSESTree.MemberExpression): string[] { const properties: string[] = []; // AST is iterated in reverse order @@ -96,44 +194,15 @@ const noRepeatedMemberAccess = createRule({ properties.push("this"); } // ignore other patterns - // Generate hierarchy chain (forward order) - // Example: - // Input is "a.b.c" - // For property ["c", "b", "a"], we reverse it to ["a", "b", "c"] + // Reverse to get forward order: ["a", "b", "c"] properties.reverse(); - - // and build chain of object ["a", "a.b", "a.b.c"] - const result: string[] = []; - let currentChain = ""; - for (let i = 0; i < properties.length; i++) { - currentChain = - i === 0 ? properties[0] : `${currentChain}.${properties[i]}`; - result.push(currentChain); - } - - return result; + return properties; } - function setModifiedFlag(chain: string, node: TSESTree.Node) { + function setModifiedFlag(chain: string[], node: TSESTree.Node) { const scope = sourceCode.getScope(node); - const scopeData = getChainMap(scope); - - for (const [existingChain, chainInfo] of scopeData) { - // Check if the existing chain starts with the modified chain followed by a dot or bracket, and if so, marks them as modified - if ( - existingChain === chain || - existingChain.startsWith(chain + ".") || - existingChain.startsWith(chain + "[") - ) { - chainInfo.modified = true; - } - } - if (!scopeData.has(chain)) { - scopeData.set(chain, { - count: 0, - modified: true, - }); - } + const chainTree = getChainTree(scope); + chainTree.markChainAsModified(chain); } function processMemberExpression(node: TSESTree.MemberExpression) { @@ -144,53 +213,26 @@ const noRepeatedMemberAccess = createRule({ return; } - const chainInfo = analyzeChain(node); - if (!chainInfo) { + const properties = analyzeChain(node); + if (!properties || properties.length === 0) { return; } const scope = sourceCode.getScope(node); - const chainMap = getChainMap(scope); - - // keeps record of the longest valid chain, and only report it instead of shorter ones (to avoid repeated reports) - let longestValidChain = ""; - - // Update chain statistics for each part of the hierarchy - for (const chain of chainInfo) { - // Skip single-level chains - if (!chain.includes(".")) { - continue; - } - - const chainInfo = chainMap.get(chain) || { - count: 0, - modified: false, - }; - if (chainInfo.modified) { - break; - } - - chainInfo.count++; - chainMap.set(chain, chainInfo); + const chainTree = getChainTree(scope); - // record longest extractable chain - if ( - chainInfo.count >= minOccurrences && - chain.length > longestValidChain.length - ) { - longestValidChain = chain; - } - } + // Insert the chain into the tree (this will increment counts automatically) + chainTree.insertChain(properties); - // report the longest chain - if (longestValidChain && !reportedChains.has(longestValidChain)) { - const chainInfo = chainMap.get(longestValidChain)!; + // Find the longest valid chain to report + const result = chainTree.findLongestValidChain(minOccurrences); + if (result && !reportedChains.has(result.chain)) { context.report({ node: node, messageId: "repeatedAccess", - data: { chain: longestValidChain, count: chainInfo.count }, + data: { chain: result.chain, count: result.count }, }); - reportedChains.add(longestValidChain); + reportedChains.add(result.chain); } } @@ -200,10 +242,8 @@ const noRepeatedMemberAccess = createRule({ // This prevents us from extracting chains that are modified AssignmentExpression: (node) => { if (node.left.type === AST_NODE_TYPES.MemberExpression) { - const chainInfo = analyzeChain(node.left); - for (const chain of chainInfo) { - setModifiedFlag(chain, node); - } + const properties = analyzeChain(node.left); + setModifiedFlag(properties, node); } }, @@ -211,10 +251,8 @@ const noRepeatedMemberAccess = createRule({ // Example: obj.prop.counter++ modifies "obj.prop.counter" UpdateExpression: (node) => { if (node.argument.type === AST_NODE_TYPES.MemberExpression) { - const chainInfo = analyzeChain(node.argument); - for (const chain of chainInfo) { - setModifiedFlag(chain, node); - } + const properties = analyzeChain(node.argument); + setModifiedFlag(properties, node); } }, @@ -222,10 +260,8 @@ const noRepeatedMemberAccess = createRule({ // Example: obj.methods.update() might modify the "obj.methods" chain CallExpression: (node) => { if (node.callee.type === AST_NODE_TYPES.MemberExpression) { - const chainInfo = analyzeChain(node.callee); - for (const chain of chainInfo) { - setModifiedFlag(chain, node); - } + const properties = analyzeChain(node.callee); + setModifiedFlag(properties, node); } }, From 7cfe59dc08e55754843799bc49bcdb460088f984 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 30 Jun 2025 12:37:23 +0800 Subject: [PATCH 18/23] change test case --- tests/rules/noRepeatedMemberAccess.test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index 2b50642..f000d2b 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -74,13 +74,6 @@ describe("Rule: no-spread", () => { const b = dataset[0][1].z * 2; notify(dataset[0][1].timestamp); `, - // WARN: DONT extract when function with possible side effect is called upon - ` - const a = data.x + data.y; - data.update(); - const b = data.x * 2; - notify(data.x); - `, ` const first = data.items[0].config['security'].rules[2].level; data.items[0].config['security'].rules[2].enabled = true; From 86081467111ea9464b9ca69d29021deb6df5bd07 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 30 Jun 2025 13:27:59 +0800 Subject: [PATCH 19/23] fix logic in mark modified --- plugins/rules/memberAccess.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 1a90636..41f303b 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -99,16 +99,16 @@ const noRepeatedMemberAccess = createRule({ markChainAsModified(properties: string[]): void { let current = this.root; - // Navigate to the target node + // Navigate to the target node, creating nodes if they don't exist for (const prop of properties) { const child = current.children.get(prop); if (child) { current = child; } else { // Create the chain if it doesn't exist - current = current.getOrCreateChild(prop); - current.parent = current; - current.modified = true; + const newChild = current.getOrCreateChild(prop); + newChild.parent = current; + current = newChild; } } From 802b6d4ae8b8a8ea92382ff0c56e965bd187b7d2 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 30 Jun 2025 17:16:37 +0800 Subject: [PATCH 20/23] set class members to private & add chains cache --- docs/rules/no-repeated-member-access.md | 12 -- plugins/rules/memberAccess.ts | 140 ++++++++++++--------- tests/rules/noRepeatedMemberAccess.test.ts | 25 +++- 3 files changed, 106 insertions(+), 71 deletions(-) diff --git a/docs/rules/no-repeated-member-access.md b/docs/rules/no-repeated-member-access.md index d51e938..3d619f8 100644 --- a/docs/rules/no-repeated-member-access.md +++ b/docs/rules/no-repeated-member-access.md @@ -28,18 +28,6 @@ y = a.b.c; z = a.b.c; ``` -## Rule Options - -This rule accepts an options object with the following properties: - -```json -{ - "minOccurrences": 3 -} -``` - -- `minOccurrences` (default: 3): Minimum number of times a member chain must be accessed before triggering the rule - ## Examples ### Incorrect diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 41f303b..459ea1a 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -10,25 +10,17 @@ const noRepeatedMemberAccess = createRule({ description: "Optimize repeated member access patterns by extracting variables", }, + schema: [], fixable: "code", - schema: [ - { - type: "object", - properties: { - minOccurrences: { type: "number", minimum: 2, default: 3 }, - }, - }, - ], messages: { repeatedAccess: - "Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.", + "Member chain '{{ chain }}' is accessed multiple times. Extract to variable.", }, }, - defaultOptions: [{ minOccurrences: 3 }], + defaultOptions: [], - create(context, [options]) { + create(context) { const sourceCode = context.sourceCode; - const minOccurrences = options.minOccurrences; // Track which chains have already been reported to avoid duplicate reports const reportedChains = new Set(); @@ -36,16 +28,46 @@ const noRepeatedMemberAccess = createRule({ // Tree-based approach for storing member access chains // Each node represents a property in the chain (e.g., a -> b -> c for a.b.c) class ChainNode { - name: string; - count: number = 0; - modified: boolean = false; - parent?: ChainNode; - children: Map = new Map(); + private name: string; + private count: number = 0; + private modified: boolean = false; + private parent?: ChainNode; + private children: Map = new Map(); constructor(name: string) { this.name = name; } + // Getter methods for private properties + get getName(): string { + return this.name; + } + + get getCount(): number { + return this.count; + } + + get isModified(): boolean { + return this.modified; + } + + get getParent(): ChainNode | undefined { + return this.parent; + } + + get getChildren(): Map { + return this.children; + } + + // Setter methods for private properties + set setParent(parent: ChainNode | undefined) { + this.parent = parent; + } + + incrementCount(): void { + this.count++; + } + // Get or create child node getOrCreateChild(childName: string): ChainNode { if (!this.children.has(childName)) { @@ -56,19 +78,23 @@ const noRepeatedMemberAccess = createRule({ // Get the full chain path from root to this node getChainPath(): string { + // Build path from child to root, then reverse at the end const path: string[] = []; let current = this as ChainNode | undefined; - while (current && current.name !== "__root__") { - path.unshift(current.name); - current = current.parent; + while (current && current.getName !== "__root__") { + path.push(current.getName); + current = current.getParent; } + + // Reverse the array once at the end + path.reverse(); return path.join("."); } // Mark this node and all its descendants as modified markAsModified(): void { this.modified = true; - for (const child of this.children.values()) { + for (const child of this.getChildren.values()) { child.markAsModified(); } } @@ -76,7 +102,9 @@ const noRepeatedMemberAccess = createRule({ // Root node for the tree (per scope) class ChainTree { - root: ChainNode = new ChainNode("__root__"); + private root: ChainNode = new ChainNode("__root__"); + private validChainsCache: Array<{ chain: string }> = []; + private cacheValid: boolean = false; // Insert a chain path into the tree and increment counts insertChain(properties: string[]): void { @@ -85,14 +113,15 @@ const noRepeatedMemberAccess = createRule({ // Navigate/create path in tree for (const prop of properties) { const child = current.getOrCreateChild(prop); - child.parent = current; + child.setParent = current; current = child; // Only increment count for non-single properties (chains with dots) if (properties.length > 1) { - current.count++; + current.incrementCount(); } } + this.cacheValid = false; } // Mark a chain and its descendants as modified @@ -101,52 +130,47 @@ const noRepeatedMemberAccess = createRule({ // Navigate to the target node, creating nodes if they don't exist for (const prop of properties) { - const child = current.children.get(prop); - if (child) { - current = child; - } else { - // Create the chain if it doesn't exist - const newChild = current.getOrCreateChild(prop); - newChild.parent = current; - current = newChild; - } + const newChild = current.getOrCreateChild(prop); + newChild.setParent = current; + current = newChild; } // Mark this node and all descendants as modified current.markAsModified(); + this.cacheValid = false; } - // Find the longest valid chain that meets the minimum occurrence threshold - findLongestValidChain( - minOccurrences: number - ): { chain: string; count: number } | null { - let bestChain: string | null = null; - let bestCount = 0; + // Find any valid chain that meets the minimum occurrence threshold + findValidChains() { + if (this.cacheValid) { + return this.validChainsCache; + } + const validChains: Array<{ chain: string }> = []; const traverse = (node: ChainNode, depth: number) => { // Only consider chains with more than one segment (has dots) - if (depth > 1 && !node.modified && node.count >= minOccurrences) { + if (depth > 1 && !node.isModified && node.getCount >= 2) { const chainPath = node.getChainPath(); - if (chainPath.length > (bestChain?.length || 0)) { - bestChain = chainPath; - bestCount = node.count; - } + validChains.push({ + chain: chainPath, + }); } // Stop traversing if this node is modified - if (node.modified) { + if (node.isModified) { return; } // Recursively traverse children - for (const child of node.children.values()) { + for (const child of node.getChildren.values()) { traverse(child, depth + 1); } }; traverse(this.root, 0); - - return bestChain ? { chain: bestChain, count: bestCount } : null; + this.cacheValid = true; + this.validChainsCache = validChains; + return validChains; } } @@ -224,15 +248,17 @@ const noRepeatedMemberAccess = createRule({ // Insert the chain into the tree (this will increment counts automatically) chainTree.insertChain(properties); - // Find the longest valid chain to report - const result = chainTree.findLongestValidChain(minOccurrences); - if (result && !reportedChains.has(result.chain)) { - context.report({ - node: node, - messageId: "repeatedAccess", - data: { chain: result.chain, count: result.count }, - }); - reportedChains.add(result.chain); + // Find all valid chains to report + const validChains = chainTree.findValidChains(); + for (const result of validChains) { + if (!reportedChains.has(result.chain)) { + context.report({ + node: node, + messageId: "repeatedAccess", + data: { chain: result.chain }, + }); + reportedChains.add(result.chain); + } } } diff --git a/tests/rules/noRepeatedMemberAccess.test.ts b/tests/rules/noRepeatedMemberAccess.test.ts index f000d2b..939e333 100644 --- a/tests/rules/noRepeatedMemberAccess.test.ts +++ b/tests/rules/noRepeatedMemberAccess.test.ts @@ -115,7 +115,25 @@ describe("Rule: no-spread", () => { const v2 = a.b.c; const v3 = a.b.c; `, - errors: [{ messageId: "repeatedAccess" }], + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], + }, + { + code: ` + const data = a.b.c.d; + const data = a.b.c.d; + const data = a.b.c.d; + const data = a.b.c; + const data = a.b.c; + + `, + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], }, { code: ` @@ -137,7 +155,10 @@ describe("Rule: no-spread", () => { return obj.a.b.d; } `, - errors: [{ messageId: "repeatedAccess" }], + errors: [ + { messageId: "repeatedAccess" }, + { messageId: "repeatedAccess" }, + ], }, { code: ` From 337bb8d97d334f90468623cd77c9b56239780649 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Mon, 30 Jun 2025 21:25:56 +0800 Subject: [PATCH 21/23] set modified in constructor to ensure the flag is inhereited; construct path during constructor --- plugins/rules/memberAccess.ts | 56 ++++++++++------------------------- 1 file changed, 16 insertions(+), 40 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 459ea1a..926c4f7 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -28,19 +28,23 @@ const noRepeatedMemberAccess = createRule({ // Tree-based approach for storing member access chains // Each node represents a property in the chain (e.g., a -> b -> c for a.b.c) class ChainNode { - private name: string; private count: number = 0; private modified: boolean = false; private parent?: ChainNode; private children: Map = new Map(); + private path: string; - constructor(name: string) { - this.name = name; - } + constructor(name: string, parent?: ChainNode) { + this.parent = parent; + this.modified = this.parent?.modified || false; - // Getter methods for private properties - get getName(): string { - return this.name; + if (name === "__root__") { + this.path = ""; + } else if (!this.parent || this.parent.path === "") { + this.path = name; + } else { + this.path = this.parent.path + "." + name; + } } get getCount(): number { @@ -59,9 +63,8 @@ const noRepeatedMemberAccess = createRule({ return this.children; } - // Setter methods for private properties - set setParent(parent: ChainNode | undefined) { - this.parent = parent; + get getPath(): string { + return this.path; } incrementCount(): void { @@ -71,30 +74,15 @@ const noRepeatedMemberAccess = createRule({ // Get or create child node getOrCreateChild(childName: string): ChainNode { if (!this.children.has(childName)) { - this.children.set(childName, new ChainNode(childName)); + this.children.set(childName, new ChainNode(childName, this)); } return this.children.get(childName)!; } - // Get the full chain path from root to this node - getChainPath(): string { - // Build path from child to root, then reverse at the end - const path: string[] = []; - let current = this as ChainNode | undefined; - while (current && current.getName !== "__root__") { - path.push(current.getName); - current = current.getParent; - } - - // Reverse the array once at the end - path.reverse(); - return path.join("."); - } - // Mark this node and all its descendants as modified markAsModified(): void { this.modified = true; - for (const child of this.getChildren.values()) { + for (const child of this.children.values()) { child.markAsModified(); } } @@ -103,8 +91,6 @@ const noRepeatedMemberAccess = createRule({ // Root node for the tree (per scope) class ChainTree { private root: ChainNode = new ChainNode("__root__"); - private validChainsCache: Array<{ chain: string }> = []; - private cacheValid: boolean = false; // Insert a chain path into the tree and increment counts insertChain(properties: string[]): void { @@ -113,7 +99,6 @@ const noRepeatedMemberAccess = createRule({ // Navigate/create path in tree for (const prop of properties) { const child = current.getOrCreateChild(prop); - child.setParent = current; current = child; // Only increment count for non-single properties (chains with dots) @@ -121,7 +106,6 @@ const noRepeatedMemberAccess = createRule({ current.incrementCount(); } } - this.cacheValid = false; } // Mark a chain and its descendants as modified @@ -131,28 +115,22 @@ const noRepeatedMemberAccess = createRule({ // Navigate to the target node, creating nodes if they don't exist for (const prop of properties) { const newChild = current.getOrCreateChild(prop); - newChild.setParent = current; current = newChild; } // Mark this node and all descendants as modified current.markAsModified(); - this.cacheValid = false; } // Find any valid chain that meets the minimum occurrence threshold findValidChains() { - if (this.cacheValid) { - return this.validChainsCache; - } const validChains: Array<{ chain: string }> = []; const traverse = (node: ChainNode, depth: number) => { // Only consider chains with more than one segment (has dots) if (depth > 1 && !node.isModified && node.getCount >= 2) { - const chainPath = node.getChainPath(); validChains.push({ - chain: chainPath, + chain: node.getPath, }); } @@ -168,8 +146,6 @@ const noRepeatedMemberAccess = createRule({ }; traverse(this.root, 0); - this.cacheValid = true; - this.validChainsCache = validChains; return validChains; } } From f9382c696cdff5b54178f0574b47317a92b47154 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Tue, 1 Jul 2025 13:25:31 +0800 Subject: [PATCH 22/23] clean up class member & rewrite path generate logic & add visitor function --- plugins/rules/memberAccess.ts | 72 +++++++++++++++-------------------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/plugins/rules/memberAccess.ts b/plugins/rules/memberAccess.ts index 926c4f7..d6a4966 100644 --- a/plugins/rules/memberAccess.ts +++ b/plugins/rules/memberAccess.ts @@ -32,19 +32,10 @@ const noRepeatedMemberAccess = createRule({ private modified: boolean = false; private parent?: ChainNode; private children: Map = new Map(); - private path: string; - constructor(name: string, parent?: ChainNode) { + constructor(parent?: ChainNode) { this.parent = parent; this.modified = this.parent?.modified || false; - - if (name === "__root__") { - this.path = ""; - } else if (!this.parent || this.parent.path === "") { - this.path = name; - } else { - this.path = this.parent.path + "." + name; - } } get getCount(): number { @@ -55,18 +46,10 @@ const noRepeatedMemberAccess = createRule({ return this.modified; } - get getParent(): ChainNode | undefined { - return this.parent; - } - get getChildren(): Map { return this.children; } - get getPath(): string { - return this.path; - } - incrementCount(): void { this.count++; } @@ -74,7 +57,7 @@ const noRepeatedMemberAccess = createRule({ // Get or create child node getOrCreateChild(childName: string): ChainNode { if (!this.children.has(childName)) { - this.children.set(childName, new ChainNode(childName, this)); + this.children.set(childName, new ChainNode(this)); } return this.children.get(childName)!; } @@ -90,47 +73,49 @@ const noRepeatedMemberAccess = createRule({ // Root node for the tree (per scope) class ChainTree { - private root: ChainNode = new ChainNode("__root__"); + private root: ChainNode = new ChainNode(); - // Insert a chain path into the tree and increment counts - insertChain(properties: string[]): void { + // Visitor function to navigate through property chain + private visitChainPath( + properties: string[], + process: (node: ChainNode) => void + ): ChainNode { let current = this.root; - // Navigate/create path in tree + // Navigate/process node in the tree for (const prop of properties) { const child = current.getOrCreateChild(prop); current = child; - - // Only increment count for non-single properties (chains with dots) - if (properties.length > 1) { - current.incrementCount(); - } + process(current); } + + return current; + } + + // Insert a chain path into the tree and increment counts + insertChain(properties: string[]): void { + this.visitChainPath(properties, (node) => { + node.incrementCount(); + }); } // Mark a chain and its descendants as modified markChainAsModified(properties: string[]): void { - let current = this.root; - - // Navigate to the target node, creating nodes if they don't exist - for (const prop of properties) { - const newChild = current.getOrCreateChild(prop); - current = newChild; - } + const targetNode = this.visitChainPath(properties, () => {}); // Mark this node and all descendants as modified - current.markAsModified(); + targetNode.markAsModified(); } // Find any valid chain that meets the minimum occurrence threshold findValidChains() { const validChains: Array<{ chain: string }> = []; - const traverse = (node: ChainNode, depth: number) => { + const dfs = (node: ChainNode, pathArray: string[]) => { // Only consider chains with more than one segment (has dots) - if (depth > 1 && !node.isModified && node.getCount >= 2) { + if (pathArray.length > 1 && !node.isModified && node.getCount >= 2) { validChains.push({ - chain: node.getPath, + chain: pathArray.join("."), }); } @@ -140,12 +125,15 @@ const noRepeatedMemberAccess = createRule({ } // Recursively traverse children - for (const child of node.getChildren.values()) { - traverse(child, depth + 1); + for (const [childName, child] of node.getChildren) { + pathArray.push(childName); + dfs(child, pathArray); + pathArray.pop(); } }; - traverse(this.root, 0); + // Start DFS from root with empty path array + dfs(this.root, []); return validChains; } } From c105abbbe7c4f499d399dc623af53d35c58b7c38 Mon Sep 17 00:00:00 2001 From: meowbmw Date: Tue, 1 Jul 2025 15:56:14 +0800 Subject: [PATCH 23/23] Update readme --- Readme.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Readme.md b/Readme.md index 1323887..109c25d 100644 --- a/Readme.md +++ b/Readme.md @@ -25,10 +25,22 @@ Optimizes code for better WebAssembly performance: - `array-init-style`: Recommends using `new Array()` instead of `[]` for initializing empty arrays +- `no-repeated-member-access`: Recommends extracting repeated member access to improve performance + ## Configuration See `sample_config/sample_eslint.config.mjs` for a detailed example of how to configure and use this plugin. +It includes some other pre-written rules including: + +- `no-implicit-globals`: Warns against creating implicit global variables +- `curly`: Requires curly braces for all control statements to prevent error-prone one-liner code +- `@typescript-eslint/no-restricted-types`: Enforces AssemblyScript-specific type usage: + - Use `string` instead of `String` + - Use `bool` instead of `Boolean` + - Disallows unsupported types like `undefined` and `object` +- `@typescript-eslint/adjacent-overload-signatures`: Requires overload signatures to be adjacent + ## Documentation For detailed rule documentation, see the [docs/rules](./docs/rules) directory.