False positive for CA2000 when disposable field is captured by an instance field

[CollinAlpert]

Analyzer

Diagnostic ID: CA2000: Dispose objects before losing scope

Analyzer source

NuGet Package: Microsoft.CodeAnalysis.NetAnalyzers

Version: 7.0.3

Describe the bug

CA2000 is emitted on an expression which is later captured by an instance field and then disposed.

Steps To Reproduce

using System;
using System.Net.Http;

class Test : IDisposable {
    private readonly HttpClient _client;

    public Test() {
        var handler = new HttpClientHandler(); // <-- CA2000 here
        _client = new HttpClient(handler, true);
    }

    public void Dispose()
    {
        _client.Dispose();
    }
}

Expected behavior

No diagnostic is emitted.

Actual behavior

warning CA2000: Call System.IDisposable.Dispose on object created by 'new HttpClientHandler()' before all references to it are out of scope

Additional context

[mavasani]

@CollinAlpert This is by design, albeit not a very preferred one. #1617 discusses the core problem associated with dispose analysis and ownership, dotnet/runtime#29631 tracks some framework enhancements to help with this problem.

We do have an option to make the dispose analysis more conservative: https://github.com/dotnet/roslyn-analyzers/blob/main/docs/Analyzer%20Configuration.md#configure-dispose-ownership-transfer-for-arguments-passed-to-constructor-invocation. I verified that setting this option avoids this false positive. Alternatively, we also have a list of well-known types whose constructor invocation indicates ownership transfer, so this one could be one more addition:

roslyn-analyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/DisposeAnalysis/DisposeAnalysisHelper.cs

Lines 24 to 30 in 0d3a2bb

	private static readonly string[] s_disposeOwnershipTransferLikelyTypes = new string[]
	{
	"System.IO.Stream",
	"System.IO.TextReader",
	"System.IO.TextWriter",
	"System.Resources.IResourceReader",
	};

.
However, having such well-known type list is not very scalable, and hence not something we have added to that often. The rule in it's current state is just a port of the old CA2000 to detect basic leaks, but we would need a much better design and framework support for implementing an analysis that is much more reliable and precise.

[CollinAlpert]

Thanks for the explanation!