Loop Var Never Changed

ID

csharp.loop_var_never_changed

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

control-flow, loop, reliability

Description

Reports a for loop whose update clause advances something other than the counter its condition tests, and where nothing else advances that counter either. for (int i = 0; i < 10; sum++) never moves i towards the bound.

Rationale

The three clauses of a for header have to agree, and this is the shape where they do not. The counter is declared, the condition tests it, and the update clause moves a different variable — usually because a loop header was copy-pasted and only two of the three names were adjusted. The result compiles cleanly and, unless the body breaks out for another reason, spins forever.

The counter examined is the single variable the loop header declares, so the analysis is anchored on a name the loop itself owns. A loop is only reported when that counter is written nowhere else: not in the update clause, not in the condition, and not in the body. Advancing the counter from inside the body is untidy but legal, so it silences the rule — as does handing the counter to a method by ref or out.

public int Sum(int[] values)
{
    int total = 0;
    for (int i = 0; i < values.Length; total++)  // FLAW — i never advances
    {
        total += values[i];
    }
    return total;
}

public void Advanced(int limit)
{
    for (int i = 0; i < limit; i++)              // OK — the counter advances
    {
        Log(i);
    }
}

public void AdvancedInBody(int limit, int other)
{
    for (int i = 0; i < limit; other++)          // OK — the body advances the counter
    {
        i += 2;
    }
}

public void Endless()
{
    for (;;)                                     // OK — the idiomatic endless loop
    {
        if (Done()) break;
    }
}

Remediation

Put the counter back in the update clause — for (int i = 0; i < values.Length; i++) — and move the other variable’s update into the body, where it belongs. When the collection is walked from start to end and the index is only used to fetch the element, foreach removes the counter and the whole class of mistake with it.

References