ReSharper can´t find unreachable code

Question

I have this code:

Assert.IsTrue(datasetMetadata1 != null && datasetMetadata1.Length == 5);
Assert.IsTrue(datasetMetadata2 != null && datasetMetadata2 .Length == 11);

if ((datasetMetadata1 == null || datasetMetadata1.Length != 5) ||
(datasetMetadata2 == null || datasetMetadata2 .Length != 11)
{ 
    /* do something */ 
}

which ReSharper simplifies by removing the redundant (because allways true) expression == null and by inverting the if-statement to something similar to:

if ((datasetMetadataPunktort.Length == 5) && (datasetMetadataFlurstueck.Length == 11)) 
    return

However for me it seems even this check is meaningless and can easily be omited as the condition is allways true. So I wonder why ReSharper detects the obsolete check against null but not for the rest.

Am I missing any case where the check fails?

How is Resharper supposed to know what values will Length have at run time? — Alex, Sep 29 '16 at 05:56
@j0ey_wh Hmmm, fair point. It can´t know if the property is immutable, that´s what you mean, isn´t it? — MakePeaceGreatAgain, Sep 29 '16 at 05:57
Maybe if you introduce a variable of the length will it notice? — Euphoric, Sep 29 '16 at 05:59
@Euphoric Nope, even when storing the `Length` into a variable and use it for both the `Assert` and the if-statement R# won´t notice they are equal conditions. — MakePeaceGreatAgain, Sep 29 '16 at 06:04
I don't think R# actually looks at values - it can eliminate null check after access to property (since it would NRE on that property access in worst case), but unlikely to check values. — Alexei Levenkov, Sep 29 '16 at 06:08
I do not cleary understand why we should write this kind of redundant code at all? — Michael Würthner, Sep 29 '16 at 07:05
@MichaelWürthner I agree, the question is: why doesn´t R# notice it *is* redundant. — MakePeaceGreatAgain, Sep 29 '16 at 07:08
To my mind, the thing is the following : Each time you test your value, you make a call to the getter. Resharper doesn't know if your actual getter modifies or not your value. It could be possible that the first time you call the getter it returns 5 and increments the value by 6. So next time you will have 11 returned. — Vasilievski, Sep 29 '16 at 07:29

score 1 · Answer 1 · answered Sep 29 '16 at 08:03

In addition to Visalievski's answer, let me add another - even simpler - example:

int i = 5;
if (i != 5)
{
    // do something
}

With this snippet, ReSharper doesn't detect unreachable code.

But let me do a small change to that code: make i constant.

const int i = 5;
if (i != 5)
{
    // do something
}

Now, ReSharper complains about unreachable code, and I get compiler warning CS0162 in VS2015.

So when dealing with specific values, both ReSharper and the compiler ask if the value is guaranteed to be constant or not. Hence I conclude that the heuristics used with null and with specific values are different.

score 0 · Accepted Answer · answered Sep 29 '16 at 07:51

to explain my comment :

To my mind, the thing is the following : Each time you test your value, you make a call to the getter. Resharper doesn't know if your actual getter modifies or not your value. It could be possible that the first time you call the getter it returns 5 and increments the value by 6. So next time you will have 11 returned.

I created this small console application as a sample :

This clas contains the parameter with the special getter.

public class TestClass
{
    private int _length;

    public int Length
    {
        get
        {
            var localLength = _length;
            _length += 6;
            return localLength;
        }
        set { _length = value; }
    }

    public TestClass(int length)
    {
        this._length = length;
    }
}

This class is used for test purposes :

  class Program
{
    static void Main(string[] args)
    {
        var testObject = new TestClass(5);
        if ((testObject.Length == 5) && (testObject.Length == 11))
        {
            Console.WriteLine("TRUE");
        }
        else
        {
            Console.WriteLine("FALSE");
        }
        Console.Read();

    }
}

And we have the following output :

TRUE

I agree this class is very special, and has been made in the purpose to make the condition working, but still it shows that the case is possible.

Overall, it shows that since the getter is called between each condition, the value can change, so the call is not redundant.

ReSharper can´t find unreachable code

2 Answers2