Comparisons of Nullable<T> types and code coverage can give some unexpected, but logical results. Earlier this week I posted a small Puzzle showing the problem.
The light blue shade of the return statements indicate that they have been executed. So both branches of the if statement have been covered. But the light pink shade of the comparison indicates that the comparison has not been completely covered.
That code was deliberately somewhat obfuscated. MyNumericType is defined by using MyNumericType = System.Nullable<IntStruct> and SingleDigitLimit is IntStruct SingleDigitLimit = new IntStruct(10);.
Simplifying the code it looks like this.
public struct IntStruct { public IntStruct(int value) { Value = value; } public readonly int Value; public static bool operator <(IntStruct v1, IntStruct v2) { return v1.Value < v2.Value; } public static bool operator >(IntStruct v1, IntStruct v2) { return v1.Value > v2.Value; } } public class SomeUtilClass { public static string IsSingleDigit(IntStruct? value) { if (value < new IntStruct(10)) { return "Yes!"; } return "No!"; } } |
The test cases that I’ve run checks for values of 9 and 10, but never null. That’s the case that’s not covered. The fix is of course to either make a test case for the null case, or to change the code to use value.Value if null will never be a possible value. But if the value can never be null why even have a nullable type? I can think of one reason and that’s when the nullable type is a member variable that can’t be null in a specific case. That’s actually exactly how I stumbled on this issue.
Only Applies for Nullable Structs
Being curious, I started to investigate the circumstances under which this occurs. As far as I’ve found out it only affects a very specific case.
- The type must be
System.Nullable<T>. Tmust be astruct.Tmust overload the operator used.
The reason for that is that internally the compiler basically changes the comparison to value.HasValue && value < new IntStruct(10)).
Into the IL code
But why does T have to be a struct for that to occur? Wouldn't it be the same for say an int?.
Well, basically it is the same, but the comparison isn't done in the same way so apparently the code coverage analyzer won't see it as a separate block to cover. This is the IL code for the code above.
.method public hidebysig static string IsSingleDigit(valuetype [mscorlib]System.Nullable`1<valuetype TestLib.NullableCodeCoverage.IntStruct> 'value') cil managed { // Code size 63 (0x3f) .maxstack 2 .locals init ([0] string CS$1$0000, [1] valuetype [mscorlib]System.Nullable`1<valuetype TestLib.NullableCodeCoverage.IntStruct> CS$0$0001, [2] valuetype TestLib.NullableCodeCoverage.IntStruct CS$0$0002, [3] bool CS$4$0003) IL_0000: nop IL_0001: ldarg.0 IL_0002: stloc.1 IL_0003: ldc.i4.s 10 IL_0005: newobj instance void TestLib.NullableCodeCoverage.IntStruct::.ctor(int32) IL_000a: stloc.2 IL_000b: ldloca.s CS$0$0001 IL_000d: call instance bool valuetype [mscorlib]System.Nullable`1<valuetype TestLib.NullableCodeCoverage.IntStruct>::get_HasValue() IL_0012: brtrue.s IL_0017 IL_0014: ldc.i4.0 IL_0015: br.s IL_0024 IL_0017: ldloca.s CS$0$0001 IL_0019: call instance !0 valuetype [mscorlib]System.Nullable`1<valuetype TestLib.NullableCodeCoverage.IntStruct>::GetValueOrDefault() IL_001e: ldloc.2 IL_001f: call bool TestLib.NullableCodeCoverage.IntStruct::op_LessThan(valuetype TestLib.NullableCodeCoverage.IntStruct, valuetype TestLib.NullableCodeCoverage.IntStruct) IL_0024: nop IL_0025: ldc.i4.0 IL_0026: ceq IL_0028: stloc.3 IL_0029: ldloc.3 IL_002a: brtrue.s IL_0035 IL_002c: nop IL_002d: ldstr "Yes!" IL_0032: stloc.0 IL_0033: br.s IL_003d IL_0035: ldstr "No!" IL_003a: stloc.0 IL_003b: br.s IL_003d IL_003d: ldloc.0 IL_003e: ret } // end of method SomeUtilClass::IsSingleDigit |
At IL_000d there is a call to HasValue for the valueargument. If that's true, it continues execution on IL_0017. But if it's false, it will push 0 (false) value on the stack and jump to IL_0024, effectively making the comparison result false. I assume that those two lines of IL (IL_0014 and IL_0015) make up the non covered block.
But shouldn't the code for an int? look the same? Let's see how it is handled.
.method public hidebysig static string IsSingleDigit(valuetype [mscorlib]System.Nullable`1<int32> 'value') cil managed { // Code size 51 (0x33) .maxstack 2 .locals init ([0] string CS$1$0000, [1] valuetype [mscorlib]System.Nullable`1<int32> CS$0$0001, [2] bool CS$4$0002) IL_0000: nop IL_0001: ldarg.0 IL_0002: stloc.1 IL_0003: ldloca.s CS$0$0001 IL_0005: call instance !0 valuetype [mscorlib]System.Nullable`1<int32>::GetValueOrDefault() IL_000a: ldc.i4.s 10 IL_000c: bge.s IL_0017 IL_000e: ldloca.s CS$0$0001 IL_0010: call instance bool valuetype [mscorlib]System.Nullable`1<int32>::get_HasValue() IL_0015: br.s IL_0018 IL_0017: ldc.i4.0 IL_0018: nop IL_0019: ldc.i4.0 IL_001a: ceq IL_001c: stloc.2 IL_001d: ldloc.2 IL_001e: brtrue.s IL_0029 IL_0020: nop IL_0021: ldstr "Yes!" IL_0026: stloc.0 IL_0027: br.s IL_0031 IL_0029: ldstr "No!" IL_002e: stloc.0 IL_002f: br.s IL_0031 IL_0031: ldloc.0 IL_0032: ret } // end of method SomeUtilClass::IsSingleDigit |
No, it doesn't look the same. It starts by calling GetValueOrDefault instead. It then changes the value < 10 comparison into 10 >= value at IL_000c. If that is true, it jumps to IL_0017 which will eventually cause it to return "No!"
If it instead continues with IL_000e because the value was indeed less than 10 (including 0 for null) it will call HasValue. Then comes the interesting trick, it jumps to IL_0018 skipping only one instruction (IL_0017). But that instruction is covered by the case when the value is greater or equal to 10.
The test at IL_001a is there to check the result of HasValue. But it is actually executed if the value was greater or equal than 10. Expressed in C# code it roughly does this:
bool isYes; if(value.GetValueOrDefault() >= 10) { isYes = false; } else { isYes = value.HasValue; } if(isYes == true) { return "Yes!"; } else { return "No!"; } |
So the code coverage is right, every single IL statement in this function is actually executed without having a test case for null!