S4158 false positive

SonarLint Visual Studio
1

SonarLint warns that the collection rs is known to be empty (S4158) on the last if statement in the code below

                Private Sub Foo()
            Dim rs As New Dictionary(Of Integer, Integer)
            Dim collection As Byte() = {1, 2}
            collection.ForEach(Sub(item)
                                   rs.Add(item, item)
                               End Sub)
            If Not rs.ContainsKey(1) Then
                'do something
            End If
        End Sub
3

I assume this is Visual Basic?!

4

Yes. VB.NET

1 Like
5

Seems a FP to me.

6

I know it is a FP (see the subject). I was hoping the product will be fixed

7

I would hope so to. I was just confirming that I agree with your observation. (There have been reports here, that turned out not to be FP’s).

1 Like
9

Hi @DBug, I’m trying to reproduce it with your snippet but it does not compile. I think it’s because the ForEach method is not available for arrays in VB.
If I change the collection type to a list, it’s compliant:

Private Sub Foo2()
    Dim rs As New Dictionary(Of Integer, Integer)
    Dim collection As New List(Of Byte) From {1, 2}
    collection.ForEach(Sub(item)
                           rs.Add(item, item)
                       End Sub)
    If Not rs.ContainsKey(1) Then ' Compliant
        'do something
    End If
End Sub

If you need an array, than this equivalent version compiles and does not raise any issue:

Private Sub Foo()
    Dim rs As New Dictionary(Of Integer, Integer)
    Dim collection As Byte() = {1, 2}
    For Each item In collection
        rs.Add(item, item)
    Next
    If Not rs.ContainsKey(1) Then ' Compliant
        'do something
    End If
End Sub

Please let me know if this resolves your issue. If not, kindly provide a new snippet with the FP for me to better understand. Thank you!

10

Thanks for noticing that. ForEach comes from an external library MoreLinq that adds more operators on IEnumerable. I think that SonarLint should be tweaked so that when it looks at this code if it sees a lambda that modifies the collection it should not assume the collection will be empty later on. It should try to see if the lambda will be executed or not (if possible)

11

I see, thanks for clarifying.
I’ve added a reproducer to our UTs and opened a ticket on our backlog to track this issue. Thanks!