Loop with an empty body

ID

vbnet.correctness.empty_loop_body

Severity

low

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Suspicious Construct

Language

VB.NET

Description

Reports a loop whose body contains no statements. All the loop forms are covered - For, For Each, While, Do …​ Loop, Do While …​ Loop, Do Until …​ Loop and the post-test Do …​ Loop While/Loop Until forms - and a body holding only a comment counts as empty, because comments are not statements.

Rationale

A loop exists to run its body, so an empty one is either doing nothing at cost or doing nothing forever. Which of the two depends on the header, and both are defects. A counted loop - For Each row In rows with nothing inside - burns an enumeration of the whole collection and produces no result, which usually means the body was deleted during a refactor, or that the work was moved out and the loop left behind; it is also what remains when a statement intended to be inside the loop ends up after Next, so it runs once instead of once per item. A condition loop with an empty body is worse: nothing inside can change the condition, so While Not ready with an empty body either exits immediately or spins forever, pinning a processor core at 100% and starving the very thread that would have set the flag. That failure mode does not show up on a developer machine with spare cores and it is not reproducible in a unit test - it shows up as an unresponsive service under load.

The following code illustrates the pattern detected by this rule:

Public Sub CountRows(ByVal rows As List(Of String))
    Dim total As Integer = 0
    ' FLAGGED: Loop with an empty body
    For Each row In rows
    Next
    Console.WriteLine(total)
End Sub

Remediation

Establish which case this is. If the body was lost, restore the work; if a statement after Next or End While was meant to be inside the loop, move it in - and check whether the loop variable or accumulator is being used outside the loop, which is the usual symptom. If the loop is no longer needed, delete it rather than leaving an empty shell for the next reader to interpret. If the intent really is to wait, do not spin: block properly with Thread.Sleep, or better, wait on the primitive that signals the change - ManualResetEventSlim.Wait, SemaphoreSlim.Wait, Task.Delay in asynchronous code - so the thread yields instead of consuming a core. Where a very short spin is genuinely wanted, make it explicit with Thread.SpinWait inside the body, so the loop states what it is doing.

Configuration

This detector does not need any configuration.

References