dotnet / roslyn-analyzers

MIT License
1.55k stars 460 forks source link

`CA2213` triggers when value is assigned in null check #7297

Open RenderMichael opened 2 months ago

RenderMichael commented 2 months ago

Analyzer

Diagnostic ID: CA2213: Disposable fields should be disposed

Analyzer source

SDK: .NET 8.0.300-preview.24203.14

Describe the bug

False positive on CA2213 when the async disposable comes from a provider (not directly through the constructor) and a null check assigns to a temp variable.

Steps To Reproduce

internal class HasDisposable : IAsyncDisposable
{
    private readonly IAsyncDisposable? _maybeValue;

    public HasDisposable(DisposableProvider maybeValue) => _maybeValue = maybeValue.Create();

    public async ValueTask DisposeAsync()
    {
        if (_maybeValue is { } d)
        {
            await d.DisposeAsync().ConfigureAwait(false);
        }
    }
}

internal class DisposableProvider
{
    public IAsyncDisposable? Create() => null;
}

Expected behavior

No diagnostic triggered

Actual behavior

A type that implements System.IDisposable declares fields that are of types that also implement IDisposable. The Dispose method of the field is not called by the Dispose method of the declaring type. To fix a violation of this rule, call Dispose on fields that are of types that implement IDisposable if you are responsible for allocating and releasing the unmanaged resources held by the field.

Additional context

if you change the dispose method to

if (_maybeValue is { })
{
    await _maybeValue.DisposeAsync().ConfigureAwait(false);
}

then the error does not trigger.

Additionally, this check exists because no await? syntax has been made. This complexity is one additional point in favor of the language feature.