Condition uses And/Or instead of the short-circuiting AndAlso/OrElse

ID

vbnet.maintainability.use_short_circuit_logic

Severity

critical

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Control Flow

Language

VB.NET

Description

Flags a condition of an If, ElseIf, While or Do Until that contains And or Or rather than the short-circuiting AndAlso or OrElse. And and Or always evaluate both operands, so the right-hand side runs even when the left-hand side has already decided the result. The condition is matched as written in the source, so a Not (a Or b) is reported like any other. In Visual Basic And and Or are also the bitwise operators, and this rule cannot tell the two uses apart, because that needs the operand types and this engine has none: a bitwise test such as If (status And Mask) <> 0 Then is reported too. Where the operands are integers or flag enumerations rather than booleans, the finding is a false positive and the remediation does not apply - AndAlso would not even compile there.

Rationale

The guard idiom If account IsNot Nothing And account.Balance > 0 Then does not guard anything: And evaluates account.Balance regardless of the null check, so the line throws a NullReferenceException on exactly the input the check was written to survive. The same shape appears as If index < count And items(index) = target Then (IndexOutOfRangeException) and If dict.ContainsKey(k) Or Load(k) IsNot Nothing Then (the expensive load runs even when the key is present). Beyond the crashes, the right-hand operand’s side effects - a counter increment, a log line, a lazily-created object - fire on paths the author believed were excluded, so behaviour diverges from the obvious reading of the code. These sites are also easy to overlook in review because they read exactly like their short-circuiting counterparts, differing by four characters.

The following code illustrates the pattern detected by this rule:

Public Function IsUsable(ByVal account As Account) As Boolean
    ' FLAGGED: Condition uses And/Or instead of the short-circuiting AndAlso/OrElse
    If account IsNot Nothing And account.Balance > 0 Then
        Return True
    End If
    Return False
End Function

Remediation

Replace And with AndAlso and Or with OrElse in boolean conditions; the short-circuit forms stop as soon as the result is determined, which makes a left-to-right guard chain such as If account IsNot Nothing AndAlso account.Balance > 0 Then actually protect its right-hand side. Keep And and Or only for integer bitwise arithmetic (flags = flags And Not Mask) and for the rare case where both operands must be evaluated for their side effects - if that is the intent, evaluate them into named booleans on their own lines first, so the intent is visible rather than resting on the choice of operator.

Configuration

This detector does not need any configuration.

References