Ported from https://github.com/dotnet/roslyn/issues/4830. See description in that issue.
We will fix this in the FxCopAnalyzers NuGet package (not in FxCop).
We've decided to cut this rule. See https://github.com/dotnet/roslyn-analyzers/issues/695
I'm not using FxCopAnalyzers and have this issue. How can I solve it, it cannot be suppressed? I'm using pure Visual Studio 2015.
@msmolka Is it an option for you to change the code
this.session?.Dispose();
to this:
if (this.session != null)
{
this.session.Dispose();
}
?
Yes, this is not causing analyzers issue, but don't allow use new null propagation. So what is a purpose of won't fix? The solution on bug in analyzers is not to use new C# features?
@msmolka The rationale for not fixing this is that it was a very noisy rule, and our opinion was that it wasn't valuable enough to do the work necessary to reduce the noise. See #695 for discussion. Not everybody agrees with the decision to cut this rule, and we're tracking the issue for future consideration in the document Proposed FxCop rule changes in Roslyn
As to the issue of not using new C# features: You can certainly use new C# features, even the null conditional operator. It's only in this very specific circumstance (namely, using it to decide whether or not to dispose a resource) that you might consider avoiding the null conditional operator to avoid the bug in the FxCop rule.
Given the noise level of this rule, I would suggest turning off the rule from the ruleset editor to avoid this issue.
Thank you for your answer. Basically I like this warning, because it keeps code clean. I also like null propagation, so I would like to have both this features.
Thanks for the feedback. We'll re-consider this decision once we're done with the first round of implementing the rules that have been identified as high-value (based on low noise and high usage).
I was also annoyed by the null conditional operator not working.
Any news on this?
I just add a SuppressMessage attribute with a justification..
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2213:DisposableFieldsShouldBeDisposed", MessageId = "_cancellationTokenSource", Justification = "Null-conditional confuses the code analysis.")]
Roy
Bump, I hit it. Jeah CA2213 seams to be noisy, but as i tried to calm the good of code analysis I was disappointed that the ? Operator is not part of the analyzer 101.
@JackGrinningCat How are you executing analysis? "Run code analysis" command or are you adding a NuGet reference to latest FxCop analyzers?
@JackGrinningCat Can you check if CA2213 implemented in Microsoft.CodeQuality.Analyzers.Exp works fine? You just need to add a NuGet package reference and build the project to trigger analysis.
I'll do that on Monday. Easy to check, thanks for the hint in the direction.Am 16.03.2018 5:47 nachm. schrieb Manish Vasani notifications@github.com:@JackGrinningCat Can you check if CA2213 implemented in Microsoft.CodeQuality.Analyzers.Exp works fine? You just need to add a NuGet package reference and build the project to trigger analysis.
—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.
According to my a few tests on .NET Framework Console App on VS2019 16.4.2 (Roslyn 3.x according to this page), the legacy code analysis (right click on project and run code analysis without changing project settings, so maybe by binary analyzer?) still has this issue but after I install Microsoft.CodeAnalysis.FxCopAnalyzers, this issue is not reproduced.
@mavasani is this behavior same as your expectation? I could not find clear information about whether this issue has been fixed.
@cactuaroid That is the correct. However, note that starting VS2019 16.5 and later, "Run Code Analysis" command in VS runs analyzers, not legacy binary analysis. See https://docs.microsoft.com/visualstudio/code-quality/how-to-run-code-analysis-manually-for-managed-code. So you should not see this issue in 16.5 or later.
I understand why people want to have such rule, but to be honest - I've never seen it working to the point where it is actually useful. The amount of false-positives is always outweighs any potential benefits.
The common pattern to dispose an owned disposable field is:
if (disposable != null)
{
disposable.Dispose();
disposable = null;
}
proper one liner for that is
Interlocked.Exchange(ref disposable, null)?.Dispose();
and it still triggers the rule.
¯\_(ツ)_/¯
Most helpful comment
Thank you for your answer. Basically I like this warning, because it keeps code clean. I also like null propagation, so I would like to have both this features.