IDE0066 incorrectly handles nullable value types

[stephentoub]

Version Used:
3.3.0-beta3-19406-05+a1905991543bed104f7f7f0842aca2b65d263b87

Steps to Reproduce:

class Program
{
    public static void Main() { }

    public static bool? GetBool(string name)
    {
        switch (name)
        {
            case "a": return true;
            case "b": return false;
            default: return null;
        }
    }
}

Expected Behavior:
IDE0066 refactors this into something that compiles, e.g.

class Program
{
    public static void Main() { }

    public static bool? GetBool(string name)
    {
        return name switch
        {
            "a" => true,
            "b" => false,
            _ => (bool?)null,
        };
    }
}

Actual Behavior:
IDE0066 refactors this into the following code that doesn't compile:

class Program
{
    public static void Main() { }

    public static bool? GetBool(string name)
    {
        return name switch
        {
            "a" => true,
            "b" => false,
            _ => null,
        };
    }
}

This fails with the error "error CS0037: Cannot convert null to 'bool' because it is a non-nullable value type".

[alrz]

Fixed in #37052

[alrz]

note that this would be still an issue if there is no target-type until dotnet/csharplang#33 is implemented, but we wont suggest 0066 in those cases at the moment.

[stephentoub]

Fixed in #37052

VS says my tools are from a190599, and according to git log, that includes fbdb480. Am I misinterpreting?

[alrz]

perhaps I am. According to the feature spec dotnet/csharplang#2389 I expect this to work - there's no natural type but all arms are implicitly convertible to the target type.

Edit: I do see why this happens though. The compiler yields bool as the best common type - excluding typeless expressions - but doesn't check if all expressions can be successfully converted. The same with (even different) reference types could work because best type algorithm would fail in the first place and if it's not, null can be seamlessly converted. My question is that if this is the intended behavior for the switch expression regarding nullable value types?

cc @gafter

[alrz]

The fix for IDE is in review #38007, in case this is the intended behavior.

[gafter]

Th problem is that there is a common type according to the current spec/implementation - because only two switch arms have expressions with types, and those are the same type. The spec and implementation should be amended to require that all arms are convertible to that common type in order for it to be considered the natural type of the switch.

[alrz]

The spec and implementation should be amended to require that all arms are convertible to that common type in order for it to be considered the natural type of the switch.

I'll note that dotnet/csharplang#33 will address this issue by taking null into account in the best-type inference, thought it is probably an unintuitive behavior at the moment.

[gafter]

Yes, but the switch expression must take lambdas, method groups, and other typeless things into account too.

[alrz]

So if I understand correctly, "nullable-enhanced common type" would not help with the following

(int?, int) t = b switch { true => (null, 1), false => (1, 2) };

but it should actually work because there's no natural type for the switch expression.

[gafter]

I think that's right. However, it should work whether or not there is a natural type for the switch expression.

[gafter]

I am going to aim to fix the compiler behavior in 16.4.

[gafter]

Compiler part moved to #38226