How can I detect when a ReadOnly property has accidentally been passed by reference?-CodePudding

I'm working on a project which is written in VB.NET. The project has several Structures which used to have writable fields. I replaced all of those fields with read-only properties, and wrote functions for creating a copy of a structure that has one of its properties changed.

I was assuming that every part of the code that attempted to write to one of these properties would become an error, and then I could simply fix all the errors by making the code call the new functions. To my dismay, it turns out that if a ReadOnly property is accidentally passed into a ByRef parameter of a function, the compiler accepts this with no warning, and the value that's assigned is silently discarded!

Here's an example:

Structure Point
    Public ReadOnly Property X As Integer
    Public ReadOnly Property Y As Integer
End Structure

Module Module1
    Sub IncreaseByOne(ByRef x As Integer)
        x = x   1
    End Sub

    Sub Main()
        Dim point As New Point
        IncreaseByOne(point.X)
        Console.WriteLine($"point.X is {point.X}")
    End Sub
End Module

I was hoping that the line IncreaseByOne(point.X) would throw an error, or at least a warning, since point.X is read-only and it doesn't make sense to pass it by reference. Instead, the code compiles with no warnings, and the value assigned to x inside of IncreaseByOne is silently discarded, and the program prints point.X is 0.

How can I detect all of the places in my code where a read-only property is passed into a function that takes it by reference? The only way I can think of is to go through every read-only property that I have, find all places where that property is used as a parameter, and look to see if that parameter is ByRef. That'll be very time-consuming, but if there's no other solution, then that's what I'll do.

I'm using Visual Studio 2019. I'm open to installing new software in order to do this.

CodePudding user response：

There isn't a way to catch this with the compiler. Even Option Strict On will allow passing a read-only property to a ByRef argument. This is defined to pass by copy-in/copy-out, and it's surprising to me that the copy-out part will compile even when the Property Set is inaccessible.

If you want to have an automated lint-type check for this, I would imagine that a custom analyzer could find it. I haven't worked with analyzers, so I don't have any specific suggestions for how to write one or set it up.

Otherwise, you're left to a manual check. As was noted in a comment, you can use the "Find All References" command from Visual Studio to help with it, but this will still require a manual review of every read-only property.

CodePudding user response：

That's really interesting. The VB.NET Compiler really tries to make a property look like a variable. Even if I explicitly declare the property as

Structure Point
    Dim _x As Integer

    ReadOnly Property X() As Integer
        Get
            Return _x
        End Get
    End Property
End Structure

The code compiles and executes as before. If the property setter is added, it even works correctly!

Structure Point
    Dim _x As Integer

    Property X() As Integer
        Get
            Return _x
        End Get
        Set(value As Integer)
            _x = value
        End Set
    End Property
End Structure

With the above change, the program correctly prints 1.

Looking at the generated IL, we can see why:

    IL_0009: ldloca.s     point
    IL_000b: call         instance int32 VisualBasicConsoleTest.Point::get_X()
    IL_0010: stloc.1      // Store returned value in local variable
    IL_0011: ldloca.s     // load address of that local variable (and pass to function call)
    IL_0013: call         void VisualBasicConsoleTest.Program::IncreaseByOne(int32&)
    IL_0018: nop
    IL_0019: ldloca.s     point
    IL_001b: ldloc.1      // Load contents of local variable again
    IL_001c: call         instance void VisualBasicConsoleTest.Point::set_X(int32) // and call setter

Even though we expect an error because a property is not a value (and a byref requires a value), the compiler fakes what we might have intended: He actually generates a call to the getter, stores the value on the stack, passes a reference to the stack(!) to the called function and then calls the setter with that value.

This works in this simple scenario, but I agree with the commenters above, this might be very confusing when looking at it in detail. If the property is actually a computed property, the outcome is just arbitrary (try implementing the getter as Return _x 1...)

C# would throw an error here, because a property is not a value and hence cannot be used as an out or ref parameter.