Variable declarations in loops should have explicit initialization

Unlike many other languages, VB.NET does not implicitly initialize variables defined in loop bodies to their default values on each iteration. The variable is only implicitly initialized when allocating its memory for the first time and keeps the its current value into the next iteration.

This code example from StackOverflow illustrates the pitfall pretty well:

It’s a compile-time error in C# to use an uninitialized local variable, so the problem does not exist there.
If someone needs to keep the value for the next iteration, the variable should be declared and initialized outside the loop body. One difference would be that the scope of that variable is no longer limited to the loop in that case.

Noncompliant code:

    Dim numbers As New List(Of Integer) From {1, 2, 3, 4, 5}
    For Each number As Integer In numbers

        Dim isEven As Boolean ' NONCOMPLIANT

        If number Mod 2 = 0 Then
            isEven = True
        End If

        If isEven Then
            Console.WriteLine(number.ToString() & " is Even")
        Else
            Console.WriteLine(number.ToString() & " is Odd")
        End If

    Next

Output:

1 is Odd
2 is Even
3 is Even
4 is Even
5 is Even

Compliant code:

    Dim numbers As New List(Of Integer) From {1, 2, 3, 4, 5}
    For Each number As Integer In numbers

        Dim isEven As Boolean = False ' COMPLIANT

        If number Mod 2 = 0 Then
            isEven = True
        End If

        If isEven Then
            Console.WriteLine(number.ToString() & " is Even")
        Else
            Console.WriteLine(number.ToString() & " is Odd")
        End If

    Next

(Even better if the rule could check if the variable is later assigned on all possible execution paths before its first use. Then it would be okay to not have the explicit initialization in the declaration.)

Output:

1 is Odd
2 is Even
3 is Odd
4 is Even
5 is Odd

External references:
This behavior in VB.NET is by design:

Even if the scope of a variable is limited to a block, its lifetime is still that of the entire procedure. If you enter the block more than once during the procedure, each block variable retains its previous value. To avoid unexpected results in such a case, it is wise to initialize block variables at the beginning of the block.

See also section 10.9 from the VB11 language specification:

Each time a loop body is entered, a fresh copy is made of all local variables declared in that body, initialized to the previous values of the variables. Any reference to a variable within the loop body will use the most recently made copy.

(This section is basically a nod to lambdas. I did not check if lambdas in VB.NET will use the value the variable had at the time the lambda was created or the value that it has when the lambda is actually run when declaring it outside of the loop.)

Type:
Code Smell

Tags:
Pitfall

Hi @CrushaKRool,

Welcome to the community and thank you for suggesting this rule.

This behavior can indeed be confusing but I see two potential problems:

1/ We cannot change the definition of “good code” because of rules’ limitations. If a rule raises issues on valid code it is a “False Positive”, and the rule is wrong. SonarQube/SonarCloud/SonarLint are used by developers to block releases when the code has actual issues, so we have to be very careful.

Today the VB.Net analyser is indeed not able to detect if a variable is initialized on all execution paths, so it would raise False Positives.

2/ The two answers on stackoverflow suggest that they would actually use the fact that a variable keeps its previous value. If this is the case the rule might only apply in some cases. ex: when developers are used to another programming language and don’t intend to use this VB.net feature. We will need to do some more research to see when this rule applies.

Cheers,
Nicolas