Skip to content

Commit 1e1b586

Browse files
committed
rephrase comment & refactors (should have no behavior change); still need to pass newly added cases
1 parent 8821081 commit 1e1b586

File tree

3 files changed

+68
-38
lines changed

3 files changed

+68
-38
lines changed

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
代码部分请仅使用英文,其他部分使用中文,尽量在说明的同时也给一些实例方便理解
2-
在回答时不要使用类似于... rest of the code ...的方式省略代码
2+
在写代码时请给出类型信息

plugins/perf-plugin.ts

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
AST_NODE_TYPES,
33
ESLintUtils,
44
TSESTree,
5+
TSESLint,
56
} from "@typescript-eslint/utils";
67

78
/**
@@ -80,21 +81,34 @@ const arrayInitStyle: ESLintUtils.RuleModule<
8081
});
8182

8283
/**
83-
* Rule: Member Variable / Array Element Get
84-
* Avoid accessing member variables multiple times in the same context.
85-
* This can significantly increase code size (8% in some proxy-like usecases) and waste CPU!
84+
* Rule: No Repeated Member Access
8685
*
87-
* Implementation overview:
88-
* - For each outermost MemberExpression (e.g. ctx.data.v1), extract the "object chain" part (e.g. ctx.data).
89-
* - Only static properties and static indices are supported; dynamic properties are ignored.
90-
* - Use the current scope's range and the object chain as a unique key to count occurrences.
91-
* - When the same object chain is accessed more than once in the same scope (default threshold: 2), report a warning to suggest refactoring.
92-
* - This helps avoid repeated property lookups and encourages caching the result in a local variable.
86+
* Description:
87+
* Detects and warns when member variables or properties are accessed multiple times
88+
* in the same scope. Repeated property access can significantly increase code size
89+
* (up to 8% in proxy-like use cases) and waste CPU cycles.
90+
*
91+
* Implementation:
92+
* 1. Identifies outermost MemberExpression nodes (e.g., ctx.data.value)
93+
* 2. Extracts the "object chain" part (e.g., ctx.data)
94+
* 3. Counts occurrences of the same chain in each scope
95+
* 4. Reports when occurrences exceed the threshold (default: 2)
96+
* 5. Suggests extracting the chain into a local variable
97+
*
98+
* Limitations:
99+
* - Only static properties and indices are supported
100+
* - Dynamic properties (obj[variable]) are ignored
101+
* - Constants, enums, and imports are skipped
93102
*
94103
* Example:
104+
* // Bad - repeated access
95105
* const v1 = ctx.data.v1;
96106
* const v2 = ctx.data.v2;
97-
* The rule will suggest extracting 'ctx.data' into a variable if accessed multiple times.
107+
*
108+
* // Good - extracted common chain
109+
* const ctxData = ctx.data;
110+
* const v1 = ctxData.v1;
111+
* const v2 = ctxData.v2;
98112
*/
99113

100114
// Define the type for the rule options
@@ -265,40 +279,54 @@ const noRepeatedMemberAccess: ESLintUtils.RuleModule<
265279
const scope = context.sourceCode.getScope(node);
266280
if (!scope || !scope.block || !scope.block.range) return;
267281

268-
let variable = null;
269-
let currentScope = scope;
282+
// Find variable in scope chain
283+
const variable = findVariableInScopeChain(scope, baseObjectName);
270284

271-
while (currentScope) {
272-
variable = currentScope.variables.find(
273-
(v) => v.name === baseObjectName
274-
);
275-
if (variable) break;
285+
// Skip certain variable types that shouldn't be extracted
286+
if (
287+
variable &&
288+
(isConstVariable(variable) ||
289+
isEnumVariable(variable) ||
290+
isImportVariable(variable))
291+
) {
292+
return;
293+
}
276294

277-
const upperScope = currentScope.upper;
278-
if (!upperScope) break;
279-
currentScope = upperScope;
295+
// Helper functions
296+
function findVariableInScopeChain(scope: TSESLint.Scope.Scope, name: string) {
297+
let currentScope: TSESLint.Scope.Scope | null = scope;
298+
while (currentScope) {
299+
const variable = currentScope.variables.find(
300+
(v) => v.name === name
301+
);
302+
if (variable) return variable;
303+
currentScope = currentScope.upper;
304+
}
305+
return null;
280306
}
281307

282-
if (variable) {
283-
// check for const/enum/import
284-
const isConst = variable.defs.every(
308+
function isConstVariable(variable: TSESLint.Scope.Variable) {
309+
return variable.defs.every(
285310
(def) => def.node && "kind" in def.node && def.node.kind === "const"
286311
);
287-
const isEnum = variable.defs.some(
312+
}
313+
314+
function isEnumVariable(variable: TSESLint.Scope.Variable) {
315+
return variable.defs.some(
288316
(def) =>
289317
(def.parent as TSESTree.Node)?.type ===
290318
AST_NODE_TYPES.TSEnumDeclaration
291319
);
292-
const isImport = variable.defs.some(
320+
}
321+
322+
function isImportVariable(variable: TSESLint.Scope.Variable) {
323+
return variable.defs.some(
293324
(def) =>
294325
def.type === "ImportBinding" ||
295326
(def.node &&
296327
"type" in def.node &&
297328
def.node.type === AST_NODE_TYPES.ImportDeclaration)
298329
);
299-
if (isConst || isEnum || isImport) {
300-
return; // skip these types as extracting them won't be helpful
301-
}
302330
}
303331

304332
const key = `${scope.block.range.join("-")}-${objectChain}`;

tests/perf-plugin.test.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ RuleTester.itOnly = test.it.only;
1010

1111
const ruleTester = new RuleTester();
1212

13-
1413
ruleTester.run(
1514
"no-repeated-member-access",
1615
perfPlugin.rules["no-repeated-member-access"],
1716
{
18-
valid: [`
17+
valid: [
18+
`
1919
import { DelayType } from "@utils/time-utils";
2020
2121
export namespace TimingConfig {
@@ -29,7 +29,8 @@ export namespace TimingConfig {
2929
*/
3030
export const highSpeedPositioningCooldown = DelayType.LONG;
3131
}
32-
32+
`,
33+
`
3334
/**
3435
* Distance-based emission strategy
3536
*/
@@ -65,7 +66,8 @@ export class DistanceBasedDeliveryStrategy implements EmissionStrategy {
6566
clone.error = this.error;
6667
return clone;
6768
}
68-
}
69+
}`,
70+
`
6971
7072
let activeConfig: BaseConfiguration;
7173
const configData = AppContext.loadSettings();
@@ -85,7 +87,8 @@ if (runtimeEnv && eventController) {
8587
runtimeEnv.registerStateChangeListener(SystemHookManager.handleStateChange);
8688
this.lastKnownState = runtimeEnv.getCurrentEnvironmentState();
8789
}
88-
90+
`,
91+
`
8992
if (
9093
currentSession.authType == AuthType.PRIVILEGED &&
9194
currentSession.status == SessionStatus.ACTIVE &&
@@ -95,10 +98,9 @@ if (
9598
serviceInstance.lastSyncTime = timestamp;
9699
}
97100
98-
`],
99-
100-
invalid: [
101+
`,
101102
],
103+
invalid: [],
102104
}
103105
);
104106

@@ -296,4 +298,4 @@ const v1 = _ctx_data.v1;
296298
],
297299
}
298300
);
299-
*/
301+
*/

0 commit comments

Comments
 (0)