Skip to content

Commit 802b6d4

Browse files
committed
set class members to private & add chains cache
1 parent 8608146 commit 802b6d4

File tree

3 files changed

+106
-71
lines changed

3 files changed

+106
-71
lines changed

docs/rules/no-repeated-member-access.md

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,6 @@ y = a.b.c;
2828
z = a.b.c;
2929
```
3030

31-
## Rule Options
32-
33-
This rule accepts an options object with the following properties:
34-
35-
```json
36-
{
37-
"minOccurrences": 3
38-
}
39-
```
40-
41-
- `minOccurrences` (default: 3): Minimum number of times a member chain must be accessed before triggering the rule
42-
4331
## Examples
4432

4533
### Incorrect

plugins/rules/memberAccess.ts

Lines changed: 83 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -10,42 +10,64 @@ const noRepeatedMemberAccess = createRule({
1010
description:
1111
"Optimize repeated member access patterns by extracting variables",
1212
},
13+
schema: [],
1314
fixable: "code",
14-
schema: [
15-
{
16-
type: "object",
17-
properties: {
18-
minOccurrences: { type: "number", minimum: 2, default: 3 },
19-
},
20-
},
21-
],
2215
messages: {
2316
repeatedAccess:
24-
"Member chain '{{ chain }}' accessed {{ count }} times. Extract to variable.",
17+
"Member chain '{{ chain }}' is accessed multiple times. Extract to variable.",
2518
},
2619
},
27-
defaultOptions: [{ minOccurrences: 3 }],
20+
defaultOptions: [],
2821

29-
create(context, [options]) {
22+
create(context) {
3023
const sourceCode = context.sourceCode;
31-
const minOccurrences = options.minOccurrences;
3224

3325
// Track which chains have already been reported to avoid duplicate reports
3426
const reportedChains = new Set<string>();
3527

3628
// Tree-based approach for storing member access chains
3729
// Each node represents a property in the chain (e.g., a -> b -> c for a.b.c)
3830
class ChainNode {
39-
name: string;
40-
count: number = 0;
41-
modified: boolean = false;
42-
parent?: ChainNode;
43-
children: Map<string, ChainNode> = new Map();
31+
private name: string;
32+
private count: number = 0;
33+
private modified: boolean = false;
34+
private parent?: ChainNode;
35+
private children: Map<string, ChainNode> = new Map();
4436

4537
constructor(name: string) {
4638
this.name = name;
4739
}
4840

41+
// Getter methods for private properties
42+
get getName(): string {
43+
return this.name;
44+
}
45+
46+
get getCount(): number {
47+
return this.count;
48+
}
49+
50+
get isModified(): boolean {
51+
return this.modified;
52+
}
53+
54+
get getParent(): ChainNode | undefined {
55+
return this.parent;
56+
}
57+
58+
get getChildren(): Map<string, ChainNode> {
59+
return this.children;
60+
}
61+
62+
// Setter methods for private properties
63+
set setParent(parent: ChainNode | undefined) {
64+
this.parent = parent;
65+
}
66+
67+
incrementCount(): void {
68+
this.count++;
69+
}
70+
4971
// Get or create child node
5072
getOrCreateChild(childName: string): ChainNode {
5173
if (!this.children.has(childName)) {
@@ -56,27 +78,33 @@ const noRepeatedMemberAccess = createRule({
5678

5779
// Get the full chain path from root to this node
5880
getChainPath(): string {
81+
// Build path from child to root, then reverse at the end
5982
const path: string[] = [];
6083
let current = this as ChainNode | undefined;
61-
while (current && current.name !== "__root__") {
62-
path.unshift(current.name);
63-
current = current.parent;
84+
while (current && current.getName !== "__root__") {
85+
path.push(current.getName);
86+
current = current.getParent;
6487
}
88+
89+
// Reverse the array once at the end
90+
path.reverse();
6591
return path.join(".");
6692
}
6793

6894
// Mark this node and all its descendants as modified
6995
markAsModified(): void {
7096
this.modified = true;
71-
for (const child of this.children.values()) {
97+
for (const child of this.getChildren.values()) {
7298
child.markAsModified();
7399
}
74100
}
75101
}
76102

77103
// Root node for the tree (per scope)
78104
class ChainTree {
79-
root: ChainNode = new ChainNode("__root__");
105+
private root: ChainNode = new ChainNode("__root__");
106+
private validChainsCache: Array<{ chain: string }> = [];
107+
private cacheValid: boolean = false;
80108

81109
// Insert a chain path into the tree and increment counts
82110
insertChain(properties: string[]): void {
@@ -85,14 +113,15 @@ const noRepeatedMemberAccess = createRule({
85113
// Navigate/create path in tree
86114
for (const prop of properties) {
87115
const child = current.getOrCreateChild(prop);
88-
child.parent = current;
116+
child.setParent = current;
89117
current = child;
90118

91119
// Only increment count for non-single properties (chains with dots)
92120
if (properties.length > 1) {
93-
current.count++;
121+
current.incrementCount();
94122
}
95123
}
124+
this.cacheValid = false;
96125
}
97126

98127
// Mark a chain and its descendants as modified
@@ -101,52 +130,47 @@ const noRepeatedMemberAccess = createRule({
101130

102131
// Navigate to the target node, creating nodes if they don't exist
103132
for (const prop of properties) {
104-
const child = current.children.get(prop);
105-
if (child) {
106-
current = child;
107-
} else {
108-
// Create the chain if it doesn't exist
109-
const newChild = current.getOrCreateChild(prop);
110-
newChild.parent = current;
111-
current = newChild;
112-
}
133+
const newChild = current.getOrCreateChild(prop);
134+
newChild.setParent = current;
135+
current = newChild;
113136
}
114137

115138
// Mark this node and all descendants as modified
116139
current.markAsModified();
140+
this.cacheValid = false;
117141
}
118142

119-
// Find the longest valid chain that meets the minimum occurrence threshold
120-
findLongestValidChain(
121-
minOccurrences: number
122-
): { chain: string; count: number } | null {
123-
let bestChain: string | null = null;
124-
let bestCount = 0;
143+
// Find any valid chain that meets the minimum occurrence threshold
144+
findValidChains() {
145+
if (this.cacheValid) {
146+
return this.validChainsCache;
147+
}
148+
const validChains: Array<{ chain: string }> = [];
125149

126150
const traverse = (node: ChainNode, depth: number) => {
127151
// Only consider chains with more than one segment (has dots)
128-
if (depth > 1 && !node.modified && node.count >= minOccurrences) {
152+
if (depth > 1 && !node.isModified && node.getCount >= 2) {
129153
const chainPath = node.getChainPath();
130-
if (chainPath.length > (bestChain?.length || 0)) {
131-
bestChain = chainPath;
132-
bestCount = node.count;
133-
}
154+
validChains.push({
155+
chain: chainPath,
156+
});
134157
}
135158

136159
// Stop traversing if this node is modified
137-
if (node.modified) {
160+
if (node.isModified) {
138161
return;
139162
}
140163

141164
// Recursively traverse children
142-
for (const child of node.children.values()) {
165+
for (const child of node.getChildren.values()) {
143166
traverse(child, depth + 1);
144167
}
145168
};
146169

147170
traverse(this.root, 0);
148-
149-
return bestChain ? { chain: bestChain, count: bestCount } : null;
171+
this.cacheValid = true;
172+
this.validChainsCache = validChains;
173+
return validChains;
150174
}
151175
}
152176

@@ -224,15 +248,17 @@ const noRepeatedMemberAccess = createRule({
224248
// Insert the chain into the tree (this will increment counts automatically)
225249
chainTree.insertChain(properties);
226250

227-
// Find the longest valid chain to report
228-
const result = chainTree.findLongestValidChain(minOccurrences);
229-
if (result && !reportedChains.has(result.chain)) {
230-
context.report({
231-
node: node,
232-
messageId: "repeatedAccess",
233-
data: { chain: result.chain, count: result.count },
234-
});
235-
reportedChains.add(result.chain);
251+
// Find all valid chains to report
252+
const validChains = chainTree.findValidChains();
253+
for (const result of validChains) {
254+
if (!reportedChains.has(result.chain)) {
255+
context.report({
256+
node: node,
257+
messageId: "repeatedAccess",
258+
data: { chain: result.chain },
259+
});
260+
reportedChains.add(result.chain);
261+
}
236262
}
237263
}
238264

tests/rules/noRepeatedMemberAccess.test.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,25 @@ describe("Rule: no-spread", () => {
115115
const v2 = a.b.c;
116116
const v3 = a.b.c;
117117
`,
118-
errors: [{ messageId: "repeatedAccess" }],
118+
errors: [
119+
{ messageId: "repeatedAccess" },
120+
{ messageId: "repeatedAccess" },
121+
],
122+
},
123+
{
124+
code: `
125+
const data = a.b.c.d;
126+
const data = a.b.c.d;
127+
const data = a.b.c.d;
128+
const data = a.b.c;
129+
const data = a.b.c;
130+
131+
`,
132+
errors: [
133+
{ messageId: "repeatedAccess" },
134+
{ messageId: "repeatedAccess" },
135+
{ messageId: "repeatedAccess" },
136+
],
119137
},
120138
{
121139
code: `
@@ -137,7 +155,10 @@ describe("Rule: no-spread", () => {
137155
return obj.a.b.d;
138156
}
139157
`,
140-
errors: [{ messageId: "repeatedAccess" }],
158+
errors: [
159+
{ messageId: "repeatedAccess" },
160+
{ messageId: "repeatedAccess" },
161+
],
141162
},
142163
{
143164
code: `

0 commit comments

Comments
 (0)