Skip to content

Commit b75d76b

Browse files
authored
refactor(utils): extract and harden identifier- and comment-parsing logic into utils (#1)
- Move INCLUDED_HOOKS, findDisableComment, extractMentionedDeps, extractHookDeps, and extractUsedIdentifiers into utils.js - Preserve original staticEval guard and only add minimal ref.resolved and defs null checks in extractUsedIdentifiers - Simplify rule index to import from utils.js and emit a single concise missingDeps message - Ensure both CLI and API runners load identical, robust logic without TypeError on unresolved refs
1 parent 76885f6 commit b75d76b

File tree

11 files changed

+423
-216
lines changed

11 files changed

+423
-216
lines changed

.vscode/launch.json

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
// Use IntelliSense to learn about possible attributes.
3+
// Hover to view descriptions of existing attributes.
4+
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
5+
"version": "0.2.0",
6+
"configurations": [
7+
{
8+
"type": "node",
9+
"request": "launch",
10+
"name": "Launch Program",
11+
"skipFiles": [
12+
"<node_internals>/**"
13+
],
14+
"program": "${workspaceFolder}/index.js"
15+
}
16+
]
17+
}

index.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
console.log('PLUGIN LOADED')
2-
31
module.exports = {
42
rules: {
5-
"require-exhaustive-deps-comment": require("./lib/rules/require-exhaustive-deps-comment")
3+
"explicit-exhaustive-deps-disable": require("./lib/rules/explicit-exhaustive-deps-disable")
64
}
75
};
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
"use strict";
2+
3+
const {
4+
INCLUDED_HOOKS,
5+
findDisableComment,
6+
extractMentionedDeps,
7+
extractHookDeps,
8+
extractUsedIdentifiers,
9+
} = require('./utils');
10+
11+
/** @type {import('eslint').Rule.RuleModule} */
12+
module.exports = {
13+
meta: {
14+
type: 'problem',
15+
docs: {
16+
description: 'Require intentional comment when disabling react-hooks/exhaustive-deps',
17+
category: 'Best Practices',
18+
recommended: false,
19+
ruleId: 'explicit-exhaustive-deps-disable'
20+
},
21+
messages: {
22+
missingDeps: 'You\'ve disabled exhaustive-deps, but you need to document why \'{{deps}}\' {{verb}} safe to omit.',
23+
},
24+
schema: [],
25+
},
26+
27+
/**
28+
* @param {import('eslint').Rule.RuleContext} context
29+
* @returns {import('eslint').Rule.RuleListener}
30+
*/
31+
create(context) {
32+
const sourceCode = context.sourceCode;
33+
34+
return {
35+
CallExpression(node) {
36+
const name = node.callee.name;
37+
38+
if (!INCLUDED_HOOKS.includes(name)) return;
39+
40+
const disabledComment = findDisableComment(node, sourceCode);
41+
42+
if (!disabledComment) return;
43+
44+
const hookDeps = extractHookDeps(node, sourceCode);
45+
const usedIdentifiers = extractUsedIdentifiers(node, sourceCode);
46+
const mentioned = extractMentionedDeps(disabledComment);
47+
48+
const missing = usedIdentifiers.filter(d => !hookDeps.includes(d) && !mentioned.includes(d));
49+
50+
if (missing.length === 0) return;
51+
52+
context.report({
53+
node,
54+
messageId: 'missingDeps',
55+
data: {
56+
deps: missing.join(', '),
57+
verb: missing.length > 1 ? 'are' : 'is',
58+
},
59+
});
60+
}
61+
};
62+
}
63+
};
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
"use strict";
2+
3+
const { getStaticValue } = require("eslint-utils");
4+
5+
const INCLUDED_HOOKS = [
6+
'useEffect',
7+
'useCallback',
8+
'useMemo',
9+
'useLayoutEffect',
10+
'useInsertionEffect',
11+
];
12+
13+
/**
14+
* Find an inline disable‐line comment for react-hooks/exhaustive-deps
15+
* attached to a hook call.
16+
*
17+
* @param {ESTree.CallExpression} node
18+
* @param {import('eslint').SourceCode} sourceCode
19+
* @returns {import('eslint').Token|null} the comment node, or null if none
20+
*/
21+
function findDisableComment(node, sourceCode) {
22+
// Walk up until we hit exactly an ExpressionStatement or VariableDeclaration
23+
let stmt = node;
24+
while (
25+
stmt &&
26+
stmt.type !== "ExpressionStatement" &&
27+
stmt.type !== "VariableDeclaration"
28+
) {
29+
stmt = stmt.parent;
30+
}
31+
if (!stmt) return null;
32+
33+
// Grab trailing comments on that statement
34+
const comments = sourceCode.getCommentsAfter(stmt) || [];
35+
return (
36+
comments.find(c =>
37+
c.type === "Line" &&
38+
c.loc.start.line === stmt.loc.end.line &&
39+
/eslint-disable-line\s+react-hooks\/exhaustive-deps/.test(c.value)
40+
) || null
41+
);
42+
}
43+
44+
/**
45+
* Extracts dependency names mentioned after the first "--" in a disable comment.
46+
* Supports multiple "--" in the payload by rejoining after the first marker.
47+
*
48+
* @param {string} commentText - The raw comment value
49+
* @returns {string[]} Array of dependency names (e.g. ["foo", "bar"])
50+
*/
51+
function extractMentionedDeps(comment) {
52+
const commentText = comment.value;
53+
// Split on the first "--" marker, keep everything after it
54+
const parts = commentText.split(/--/);
55+
if (parts.length < 2) return []; // no "--" at all
56+
57+
// Rejoin anything after the first "--" in case reasons contain "--"
58+
const payload = parts.slice(1).join("--").trim();
59+
if (!payload) return []; // "--" present but nothing after it
60+
61+
// Extract all valid identifier tokens (including nested like user.id)
62+
const matches = payload.match(/[\w$.]+/g);
63+
return matches || [];
64+
}
65+
66+
/**
67+
* Given a CallExpression node for a hook,
68+
* return an array of everything listed in its deps array.
69+
*
70+
* Supports both simple identifiers and member expressions (e.g. user.id).
71+
*/
72+
function extractHookDeps(node, sourceCode) {
73+
const depsArg = node.arguments[1];
74+
if (!depsArg || depsArg.type !== "ArrayExpression") {
75+
return [];
76+
}
77+
78+
return depsArg.elements
79+
.filter(el => el && (el.type === "Identifier" || el.type === "MemberExpression"))
80+
.map(el => {
81+
if (el.type === "Identifier") {
82+
return el.name;
83+
} else {
84+
// for MemberExpression, just grab the source text (e.g. "user.id")
85+
return sourceCode.getText(el);
86+
}
87+
});
88+
}
89+
90+
/**
91+
* Collect every identifier reference in the hook callback’s closure,
92+
* skipping those that statically evaluate to a primitive.
93+
*
94+
* @param {ESTree.FunctionExpression|ESTree.ArrowFunctionExpression} callback
95+
* @param {import('eslint').SourceCode} sourceCode
96+
* @returns {string[]}
97+
*/
98+
function extractUsedIdentifiers(node, sourceCode) {
99+
const callback = node.arguments[0];
100+
const hookName = node.callee.name;
101+
102+
// Figure out if this call is stored in a variable:
103+
// e.g. const handleClick = useCallback(...)
104+
let varName;
105+
if (
106+
node.parent &&
107+
node.parent.type === "VariableDeclarator" &&
108+
node.parent.id.type === "Identifier"
109+
) {
110+
varName = node.parent.id.name;
111+
}
112+
113+
// ensure we have a block body
114+
if (!callback.body || callback.body.type !== "BlockStatement") {
115+
return [];
116+
}
117+
118+
const { start, end } = callback.body.loc;
119+
120+
// grab the scope for the callback
121+
const scope = sourceCode.scopeManager.acquire(callback);
122+
if (!scope) return [];
123+
124+
const deps = new Set();
125+
126+
// walk up through scopes but focus on "through" refs for the callback
127+
for (const ref of scope.through) {
128+
const idNode = ref.identifier;
129+
const name = idNode.name;
130+
const { line } = idNode.loc.start;
131+
132+
// only consider references whose definition *use* is inside the callback
133+
if (line < start.line || line > end.line) continue;
134+
135+
// skip the hook itself
136+
if (name === hookName) continue;
137+
// skip the variable the hook is assigned to
138+
if (name === varName) continue;
139+
140+
// skip globally‐resolved names (console, window, etc.)
141+
// ref.resolved will be undefined if it’s undeclared,
142+
// but if it’s a built‐in, resolved.scope.type === 'global'
143+
if (ref.resolved && ref.resolved.scope.type === 'global') {
144+
continue;
145+
}
146+
147+
// skip primitives that statically evaluate
148+
const staticEval = getStaticValue(idNode, sourceCode.getScope(idNode));
149+
if (
150+
staticEval &&
151+
(typeof staticEval.value === "string" ||
152+
typeof staticEval.value === "number" ||
153+
typeof staticEval.value === "boolean")
154+
) {
155+
continue;
156+
}
157+
158+
// skip any unresolved refs (avoid defs lookup on null)
159+
if (!ref.resolved) {
160+
continue;
161+
}
162+
163+
// generic member-expression logic
164+
if (
165+
idNode.parent.type === "MemberExpression" &&
166+
idNode.parent.object === idNode &&
167+
idNode.parent.property.type === "Identifier"
168+
) {
169+
const objName = name;
170+
const propName = idNode.parent.property.name;
171+
// safely grab defs array (may be empty)
172+
const defs = Array.isArray(ref.resolved.defs) ? ref.resolved.defs : [];
173+
const varDef = defs[0]?.node; // VariableDeclarator
174+
const init = varDef && varDef.init;
175+
176+
if (init && init.type === "ArrayExpression") {
177+
// any array method → just record the array variable
178+
deps.add(objName);
179+
} else if (init && init.type === "ObjectExpression") {
180+
// if this object literal defines that key, record full obj.prop
181+
const hasOwn = init.properties.some(prop => {
182+
return (
183+
prop.key.type === "Identifier" &&
184+
prop.key.name === propName
185+
);
186+
});
187+
deps.add(hasOwn ? `${objName}.${propName}` : objName);
188+
} else {
189+
// fallback: assume this is your own custom method
190+
deps.add(`${objName}.${propName}`);
191+
}
192+
} else {
193+
deps.add(name);
194+
}
195+
}
196+
197+
return Array.from(deps);
198+
}
199+
200+
module.exports = {
201+
INCLUDED_HOOKS,
202+
findDisableComment,
203+
extractMentionedDeps,
204+
extractHookDeps,
205+
extractUsedIdentifiers,
206+
};

0 commit comments

Comments
 (0)