Resharper, ICloneable and never null

Question

Resharper complains about the following code, saying that the last null check is redundant as the 'expression is always false':

  ICloneable data = item as ICloneable;
  if (data == null)
    throw new InvalidCastException("blah blah, some error message");

  object copy = data.Clone();
  if (copy == null) //  <-- this is where it complains.
    return default(T);

How does it know it can never be null?

Phonology

well if it is true, you'll only return null. My guess is that's what it's saying is redundant. Just use copy and if it's null it'll return null and if it's not it'll return not null. – Lauryn

Servy, apologies, I edited the post to show the actual warning I get, which is "expression is always false". – Phonology

Answer 1

ReSharper assumes that your object adheres to the contract of ICloneable, which says among other things that

The resulting clone must be of the same type as, or compatible with, the original instance.

From the fact that data is checked to be non-null and the assumption that you return an object of the same or compatible type from your implementation of ICloneable.Clone() ReSharper concludes that copy is also non-null, triggering the warning.

Of course it is definitely possible to return null from Clone. However, returning null would be a coding error, so it is a good idea to skip that null check.

Anse

Well.. here's hoping everyone plays nice. – Phonology

Answer 2

From MSDN:

Notes to Implementers
The ICloneable interface simply requires that your implementation of the Clone method return a copy of the current object instance.

If you're fulfilling the requirement of the contract, the Clone method should never return null.

Deon