Feedback request on new warning

We’ve been internally discussing a new warning that we’ve added to VB 2005 and how we should treat it for upgrade, and I’d like to get some feedback from those people who have been using the beta or the community previews. A common error that we see is using a reference variable before it’s been assigned a value. For example:

Dim x As Collection
x.Add(10)

This code will throw an exception because I forgot to create a new collection using New. In VB 2005, this code will produce the following warning:

Variable 'x' is used before it has been assigned a value. A null reference exception could result at runtime.

We determine this warning by doing flow of control analysis on the function and determining that you’re using the variable before you gave it a value. This is all well and good, but the warning isn’t perfect. Just because a variable hasn’t been assigned a value doesn’t necessarily mean that there’s going to be a problem. For example:

Dim x As Collection

If Test() Then
x = New Collection()
End If

If x IsNot Nothing Then
x.Add(10)
End If

The test “x IsNot Nothing” will cause a warning that x might be used before it has been assigned a value. This is sort-of true if you look at the code in a simplistic way, but it’s really not a problem, of course, because we’re just testing against Nothing. An ideal solution, and one that we’re considering beyond VB 2005, is to add a more sophisticated level of analysis to the compiler that would allow more fine-grained analysis of null reference problems that would recognize the above situation as safe. But that’s beyond the scope of this release. For now, the level of analysis that we can do is identify places where you used a reference variable without assigning it a value.

Now, to the question: have people found this warning to be annoying on upgrade? New projects don’t seem to have major issues with this warning because you fix the warnings as you go along, by and large. But upgraded projects can be a different story: depending on the coding style used, a project might have a ton of these warnings show up when you first open it in VB 2005. For example, I recently upgraded the VBParser project and spent a little while fixing up 100 or so of these warnings, all of which were what you might call “false positives” – I was using variables before they had been assigned, but always in a safe and courteous manner.

What have people’s experiences been with this warning on upgrade? Was it annoying? Was it helpful? Would it be desirable to not have the warning turned on by default in upgrade? Just to be clear: we’re not planning on changing anything as it stands right now. But now that the beta’s been out in the wild for a while, we thought it might be a good time to validate our assumptions against some cold, hard customer feedback…

23 thoughts on “Feedback request on new warning

  1. Scott Shriver

    I agree with Phillip. I like the warning and I think it’s helpful much more often than it gets in the way. If I had a nickel for every time I forgot the New keyword in VS2003 and had to stop and restart the app….

    Reply
  2. Daniel Moth

    Dim s As String ‘ warning!
    s &= "Why?" Another one:
    ‘ if only VB had "out" like C# has
    Private Function ReturnsValueByRef(ByRef theMethodAssignsThis As String) As Boolean
    theMethodAssignsThis = "some value to return"
    Return True
    End Function Private Sub Calls()
    Dim s As String ‘ warning!
    Me.ReturnsValueByRef(s)
    End Sub I filed a bug report back in July… it is still open!
    http://lab.msdn.microsoft.com/productfeedback/viewfeedback.aspx?feedbackid=7a497fd9-3d09-438f-97f3-9d43c86c8db8]]>

    Reply
  3. Dan McKinley

    I’ve found this to be useful. The one time I ran a very large project through the upgrade, my impression was that by and large the issues found were legitimate. I definitely would leave it on.

    Reply
  4. Sergio Pereira

    It is a good indication that there might be some hole in the logic and, even if there isn’t, you’ll be compelled to be more defensive.]]>

    Reply
  5. Karl

    but you quickly pick up the right habbit to stpp the compiler from complaining. As far as upgrades, it seems like few people use the "Treat compiler warnings as errors" flag. My experience with large upgrades is that such flags get turned off and it takes a while to be able to turn them back on again…which I think hurts the application (think option strict). As has been suggested, I think as long as you can disable that specific warning (as opposed to disabling all warnings as errors) you give developers total flexibility. Karl]]>

    Reply
  6. Pingback: Daniel Moth

  7. Kyle Reed

    dv = new dataview(tbl)
    Catch ex as exception
    throw ex
    Finally
    if not dv is nothing then
    dv.dispose
    end if
    End Try To fix this problem I have to do something very dumb and declare the variable like this: dim dv as dataview = Nothing. ]]>

    Reply
  8. Christopher Quick

    ….
    Catch

    Return X End Try We need to be able to do a "Ignore" on a warning like we can in any kind of spell checking system. Please!]]>

    Reply

Leave a Reply