Meaningless Size Comparison
ID |
javascript.meaningless_size_comparison |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
JavaScript |
Tags |
reliability, suspicious-comparison |
Description
Reports a size or length compared against a bound the value can never cross, which makes the
comparison a constant. length on a string or an array, size on a Map or a Set, and a
no-argument size() accessor on a collection wrapper all report how many elements there are and
none of them can be negative, so >= 0 and > -1 always hold, while < 0 and ⇐ -1 never do.
Rationale
A comparison that cannot change its answer is either a guard that guards nothing or — far more
often — an emptiness check that was written with the wrong bound. if (items.length >= 0) reads
like "if there is anything in the list" and behaves like "always", so the code inside runs for the
empty list too and the bug surfaces later as an undefined property read or an empty report. The
mirrored form if (0 > items.length) has the opposite failure: the branch is dead, and whatever it
was supposed to handle is silently never handled.
class Cart {
constructor() {
this.items = [];
this.tags = new Set();
}
report(data, name) {
if (this.items.length >= 0) { // FLAW — always true, the guard does nothing
console.log(this.items[0].sku);
}
if (data.length < 0) { // FLAW — always false, this branch is dead
console.log("no data");
}
if (this.tags.size < 0) { // FLAW — a Set size is never negative either
console.log("no tags");
}
if (name.length <= -1) { // FLAW — always false
console.log("empty name");
}
if (this.items.length > 0) { // OK — genuine emptiness check
console.log(this.items[0].sku);
}
if (data.length >= 2) { // OK — a real bound
console.log(data[1]);
}
}
}Both operand orders are recognised. Only the bounds 0 and -1 are considered — every other bound
expresses a real condition.
Matching is by member name on a receiver, so a plain identifier named length is not treated as a
size and size() must be called on something. No other member name is accepted: length and
size are the only two whose non-negativity holds without knowing the receiver’s type.
Remediation
Decide what the test was meant to say and write that bound. "Has elements" is length > 0 (or
length !== 0); "is empty" is length === 0. If the comparison was defensive and the value
genuinely cannot be negative, delete it — it adds no protection and draws the reader’s eye away
from the checks that do:
if (this.items.length > 0) {
console.log(this.items[0].sku);
}
if (data.length === 0) {
console.log("no data");
}