[BUG] Possible false positive with ImplementIDisposableCorrectly
Hi guys,
I have encountered what appears to be a false positive with ImplementIDisposableCorrectly.
When analyzing the code below, DerivedClass appears to trigger this violation incorrectly stating: 'Remove IDisposable from the list of interfaces implemented by WindowsApplication3.DerivedClass and override the base class Dispose implementation instead.'
After some research, I found out that even though DerivedClass doesn't implement IDisposable directly, it is still included as one of its implemented interfaces by the C# compiler.
Interestingly enough, if you remove the DisposableClass from the inheritance chain this rule doesn't get violated, nor does it if you remove BaseClass's protected Dispose(Boolean) override.
| |
publicclass DisposableClass : IDisposable
{
publicvoid Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
} protectedvirtualvoid Dispose(bool disposing)
{
}
} publicclass BaseClass : DisposableClass
{
protectedoverridevoid Dispose(bool disposing)
{
base.Dispose(disposing);
}
} publicclass DerivedClass : BaseClass, IDerivedClass
{
protectedoverridevoid Dispose(bool disposing)
{
base.Dispose(disposing);
}
} publicinterface IDerivedClass : IDisposable
{
}
|
What do you think?
David
Interesting! The violation here is that you're not supposed to reimplement IDisposable. Of course, if you reimplement it and dispatch identically to a virtual call to Dispose() on your base class, then there's no harm, and this is exactly what the compiler has done in this case.
The second issue about not firing was another bug, nice catch! I've got fixes ready for both issues.
Thanks for reporting these,
Nick
Is this problem still present?
I get the same violation from the following code for the derived class, with a few changes:
Code Snippet
public class DisposableClass : IDisposableClass /////////////
{
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
protected virtual void Dispose(bool disposing)
{
}
}
///////////////
public interface IDisposableClass : IDisposable
{}
///////////////
public class DerivedClass : BaseClass, IDerivedClass
{
}
public interface IDerivedClass : IDisposable
{
}
Should the above code create that violation? And if so, is this just a bad idea in general?
Everything is ok if I take IDisposable out of IDerivedClass, but I need that there, so this makes me think there is a design flaw.
Nicholas,
Unfortuantely, while the original problem was fixed (which was actually raised by me before I worked at Microsoft 
), the second issue you've raised is not fixed.
We do already have a bug on this, unfortunately, it was found to late to make into the Visual Studio 2008/FxCop 1.36 releases.
For now, unfortunately, you will need to suppress the warning using the SuppressMessageAttribute.
Regards
David
