IndexOf result compared against a positive bound

ID

vbnet.maintainability.indexof_check_wrong_bound

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Control Flow

Language

VB.NET

Description

Reports the result of IndexOf or LastIndexOf being compared against a positive bound - > 0, >= 1, < 1 or ⇐ 0 - where the "not found" sentinel is -1 and the correct bound is therefore 0.

Rationale

IndexOf and LastIndexOf return the zero-based position of the item and -1 when it is not present, so a match at the very first position returns 0. Testing > 0 or >= 1 therefore treats a match at position 0 exactly like "not found": path.IndexOf("/") > 0 reports False for "/etc/passwd", and list.IndexOf(item) > 0 reports False for the first element. The negated forms < 1 and ⇐ 0 are wrong in the same way, reporting "absent" for an item found at position 0. The result is an off-by-one that only shows up on the boundary case, so it survives ordinary testing.

The following code illustrates the pattern detected by this rule:

Public Function HasSeparator(ByVal header As String) As Boolean
    ' FLAGGED: IndexOf result compared against a positive bound
    Return header.IndexOf(":") > 0
End Function

Remediation

Compare against the sentinel, not against a positive number: use >= 0 (or <> -1) for "found" and < 0 (or = -1) for "not found". When the position itself is not needed, prefer the intent-revealing Contains for strings and collections, or StartsWith when the test is really about the first position.

' Before: False for a separator at position 0
Return header.IndexOf(":") > 0

' After
Return header.IndexOf(":") >= 0
' Or, when the position is not needed
Return header.Contains(":")

Configuration

This detector does not need any configuration.

References