Fix S2933 FP: readonly fields in a struct re-assigned with 'this'

[fiotti]

Description

S2933 disagrees with IDE0064.

Repro steps

struct Buffer : IDisposable
{
    byte[]? _rented; // S2933: Make '_rented' 'readonly'.
    byte[] _bytes; // S2933: Make '_bytes' 'readonly'.

    public byte[] Data => _bytes;

    public Buffer(byte[] buffer)
    {
        _bytes = buffer;
    }

    public Buffer(int capacity)
    {
        _rented = ArrayPool<byte>.Shared.Rent(capacity);
        _bytes = _rented;
    }

    public void Dispose()
    {
        if (_rented != null)
            ArrayPool<byte>.Shared.Return(_rented);

        this = default; // <-- Look here.
    }
}

But after adding the readonly modifiers, Visual Studio complains with:

IDE0064: Struct contains assignment to 'this' outside of costructor. Make readonly fields writable.

Expected behavior

I would expect SonarLint to either disable IDE0064 or not report this case at all.

According to Microsoft it is incorrect to add the readonly modifiers in this case.

Actual behavior

SonarLint reports a warning.

Known workarounds

#pragma warning disable S2933

or

#pragma warning disable IDE0064

or

 public void Dispose()
 {
     if (_rented != null)
         ArrayPool<byte>.Shared.Return(_rented);

-    this = default; // <-- Look here.
+    _rented = null;
+    _bytes = null!;
 }

Related information

• SonarLint for Visual Studio 2022, version 8.9.0.92083
• Visual Studio Professional 2022 (64 bit), version 17.10.4
• dotnet 8.0.303
• MSBuild version 17.10.4+10fbfbf2e for .NET Framework, version 17.10.4.21802
• Windows 11 Pro 23H2 22631.3880

[zsolt-kolbay-sonarsource]

Hi @fiotti. Thank you for reporting the issue. Confirmed as False Positive.

Out of curiosity: why do you use a struct instead of a class in the code sample? Creating an IDisposable struct sounds dangerous, as it's passed by value (copied).

var buf = new Buffer();
using (var insideBuffer = new Buffer(1000))
{
   buf = insideBuffer;
}
// After this line buf._rented will point to a memory location that's no longer usable.

[fiotti]

Hi @zsolt-kolbay-sonarsource 🙋‍♂️ This example is artificial just to trigger the warning. I used a struct instead of a class because it's not possible to assign this = something; inside classes.

For reference, I took inspiration from this code from Microsoft, which happens to involve a ref struct with a Dispose() method, even though it doesn't implement IDisposable as ref structs cannot implement interfaces.

Microsoft agrees with you, btw. They are not making this struct public until we get support for the [NonCopyable] attribute which allows to mark structs as non-safe-for-copy.