Boolean expression compared with the True or False literal

ID

vbnet.maintainability.boolean_compare_with_literal

Severity

low

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Control Flow

Language

VB.NET

Description

Reports an expression compared against the True or False literal with = or <>, where the comparison adds nothing because the expression is already a condition. All four combinations are reported - = True, = False, <> True and <> False - in an If or ElseIf condition (block and single-line forms), in a While or Do While header, and in a Return. Assigning a Boolean literal to a variable is a different thing and is not reported: isActive = True as a statement is an assignment, not a comparison.

Rationale

Comparing a condition with a Boolean literal is redundant, and the redundancy is what makes it worth removing rather than tolerating. Every reader has to check the literal to work out the sense of the test, and because = True and <> False mean the same thing while = False and <> True mean the opposite, four spellings appear across a codebase for what are only two tests - so reading a condition becomes an exercise in double negation rather than reading a name. That is where the defects come from: <> False sitting next to = False in an If …​ ElseIf chain is very easy to write when the negation was meant, and equally easy to read past in review. The form also invites a genuine bug in Visual Basic, where = is both the comparison and the assignment operator: dropping the comparison entirely removes any chance of reading one as the other.

The following code illustrates the pattern detected by this rule:

Public Function Greeting(ByVal isActive As Boolean) As String
    ' FLAGGED: Boolean expression compared with the True or False literal
    If isActive = True Then
        Return "Welcome back"
    End If
    Return "Account suspended"
End Function

Remediation

Drop the comparison and use the condition on its own. If isActive = True Then becomes If isActive Then, and Return isBanned <> False becomes Return isBanned. For the negative cases use the Not operator rather than a literal: If isActive = False Then becomes If Not isActive Then, and Return isActive <> True becomes Return Not isActive. If the result reads awkwardly once the literal is gone, the fix is usually the name and not the comparison - rename the operand to a positive assertion such as isActive or hasAccess so the bare condition reads as a sentence.

Configuration

This detector does not need any configuration.

References