Unconditional Element Replace
ID |
javascript.no_unconditional_element_replace |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
JavaScript |
Tags |
collection, dead-code, reliability |
Description
Reports two statements in a row that store to the same element of the same container with nothing in between: cache[key] = a; cache[key] = b; or byId.set(id, a); byId.set(id, b);.
Two store shapes are recognised — an element assignment container[key] = value on a container named by a plain identifier, and a two-argument set on a receiver the rule can see was built with new Map() or new WeakMap(). The set case needs that declaration: set is a name anything can carry, and on many of the things that carry it — an immutable collection, a fluent builder, a signal — set returns a new value instead of replacing one in place, so the earlier call is not dead.
The two stores have to be immediately consecutive statements. Keys are compared by spelling, never by value: a literal against the same literal, an identifier against the same identifier. A computed key (a[i + 1] = x) is not matched at all, a compound assignment is not a store, and neither is a second store whose value names the container.
Rationale
// Bad
limits[tier] = base;
limits[tier] = base * factor;-
The first value is discarded before anyone can read it. At best the line is redundant; far more often it is a copy-paste slip that wrote the wrong key on one of the two lines, and the intended entry was never set at all.
-
Because the two lines look almost identical, the mistake reads as deliberate — a default followed by an override — which is exactly why it survives review. Nothing in the code marks which of the two keys is wrong.
-
Adjacency is what makes the earlier value provably dead: nothing ran in between that could read the element, hand the whole container to something that reads it, or replace the container outright.
That last point is the reason the rule is narrow. A version that allowed statements between the two stores, and looked only for an exact read of that one key, produced hundreds of findings and no true positive on a real corpus — nearly all of them tests that filled a map, passed it to the code under test, and refilled it for the next case. Giving up the pair separated by an unrelated statement is what buys the precision.
Remediation
Decide which value belongs at that key and write it once — or fix the key that was meant to be different.
// Good — one store, the intended value
limits[tier] = base * factor;
// Good — two keys, which is what the two lines were for
limits[tier] = base;
limits[premiumTier] = base * factor;When the value really is built in two steps, build it in a local and store once; that also makes it obvious that only one entry is written.
// Good
let limit = base;
if (isPremium(tier)) {
limit *= factor;
}
limits[tier] = limit;