Redundant Return or Exit statement at the end of a procedure

ID

vbnet.maintainability.omit_redundant_control_flow

Severity

low

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Control Flow

Language

VB.NET

Description

Reports a bare Return, Exit Sub, Exit Function or Exit Property written as the last statement of a procedure, where control would leave the procedure at that point anyway. Sub procedures, Function procedures and property Get/Set accessors are all covered. Only the genuinely redundant case is reported: an exit inside a condition, a loop, a Select Case, a Try, a Using, a SyncLock or a With block is doing real work, and an exit with any statement after it in the same block is not redundant either - notably the Exit Sub that stops a procedure falling into an On Error GoTo handler label, and the exit that leaves unreachable code behind it, which is a different defect. A Return that carries a value is never reported.

Rationale

The statement has no effect, so the only thing it can do is mislead. A reader who sees an explicit exit assumes it is there to skip something, and has to scan to the end of the procedure to establish that there is nothing left to skip - a cost paid on every reading, for no benefit. It also creates a hazard for the next edit: a statement appended after the exit becomes unreachable, which is a defect the compiler accepts, and appending is exactly what happens when a procedure grows. Redundant exits accumulate in the same procedures that acquire real early exits, and once a procedure contains a mixture of the two, the meaningful ones stop standing out - which is when a real early exit gets removed during cleanup because it looked like the decorative kind.

The following code illustrates the pattern detected by this rule:

Public Sub Write(ByVal entry As String)
    Console.WriteLine(entry)
    ' FLAGGED: Redundant Return or Exit statement at the end of a procedure
    Exit Sub
End Sub

Remediation

Delete the statement. Falling off the end of a Sub or a property accessor returns to the caller, and falling off the end of a Function returns the return type’s default, so behaviour is unchanged. If the procedure is a Function and reaching the end without an explicit value was not intended, that is the real defect: replace the bare Exit Function with a Return that supplies the value the caller expects, or throw, rather than leaving the caller to receive Nothing, zero or False.

Configuration

This detector does not need any configuration.

References