Why doesn't FxCop tell CA2000 for this trivial case of non-deleted class instances?

Question

The following code creates a CA2000 ("Dispose objects before lose scope") violation for the first line of Main, but not the second. I would really like the CA2000 violation for the second line, because this is (obviously, a simplified) pattern, often found in the large code base I'm working on.

Does anyone know why the violation is not made for the second line?

public static void Main() { new Disposable(); MakeDisposable(); } private static Disposable MakeDisposable() { return new Disposable(); } private sealed class Disposable : IDisposable { public void Dispose() { } } 

Answer 1

Short answer: CA2000 is broken and is unlikely to be fixed soon. See this error report , which almost matches your question:

CA2000 warning, in particular, is a rule with known problems and that we cannot fix it in its current form.

---

The longer answer is that using the CA2000 correctly is difficult. In previous versions of Visual Studio, especially in 2010, false CA2000 warnings ran everywhere and you could not do anything to get them to leave. Search for any of the dozens of questions about this.

Even in cases where you can eliminate the warning, the workaround was almost worse than just leaving it in place. The problem is that when you are here, it is that you do not want the object to be deleted before it leaves the scope of the factory method. In addition, you do - if it throws an exception . In this case, the return value of the method is lost, and the calling object does not have the right to dispose of the object for itself.

Unfortunately, trying to figure this out means analyzing the program flow of the compiled IL to determine if any code path (including the exceptional ones) allows the object to flow. The end result was just about any case where you tried to return an IDisposable from a method would create an error.

Microsoft responded by doing two things:

• Decrease in sensitivity of CA2000 so that it works only in the most obvious cases. This seems reasonable, as obvious cases like your first line are more likely to be errors than more obscure, and easier to fix.
• Take CA2000 from their recommended code analysis rules; note that there is no longer a CA2000 in their Extended Validity rule set.

Along with rewriting the Roslyn compiler, it is possible for tools such as FxCop to do some analysis at the source level, which might be easier to get in this case. Meanwhile, the general consensus is to simply turn off the CA2000.

---

In case you're interested, a little testing seems to confirm that the current (CA) CA rule only fires if a locally containing IDisposable instance is dropped out of scope. IOW, an object cannot leave a method in which you are new , or CA ignores it. This leads me to believe that CA simply does not delve into methods for analysis. Besides trying to silence false positives, can it also be part of a general attempt to speed up the CA process by cutting out some expensive checks that, in my opinion, occurred between 2010 and 2012?

If I add a bit to your example, you will see an obvious pattern from which you will get a warning:

 class Program { public static void Main() { new Disposable(); // CA2000 IDisposable x; MakeDisposable(out x); Test(); } public static Disposable Test() { IDisposable z; var x = MakeDisposable(out z); var y = new Disposable(); // CA2000 return x; } private static Disposable MakeDisposable(out IDisposable result) { result = new Disposable(); new Disposable(); // CA2000 return new Disposable(); } } 

Answer 2

I have a suspicion that this is due to a combination of factors.

---

1. MakeDisposable works fine :

As a result of MakeDisposable there is no error, because, well, why? You may have

 using ( var disposable = MakeDisposable() ) { // elided } 

anywhere in your code.

---

2. Links to IDisposable in MakeDisposable not counted on Main

Calling MakeDisposable() in the Main method does not raise an error, because the compiler does not know that the return value of MakeDisposable is a reference only to IDisposable .

In other words, in the eyes of the compiler, the following forms of MakeDisposable() equivalent:

 private static Disosable MakeDisposable() { return new Disosable(); } 

(original) or set the support field, creating if necessary:

 private static Disposable disposable; private static Disposable MakeDisposable() { if ( disposable == null ) disposable = new Disposable(); return disposable; } private static void DisposeDisposable() { // elided } 

or even exposing an already created support field:

 private static Disposable disposable = new Disposable(); private static Disposable MakeDisposable() { return disposable; } private static void DisposeDisposable() { // elided } 

therefore, the call to Main valid.

In Main() , where you create an instance of IDisposable , with new Disposable(); the compiler knows that you are not passing it from this area, therefore it correctly throws an error.