Code Analysis CA1063 fires when deriving from IDis

问题:

I have some code that will trigger Code Analysis warning CA1063:

CA1063 : Microsoft.Design : Remove IDisposable from the list of interfaces implemented by 'Functionality' and override the base class Dispose implementation instead.

However, I'm not sure what I need to do to fix this warning.

Briefly, I have an interface IFunctionality that derives from IDisposable. Class Functionality implements IFunctionality but derives from class Reusable to be able to reuse som code. Class Reusable also derives from IDisposable.

public class Reusable : IDisposable {

  ~Reusable() {
    Dispose(false);
  }

  public void Dispose() {
    Dispose(true);
    GC.SuppressFinalize(this);
  }

  protected virtual void Dispose(Boolean disposing) {
    // ...
  }

  public void DoSomething() {
    // ...
  }

}

public interface IFunctionality : IDisposable {

  void DoSomething();

  void DoSomethingElse();

}

public class Functionality : Reusable, IFunctionality {

  public void DoSomethingElse() {
    // ...
  }

#if WORK_AROUND_CA1063
  // Removes CA1063
  protected override void Dispose(Boolean disposing) {
    base.Dispose(disposing);
  }
#endif

}

I can get rid of the warning by overriding Dispose on Functionality and calling the base class Dispose even though doing that should not change the semantics of the code .

So is there something about IDisposable in this context I have overlooked or is it just CA1063 that misfires for this particular construct?

I know that I can suppress CA1063 but the rule is quite broad and I don't want to miss any other problems in implementing IDisposable reported by this rule.

回答1:

This is a false positive due to a minor bug in the rule itself. When trying to figure out if a class re-implements IDisposable (after figuring out that there's a base class implementation that could be overridden), it only looks at whether the class' interfaces include IDisposable. Unfortunately, the interface list that shows up in the assembly metadata includes the "exploded" list of interfaces, including any interfaces inherited via interfaces that the class explicitly implements in the original C# code. This means that FxCop is seeing a declaration that looks like the following for your Functionality class:

public class Functionality : Reusable, IFunctionality, IDisposable
{
    ...
}

Given this metadata representation, the ImplementIDisposableCorrectly rule should be a bit more intelligent about how it attempts to determine whether the class is actually re-implementing IDisposable (for example, by looking for an explicit Dispose() implementation if the base class has an overridable Dispose(bool)). However, given that the rule doesn't do this, your best approach is to suppress the false positives.

BTW, I would recommend seriously considering using SuppressMessageAttribute for suppressing the false positives instead of your current conditional compilation approach. e.g.:

[SuppressMessage("Microsoft.Design", "CA1063:ImplementIDisposableCorrectly",
    Justification = "False positive.  IDisposable is inherited via IFunctionality.  See http://stackoverflow.com/questions/8925925/code-analysis-ca1063-fires-when-deriving-from-idisposable-and-providing-implemen for details.")]
public class Functionality : Reusable, IFunctionality
{
    ...
}

Also, you might want to seriously consider getting rid of the finalizer...

回答2:

Your 'workaround' is the correct pattern here, for a derived class that implements IDisposable again.

But I think you should reconsider the design of IFunctionality : IDisposable. Is being Disposable really a concern of IFunctionality ? I think that decision belongs to the implementing class.

回答3:

This is related to the use of IDisposable rather than the interface itself. You are simply implementing the recommended pattern for its use by providing and overriding a protected Dispose(bool) method - this is not part of the interface itself.

If your override method doesn't actually add anything, then there is no problem omitting it. Warning CA1063 is there to highlight the issue to you, but if you know there are actually no resources to dispose in your case, then you can get around it by either providing the empty implementation you have, or by excluding this particular rule for this particular file.

You can do this with the [SuppressMessage] attribute.