Skip to content

Feat: Add no-member-access rule #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Jul 3, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2a05148
initial work on no-repeated-member-access rule
meowbmw May 23, 2025
00f0549
second attempt; still trying to figure this out
meowbmw May 26, 2025
434d4e8
Add no repeated member access rule
meowbmw Jun 3, 2025
3fac4ff
Add more documentation on how this rule works
meowbmw Jun 5, 2025
9b6f232
Merge branch 'main' into main
xpirad Jun 6, 2025
7667f9d
change default minOcurrences to 3; small code clean up
meowbmw Jun 6, 2025
21732a1
clean up test case
meowbmw Jun 6, 2025
6221add
enable debug support
meowbmw Jun 13, 2025
4f44e12
add barebone rewrite
meowbmw Jun 13, 2025
defaea0
still wip;
xpirad Jun 15, 2025
500b0ca
wip 2: refactor old version
meowbmw Jun 16, 2025
edecf9f
wip 3; near completion??
xpirad Jun 16, 2025
00709fb
ready for review
meowbmw Jun 17, 2025
a84bc60
clearer naming
meowbmw Jun 17, 2025
af3a92e
clean up comment
meowbmw Jun 17, 2025
b6e7556
clean up draft file
xpirad Jun 17, 2025
dd0221a
remove files to support ts debug; add force curly after if; minor adj…
meowbmw Jun 30, 2025
2c569fa
refactor memberaccess using tree structures
meowbmw Jun 30, 2025
7cfe59d
change test case
meowbmw Jun 30, 2025
8608146
fix logic in mark modified
meowbmw Jun 30, 2025
802b6d4
set class members to private & add chains cache
meowbmw Jun 30, 2025
337bb8d
set modified in constructor to ensure the flag is inhereited; constru…
meowbmw Jun 30, 2025
f9382c6
clean up class member & rewrite path generate logic & add visitor fun…
meowbmw Jul 1, 2025
c105abb
Update readme
meowbmw Jul 1, 2025
857cca2
Merge branch 'main' into main
xpirad Jul 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 0 additions & 33 deletions .vscode/launch.json

This file was deleted.

3 changes: 0 additions & 3 deletions .vscode/settings.json

This file was deleted.

13 changes: 8 additions & 5 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
},
}
);
13 changes: 0 additions & 13 deletions loader.mjs

This file was deleted.

250 changes: 147 additions & 103 deletions plugins/rules/memberAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,37 +33,135 @@ const noRepeatedMemberAccess = createRule({
// Track which chains have already been reported to avoid duplicate reports
const reportedChains = new Set<string>();

// 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<string, ChainNode> = 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<Scope.Scope, ChainMap>();

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++;
}
>();
scopeDataMap.set(scope, newChainMap);
}
}

// 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 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;
}
}

// 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);
}
};

traverse(this.root, 0);

return bestChain ? { chain: bestChain, count: bestCount } : null;
}
}

// Stores mapping of scope to ChainTree
const scopeDataMap = new WeakMap<Scope.Scope, ChainTree>();

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
Expand Down Expand Up @@ -96,93 +194,45 @@ 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) {
// 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;
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);
}
}

Expand All @@ -192,32 +242,26 @@ 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);
}
},

// 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);
for (const chain of chainInfo) {
setModifiedFlag(chain, node);
}
const properties = analyzeChain(node.argument);
setModifiedFlag(properties, 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);
for (const chain of chainInfo) {
setModifiedFlag(chain, node);
}
const properties = analyzeChain(node.callee);
setModifiedFlag(properties, node);
}
},

Expand Down
4 changes: 3 additions & 1 deletion plugins/rules/noConcatString.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading