VB -> C#: Problem converting long if statement with nullable integers

[cysieks]

VB.Net input code

'Dim newDays As Integer?
'Dim oldDays As Integer?

If (newDays.HasValue AndAlso Not oldDays.HasValue) _
                OrElse (newDays.HasValue AndAlso oldDays.HasValue AndAlso newDays <> oldDays) _
                OrElse (Not newDays.HasValue AndAlso oldDays.HasValue) Then

'Some code
End If

Erroneous output

if ((bool)((newDays.HasValue && !oldDays.HasValue ? true : newDays.HasValue && oldDays.HasValue ? newDays is var arg1 && oldDays is { } arg2 && arg1.HasValue ? arg1 != arg2 : null : false).GetValueOrDefault() || !newDays.HasValue && oldDays.HasValue))
// Some code

Expected output

if ((newDays.HasValue && !oldDays.HasValue) || (newDays.HasValue && oldDays.HasValue && newDays != oldDays) || (!newDays.HasValue && oldDays.HasValue))

Details

• Product in use: e.g. both
• Version in use: e.g. 9.1.2
• Did you see it working in a previous version, which? No
• Any other relevant information to the issue, or your interest in contributing a fix.

[GrahamTheCoder]

Thanks for this. It looks like the code dealing with nullables in boolean statements could do with some special cases for things like HasValue to tidy them up.

[cysieks]

Yes, the same is with e.g. nullable enums. We have a huge solution that needs to be migrated to C#, and this becomes a blocker.
Thank you for the great tool.

[GrahamTheCoder]

Just to confirm, have you seen the code producing different results in any case, or is it "just" that code looks terrible?

[cysieks]

Hi,

During conversion we have multiple strange results like these:

 adminDate = administrationAccess.SelectAdminDate(XXX) 'this is nullable
                
If adminDate Is Nothing OrElse adminDate > chartingTimeAllowanceEnd Then
    adminDate = Clock.Now
End If

is converted to:

if ((bool)(adminDate is null || (chartingTimeAllowanceEnd is var arg2 && adminDate is { } arg1 ? arg1 > arg2 : (bool?)null).GetValueOrDefault()))

or

Dim status As PasswordStatus? = securityManager.GetUserPasswordStatus(userName)

  If status = PasswordStatus.Locked Then
      failedCommZoneMessage.Message += " (the user is locked out of the system)"
  End If

to

var status = securityManager.GetUserPasswordStatus(userName);
if (Conversions.ToInteger(PasswordStatus.Locked) is var arg2 && (int?)status is { } arg1 && arg1 == arg2)
{
    failedCommZoneMessage.Message += " (the user is locked out of the system)";
}

[GrahamTheCoder]

Thanks for the examples. Yep I can see how littering the code base with those would be a blocker.
In terms of fixing this, if anyone's interested (@Yozer ?) this is the area that needs some special cases: https://github.com/Yozer/CodeConverter/blob/master/CodeConverter/CSharp/VisualBasicNullableExpressionsConverter.cs

[GrahamTheCoder]

I'm planning to have an initial look at improving this today

[GrahamTheCoder]

Since it will be hard to make this work perfectly, a useful workaround for people who have just a few cases of this, is to change the input code to use .Value when you know that the value isn't null.

e.g. newDays.Value <> oldDays.Value in:

'Dim newDays As Integer?
'Dim oldDays As Integer?

If (newDays.HasValue AndAlso Not oldDays.HasValue) _
                OrElse (newDays.HasValue AndAlso oldDays.HasValue AndAlso newDays.Value <> oldDays.Value) _
                OrElse (Not newDays.HasValue AndAlso oldDays.HasValue) Then

'Some code
End If

will convert to:

        if (newDays.HasValue && !oldDays.HasValue || newDays.HasValue && oldDays.HasValue && newDays.Value != oldDays.Value || !newDays.HasValue && oldDays.HasValue)

        {

            // Some code
        }

[Yozer]

This is a super tricky area. We have to take into account vb.net nullable logic and output something that doesn't look like complete garbage.

1. One big improvement would be to do flow analysis/heuristic checks to verify if a variable can be null. This would help a lot with conditions where we have .HasValue but we access nullable variable without .Value.
2. Another one is to simplify the logic when we deal with constants on lhs or rhs.
3. Yet another improvement can be achieved if we know that the result of comparison has to be bool (like in the second example in this issue).
   status = PasswordStatus.Locked type of this in vb.net is Nullable(Of Boolean) but when we do it in if/switch etc then we can just output simple bool.
   This optimization can only be applied in a few special cases otherwise we will break conversion for code that does this:

Dim result = status = PasswordStatus.Locked ' can't do simple conversion to status = PasswordStatus.Locked because in C# it will be bool instead of bool?
If Not result.HasValue Then Console.WriteLine("Failed to auth")

In theory, this should deal with it so it doesn't generate super over-complicated code but looks like it doesn't work:
https://github.com/Yozer/CodeConverter/blob/13d0ff4c62af601e115a1aa9f7a331dde98db5cd/CodeConverter/CSharp/VisualBasicNullableExpressionsConverter.cs#L117

Dim status As PasswordStatus? = GetUserPasswordStatus(userName)

If status = PasswordStatus.Locked Then
    Console.WriteLine(" (the user is locked out of the system)")
End If

sharplab.io output:

Nullable<PasswordStatus> userPasswordStatus = GetUserPasswordStatus();
Nullable<int> num = (userPasswordStatus.HasValue ? new Nullable<int>((int)userPasswordStatus.GetValueOrDefault()) : null);
if ((num.HasValue ? new Nullable<bool>(num.GetValueOrDefault() == 0) : null).GetValueOrDefault())
{
    Console.WriteLine("(the user is locked out of the system)");
}

It's interesting that the converter uses here Conversions.ToInteger when casting enum to int:

public void M()
{
    var status = GetUserPasswordStatus();

    if (Conversions.ToInteger(PasswordStatus.Locked) is var arg2 && (int?)status is { } arg1 && arg1 == arg2)
    {
        System.Console.WriteLine("(the user is locked out of the system)");
    }
}

[GrahamTheCoder]

Thanks for the thoughts on this. After putting together #984 I have even more appreciation for your previous work on it (and the tests that went with it) so thanks again for that too. I made a start on points 1 and 3, and believe I have dealt with the Conversions.ToInteger issue. That's about all the time I have to work on this for now, so feel free to pick up where I left off if you're interested. I'll close this issue since I believe I've covered the examples given, but just reopen it or create a new issue if there are more examples worth fixing

[cysieks]

Thank you!
Is there any ETA when this will be available in dotnet tool version? tool update shows the latest '9.1.2.564'.

[GrahamTheCoder]

Tomorrow or the next day hopefully 🙂

[GrahamTheCoder]

Should be available now. I know some cases will still be ugly so if you see other nullable related issues in the new version, feel free to reopen this with as many examples as you can find. Thanks!