Meaningless Size Comparison
ID |
php.meaningless_size_comparison |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Php |
Tags |
reliability, suspicious-comparison |
Description
A size is never negative, so comparing one against 0 or -1 with a relational operator produces
a condition whose answer is fixed at compile time.
count(), sizeof(), strlen(), mb_strlen(), iterator_count() and a Countable object’s
count() all report a number of elements or bytes. >= 0 and > -1 are therefore always true,
and < 0 and ⇐ -1 are always false. The guard either protects nothing or, far more often, was
meant to be an emptiness check and now lets every input through.
Rationale
public function render(array $rows): string
{
if (count($rows) >= 0) { // always true: every array reaches this branch
return $this->table($rows);
}
return $this->placeholder(); // dead code
}
if (strlen($name) < 0) { // never true
throw new \InvalidArgumentException('empty name');
}The first guard renders the table for an empty array too, which is exactly what the author was trying to avoid. The second validation never fires, so an empty name flows straight through.
Remediation
Compare against the bound the value can actually cross, or test emptiness directly.
public function render(array $rows): string
{
if (count($rows) > 0) { // the emptiness check that was meant
return $this->table($rows);
}
return $this->placeholder();
}
if ($name === '') { // the validation that was meant
throw new \InvalidArgumentException('empty name');
}Both operand orders are reported, so 0 ⇐ count($rows) is flagged as well. Bounds other than 0
and -1 state a real condition and are left alone, and so are the equality shapes === 0 and
!== 0, which are genuine emptiness tests. An operand that merely contains a size call, such as
$offset + count($rows) >= 0, is not reported: the sum really can go negative.