Skip to content

Commit 4bb738f

Browse files
authored
eslint-factory: resolve destructured bindings precisely in resolveInitializer (#53960)
1 parent 3afdd51 commit 4bb738f

4 files changed

Lines changed: 149 additions & 1 deletion

File tree

eslint-factory/src/rules/command-initializer-utils.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,95 @@
11
import { AST_NODE_TYPES, TSESLint, TSESTree } from "@typescript-eslint/utils";
22

3+
/**
4+
* Resolves the element of an array literal bound to `name` by an array
5+
* destructuring pattern (for example `const [cmd] = [command]`). Returns null
6+
* when the binding cannot be resolved precisely — a non-literal right-hand
7+
* side, a rest element before the binding, a spread element in the array
8+
* literal, a hole, or a default value.
9+
*/
10+
function resolveArrayPatternElement(pattern: TSESTree.ArrayPattern, init: TSESTree.Expression, name: string): TSESTree.Expression | null {
11+
if (init.type !== AST_NODE_TYPES.ArrayExpression) return null;
12+
for (let index = 0; index < pattern.elements.length; index++) {
13+
const element = pattern.elements[index];
14+
// A rest element consumes the remaining values, so positions after it no
15+
// longer line up with the array literal.
16+
if (element !== null && element.type === AST_NODE_TYPES.RestElement) return null;
17+
if (element === null || element.type !== AST_NODE_TYPES.Identifier || element.name !== name) continue;
18+
// A spread element at or before this position shifts the value positions.
19+
if (init.elements.slice(0, index + 1).some(value => value !== null && value.type === AST_NODE_TYPES.SpreadElement)) return null;
20+
const value = init.elements[index];
21+
if (value === null || value.type === AST_NODE_TYPES.SpreadElement) return null;
22+
return value;
23+
}
24+
return null;
25+
}
26+
27+
/**
28+
* Returns the static property name of a non-computed property key, or null
29+
* when the key is not statically known.
30+
*/
31+
function getStaticPropertyName(key: TSESTree.Node): string | null {
32+
if (key.type === AST_NODE_TYPES.Identifier) return key.name;
33+
if (key.type === AST_NODE_TYPES.Literal && (typeof key.value === "string" || typeof key.value === "number")) return String(key.value);
34+
return null;
35+
}
36+
37+
/**
38+
* Narrows a property value to a plain expression, rejecting binding patterns
39+
* and other non-expression property values.
40+
*/
41+
function asExpression(value: TSESTree.Property["value"]): TSESTree.Expression | null {
42+
switch (value.type) {
43+
case AST_NODE_TYPES.ArrayPattern:
44+
case AST_NODE_TYPES.AssignmentPattern:
45+
case AST_NODE_TYPES.ObjectPattern:
46+
case AST_NODE_TYPES.TSEmptyBodyFunctionExpression:
47+
return null;
48+
default:
49+
return value;
50+
}
51+
}
52+
53+
/**
54+
* Resolves the property value of an object literal bound to `name` by an
55+
* object destructuring pattern (for example `const { cmd } = { cmd: command }`).
56+
* Returns null when the binding cannot be resolved precisely — a non-literal
57+
* right-hand side, a spread element, a computed or accessor property, or a
58+
* default value.
59+
*/
60+
function resolveObjectPatternProperty(pattern: TSESTree.ObjectPattern, init: TSESTree.Expression, name: string): TSESTree.Expression | null {
61+
if (init.type !== AST_NODE_TYPES.ObjectExpression) return null;
62+
// A spread can override any property, so the literal is no longer authoritative.
63+
if (init.properties.some(property => property.type === AST_NODE_TYPES.SpreadElement)) return null;
64+
65+
let key: string | null = null;
66+
for (const property of pattern.properties) {
67+
if (property.type !== AST_NODE_TYPES.Property || property.computed) continue;
68+
if (property.value.type !== AST_NODE_TYPES.Identifier || property.value.name !== name) continue;
69+
key = getStaticPropertyName(property.key);
70+
break;
71+
}
72+
if (key === null) return null;
73+
74+
let resolved: TSESTree.Expression | null = null;
75+
for (const property of init.properties) {
76+
if (property.type !== AST_NODE_TYPES.Property || property.computed || property.kind !== "init") continue;
77+
if (getStaticPropertyName(property.key) !== key) continue;
78+
// Later properties win over earlier duplicates.
79+
resolved = asExpression(property.value);
80+
}
81+
return resolved;
82+
}
83+
384
/**
485
* When `identifier` is a write-once local variable binding, returns its
586
* initializer expression so the caller can apply further checks. Returns null
687
* for parameters, imports, multiply-assigned vars, and vars with no
788
* initializer.
89+
*
90+
* Destructured bindings (for example `const [cmd] = [command]`) resolve to the
91+
* specific destructured value when it can be determined precisely, and to null
92+
* otherwise — never to the whole right-hand side expression.
893
*/
994
function resolveInitializer(identifier: TSESTree.Identifier, sourceCode: TSESLint.SourceCode): TSESTree.Expression | null {
1095
const startScope = sourceCode.getScope(identifier);
@@ -28,7 +113,11 @@ function resolveInitializer(identifier: TSESTree.Identifier, sourceCode: TSESLin
28113
// Reject re-assigned bindings (write references that are not the initializer).
29114
if (variable.references.some(ref => ref.isWrite() && !ref.init)) return null;
30115
const declarator = def.node as TSESTree.VariableDeclarator;
31-
return declarator.init ?? null;
116+
if (declarator.init === null || declarator.init === undefined) return null;
117+
if (declarator.id.type === AST_NODE_TYPES.Identifier) return declarator.init;
118+
if (declarator.id.type === AST_NODE_TYPES.ArrayPattern) return resolveArrayPatternElement(declarator.id, declarator.init, identifier.name);
119+
if (declarator.id.type === AST_NODE_TYPES.ObjectPattern) return resolveObjectPatternProperty(declarator.id, declarator.init, identifier.name);
120+
return null;
32121
}
33122
scope = scope.upper;
34123
}

eslint-factory/src/rules/no-child-process-interpolated-command.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ describe("no-child-process-interpolated-command", () => {
4141
{ code: `const { execSync } = require("child_process"); execSync("git-status".replace("-", () => " "));` },
4242
// Un-reassigned options object without shell — safe, must not over-flag
4343
{ code: `const { spawnSync } = require("child_process"); const cmd = \`git checkout \${branch}\`; const opts = {}; spawnSync(cmd, [], opts);` },
44+
// Statically destructured command — safe, must not be flagged
45+
{ code: `const { execSync } = require("child_process"); const [cmd] = ["git status"]; execSync(cmd);` },
46+
// Destructured from an unresolvable right-hand side — must not be flagged
47+
{ code: `const { execSync } = require("child_process"); function run(parts) { const [cmd] = parts; execSync(cmd); }` },
4448
],
4549
invalid: [
4650
{
@@ -141,6 +145,16 @@ describe("no-child-process-interpolated-command", () => {
141145
code: `require("child_process").spawn(\`git checkout \${branch}\`, { shell: true });`,
142146
errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "spawn" } }],
143147
},
148+
// Array-destructured dynamic command must resolve to the destructured element
149+
{
150+
code: `const { execSync } = require("child_process"); const [cmd] = [\`git checkout \${branch}\`]; execSync(cmd);`,
151+
errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }],
152+
},
153+
// Object-destructured dynamic command must resolve to the destructured property
154+
{
155+
code: `const { execSync } = require("child_process"); const { cmd } = { cmd: \`git checkout \${branch}\` }; execSync(cmd);`,
156+
errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "execSync" } }],
157+
},
144158
],
145159
});
146160
});

eslint-factory/src/rules/no-exec-interpolated-command.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ describe("no-exec-interpolated-command", () => {
5959
{ code: `exec.exec("git-checkout".replace("-", () => " "), [branch]);` },
6060
// A write-once interpolation resolving to a literal is safe
6161
{ code: `function run() { const branch = "main"; const ref = branch; exec.exec(\`git checkout \${ref}\`, []); }` },
62+
// Statically destructured command — safe, must not be flagged
63+
{ code: `function run() { const [cmd] = ["git status"]; exec.exec(cmd, []); }` },
64+
// Destructured from an unresolvable right-hand side — must not be flagged
65+
{ code: `function run(parts) { const [cmd] = parts; exec.exec(cmd, []); }` },
6266
// A digits-only sanitized interpolation is safe
6367
{ code: `function run(port) { const safePort = String(port).replace(/[^0-9]/g, ""); exec.exec(\`netstat | grep :\${safePort}\`, []); }` },
6468
],
@@ -153,6 +157,16 @@ describe("no-exec-interpolated-command", () => {
153157
code: "function run(branch) { const dynamic = `git checkout ${branch}`; const cmd = dynamic; exec.exec(cmd, []); }",
154158
errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }],
155159
},
160+
// Array-destructured dynamic command must resolve to the destructured element
161+
{
162+
code: "function run(branch) { const [cmd] = [`git checkout ${branch}`]; exec.exec(cmd, []); }",
163+
errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }],
164+
},
165+
// Object-destructured dynamic command must resolve to the destructured property
166+
{
167+
code: "function run(branch) { const { cmd } = { cmd: `git checkout ${branch}` }; exec.exec(cmd, []); }",
168+
errors: [{ messageId: "interpolatedCommand", data: { kind: "interpolated template literal", method: "exec" } }],
169+
},
156170
// execApi parameter-alias with array-shaped args — flagged (matches git_helpers.cjs / create_pull_request.cjs convention)
157171
{
158172
code: "function run(execApi, branch) { execApi.exec(`git checkout ${branch}`, []); }",

eslint-factory/src/rules/no-github-request-interpolated-route.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,4 +389,35 @@ describe("no-github-request-interpolated-route", () => {
389389
invalid: [],
390390
});
391391
});
392+
393+
it("destructured route bindings resolve to the destructured value", () => {
394+
cjsRuleTester.run("no-github-request-interpolated-route", noGithubRequestInterpolatedRouteRule, {
395+
valid: [
396+
// Statically destructured route — no interpolation, must not be flagged
397+
`function f() { const [route] = ["GET /repos/{owner}/{repo}"]; github.request(route, {}); }`,
398+
// Destructured from a non-literal right-hand side — unresolvable, must not be flagged
399+
"function f(routes) { const [route] = routes; github.request(route, {}); }",
400+
],
401+
invalid: [
402+
{
403+
code: "function f(owner, repo) { const [route] = [`GET /repos/${owner}/${repo}`]; github.request(route, {}); }",
404+
errors: [
405+
{
406+
messageId: "interpolatedRoute",
407+
data: { kind: "template literal with interpolations", client: "github" },
408+
},
409+
],
410+
},
411+
{
412+
code: "function f(owner, repo) { const { route } = { route: `GET /repos/${owner}/${repo}` }; github.request(route, {}); }",
413+
errors: [
414+
{
415+
messageId: "interpolatedRoute",
416+
data: { kind: "template literal with interpolations", client: "github" },
417+
},
418+
],
419+
},
420+
],
421+
});
422+
});
392423
});

0 commit comments

Comments
 (0)