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?

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. — Servy, Jun 12 '14 at 19:15
Servy, apologies, I edited the post to show the actual warning I get, which is "expression is always false". — David Rutten, Jun 12 '14 at 19:16

score 3 · Accepted Answer · answered Jun 12 '14 at 19:15

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.

Well.. here's hoping everyone plays nice. – David Rutten Jun 12 '14 at 19:19 — David Rutten, Jun 12 '14 at 19:19

score 2 · Answer 2 · answered Jun 12 '14 at 19:15

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.

Resharper, ICloneable and never null

2 Answers2