Unconditional Element Replace
ID |
php.no_unconditional_element_replace |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Php |
Tags |
collection, dead-code, reliability |
Description
Two consecutive statements store to the same array element, so the first value is overwritten before anything can read it.
Nothing runs between the two writes that could read the element, pass the array to something that reads it, or replace the array outright. The first store is therefore dead — usually because a copy-paste left the wrong key, or the wrong line, behind.
Rationale
$limits[$tier] = $base;
$limits[$tier] = $base * $factor; // the plain $base never reached anyone
$matrix['eu']['read'] = 100;
$matrix['eu']['read'] = 200; // same element, same statement list
Written out, the intent is almost always one of two things: the second line was meant to use a different key, or the first line was left over from an earlier edit. Either way the code as written computes a value and throws it away.
Remediation
Keep the store that matters, or correct the key that was mistyped.
$limits[$tier] = $base * $factor; // one store
$matrix['eu']['read'] = 100;
$matrix['eu']['write'] = 200; // the key that was meant
Reading the previous value makes the first store live, and is never reported:
$counts[$key] = $counts[$key] + 1; // builds on what is there
$totals[$key] += 1; // a compound assignment reads before it writes
$queue[] = $item; // an append has no element to replace
The two stores must be immediately consecutive statements of one statement list; a statement in
between means the earlier value may well have been read. The container must be a plain variable and
every key a literal or a plain variable, so that the two stores can be compared as written. A store
through a property chain such as $this→cache[$key], or under a computed key such as
$a[$i + 1], is outside this rule.