Skip to content

Commit 161ffe1

Browse files
authored
fix(get-static-value): incorrect return when variable has object mutation (#257)
1 parent 692ce08 commit 161ffe1

File tree

2 files changed

+172
-22
lines changed

2 files changed

+172
-22
lines changed

src/get-static-value.mjs

Lines changed: 123 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { findVariable } from "./find-variable.mjs"
44
/** @typedef {import("./types.mjs").StaticValue} StaticValue */
55
/** @typedef {import("eslint").Scope.Scope} Scope */
6+
/** @typedef {import("eslint").Scope.Variable} Variable */
67
/** @typedef {import("estree").Node} Node */
78
/** @typedef {import("@typescript-eslint/types").TSESTree.Node} TSESTreeNode */
89
/** @typedef {import("@typescript-eslint/types").TSESTree.AST_NODE_TYPES} TSESTreeNodeTypes */
@@ -266,9 +267,40 @@ function getElementValues(nodeList, initialScope) {
266267
return valueList
267268
}
268269

270+
/**
271+
* Checks if a variable is a built-in global.
272+
* @param {Variable|null} variable The variable to check.
273+
* @returns {variable is Variable & {defs:[]}}
274+
*/
275+
function isBuiltinGlobal(variable) {
276+
return (
277+
variable != null &&
278+
variable.defs.length === 0 &&
279+
builtinNames.has(variable.name) &&
280+
variable.name in globalObject
281+
)
282+
}
283+
284+
/**
285+
* Checks if a variable can be considered as a constant.
286+
* @param {Variable} variable
287+
* @returns {variable is Variable & {defs: [import("eslint").Scope.Definition & { type: "Variable" }]}} True if the variable can be considered as a constant.
288+
*/
289+
function canBeConsideredConst(variable) {
290+
if (variable.defs.length !== 1) {
291+
return false
292+
}
293+
const def = variable.defs[0]
294+
return Boolean(
295+
def.parent &&
296+
def.type === "Variable" &&
297+
(def.parent.kind === "const" || isEffectivelyConst(variable)),
298+
)
299+
}
300+
269301
/**
270302
* Returns whether the given variable is never written to after initialization.
271-
* @param {import("eslint").Scope.Variable} variable
303+
* @param {Variable} variable
272304
* @returns {boolean}
273305
*/
274306
function isEffectivelyConst(variable) {
@@ -283,6 +315,68 @@ function isEffectivelyConst(variable) {
283315
return false
284316
}
285317

318+
/**
319+
* Checks if a variable has mutation in its property.
320+
* @param {Variable} variable The variable to check.
321+
* @param {Scope|null} initialScope The scope to start finding variable. Optional. If the node is a computed property node and this scope was given, this checks the computed property name by the `getStringIfConstant` function with the scope, and returns the value of it.
322+
* @returns {boolean} True if the variable has mutation in its property.
323+
*/
324+
function hasMutationInProperty(variable, initialScope) {
325+
for (const ref of variable.references) {
326+
let node = /** @type {TSESTreeNode} */ (ref.identifier)
327+
while (node && node.parent && node.parent.type === "MemberExpression") {
328+
node = node.parent
329+
}
330+
if (!node || !node.parent) {
331+
continue
332+
}
333+
if (
334+
(node.parent.type === "AssignmentExpression" &&
335+
node.parent.left === node) ||
336+
(node.parent.type === "UpdateExpression" &&
337+
node.parent.argument === node)
338+
) {
339+
// This is a mutation.
340+
return true
341+
}
342+
if (
343+
node.parent.type === "CallExpression" &&
344+
node.parent.callee === node &&
345+
node.type === "MemberExpression"
346+
) {
347+
const methodName = getStaticPropertyNameValue(node, initialScope)
348+
if (isNameOfMutationArrayMethod(methodName)) {
349+
// This is a mutation.
350+
return true
351+
}
352+
}
353+
}
354+
return false
355+
356+
/**
357+
* Checks if a method name is one of the mutation array methods.
358+
* @param {StaticValue|null} methodName The method name to check.
359+
* @returns {boolean} True if the method name is a mutation array method.
360+
*/
361+
function isNameOfMutationArrayMethod(methodName) {
362+
if (methodName == null || methodName.value == null) {
363+
return false
364+
}
365+
const name = methodName.value
366+
return (
367+
name === "copyWithin" ||
368+
name === "fill" ||
369+
name === "pop" ||
370+
name === "push" ||
371+
name === "reverse" ||
372+
name === "shift" ||
373+
name === "sort" ||
374+
name === "splice" ||
375+
name === "unshift"
376+
)
377+
}
378+
}
379+
286380
/**
287381
* @template {TSESTreeNodeTypes} T
288382
* @callback VisitorCallback
@@ -512,28 +606,35 @@ const operations = Object.freeze({
512606
if (initialScope != null) {
513607
const variable = findVariable(initialScope, node)
514608

515-
// Built-in globals.
516-
if (
517-
variable != null &&
518-
variable.defs.length === 0 &&
519-
builtinNames.has(variable.name) &&
520-
variable.name in globalObject
521-
) {
522-
return { value: globalObject[variable.name] }
523-
}
609+
if (variable != null) {
610+
// Built-in globals.
611+
if (isBuiltinGlobal(variable)) {
612+
return { value: globalObject[variable.name] }
613+
}
524614

525-
// Constants.
526-
if (variable != null && variable.defs.length === 1) {
527-
const def = variable.defs[0]
528-
if (
529-
def.parent &&
530-
def.type === "Variable" &&
531-
(def.parent.kind === "const" ||
532-
isEffectivelyConst(variable)) &&
533-
// TODO(mysticatea): don't support destructuring here.
534-
def.node.id.type === "Identifier"
535-
) {
536-
return getStaticValueR(def.node.init, initialScope)
615+
// Constants.
616+
if (canBeConsideredConst(variable)) {
617+
const def = variable.defs[0]
618+
if (
619+
// TODO(mysticatea): don't support destructuring here.
620+
def.node.id.type === "Identifier"
621+
) {
622+
const init = getStaticValueR(
623+
def.node.init,
624+
initialScope,
625+
)
626+
if (
627+
init &&
628+
typeof init.value === "object" &&
629+
init.value !== null
630+
) {
631+
if (hasMutationInProperty(variable, initialScope)) {
632+
// This variable has mutation in its property.
633+
return null
634+
}
635+
}
636+
return init
637+
}
537638
}
538639
}
539640
}

test/get-static-value.mjs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,55 @@ const aMap = Object.freeze({
398398
},
399399
]
400400
: []),
401+
// Mutations
402+
{
403+
code: "const a = {foo: 'a'}; a.bar = 'b'; a",
404+
expected: null,
405+
},
406+
{
407+
code: "const a = ['a']; a[0] = 'b'; a.join()",
408+
expected: null,
409+
},
410+
{
411+
code: "const a = ['a']; a.copyWithin(0, 1); a.join()",
412+
expected: null,
413+
},
414+
{
415+
code: "const a = ['a']; a.fill('b'); a.join()",
416+
expected: null,
417+
},
418+
{
419+
code: "const a = ['a']; a.pop(); a.join()",
420+
expected: null,
421+
},
422+
{
423+
code: "const a = ['a']; a.push('b'); a.join()",
424+
expected: null,
425+
},
426+
{
427+
code: "const a = ['a', 'b']; a.reverse(); a.join()",
428+
expected: null,
429+
},
430+
{
431+
code: "const a = ['a']; a.shift(); a.join()",
432+
expected: null,
433+
},
434+
{
435+
code: "const a = ['b', 'a', 'c']; a.sort(); a.join()",
436+
expected: null,
437+
},
438+
{
439+
code: "const a = ['a', 'c']; a.splice(1, 0, 'b'); a.join()",
440+
expected: null,
441+
},
442+
{
443+
code: "const a = ['a']; a.unshift('b'); a.join()",
444+
expected: null,
445+
},
446+
{
447+
code: "const a = {foo: ['a']}; a.foo.shift(); a",
448+
expected: null,
449+
},
401450
// TypeScript support
402451
{
403452
code: `const a = 42; a as number;`,

0 commit comments

Comments
 (0)