Useless Increment
ID |
javascript.no_useless_increment |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
JavaScript |
Tags |
dead-code, reliability, suspicious-assignment |
Description
Reports a postfix ++ / -- whose effect is thrown away, in the two shapes where the mistake is unambiguous:
-
return i;` — and the arrow equivalent `const bump = (i) => i;. The postfix form yields the value from before the update, and the binding it just changed dies with the call, so the update can never be observed. -
i = i++;— the right-hand side evaluates to the old value ofi, which is then written straight back over the incremented one.
The returned-value shape fires only when the operand resolves to a binding declared in the same function — a local or a parameter. When the operand cannot be resolved to a declaration, nothing is reported.
Rationale
// Bad
function nextIndex(i) {
return i++; // returns i, unchanged; the increment dies with the call
}
function reset(n) {
let i = n;
i = i++; // reads i, increments it, writes the old value back
return i;
}-
i++is an expression whose value is the operand before the update. Written as the whole returned expression it reads like "return the next index" and does the opposite: the caller gets the old value, and the new one is discarded with the frame. -
i = i++is worse than a no-op to read: the increment is right there in the source, so a reviewer sees the intent and not the fact that the assignment undoes it. -
Both shapes survive review because they look like the working idiom.
return i` and `i;differ from them by one character and do what the author meant.
The exclusions are the cases where the update is genuinely kept. A module-level counter keeps it — let seq = 0; function nextId() { return seq; }` is the canonical id generator. So does a captured local: in `function make() { let i = 0; return () => i; } the binding lives in the enclosing function, which is still there after the inner call returns. A value that is consumed (return i + 1`, `use(i)) is not discarded either, and a member target (this.n`, `a[k]) is left alone rather than guessing whether the object outlives the call.
Remediation
Say which value you want. If it is the updated one, use the prefix form or plain arithmetic; if you want the side effect, make it its own statement.
// Good
function nextIndex(i) {
return i + 1; // no mutation at all, which is what a pure function wants
}
function reset(n) {
let i = n;
i++; // the update stands on its own
return i;
}return i + 1 is usually the better of the two fixes: it makes the function pure, and a parameter that is only read is easier to reason about than one that is mutated and then thrown away.