Fix #1300 - nullable loop conditions

CodeConverter/CSharp/ExpressionNodeVisitor.cs

@@ -412,6 +412,9 @@ public override async Task<CSharpSyntaxNode> VisitUnaryExpression(VBasic.Syntax.
 
     private async Task<ExpressionSyntax> NegateAndSimplifyOrNullAsync(VBSyntax.UnaryExpressionSyntax node, ExpressionSyntax expr, ITypeSymbol operandConvertedType)
     {
+        if (expr.SkipIntoParens().HasAnnotation(VisualBasicNullableExpressionsConverter.NegatedForParentNotAnnotation)) {
+            return expr;
+        }
         if (await _operatorConverter.ConvertReferenceOrNothingComparisonOrNullAsync(node.Operand.SkipIntoParens(), TriviaConvertingExpressionVisitor.IsWithinExpressionTree, true) is { } nothingComparison) {
             return nothingComparison;
         }

CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs

@@ -571,8 +571,7 @@ await node.Name.AcceptAsync<NameSyntax>(_expressionVisitor),
 
     public override async Task<SyntaxList<StatementSyntax>> VisitSingleLineIfStatement(VBSyntax.SingleLineIfStatementSyntax node)
     {
-        var condition = await node.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
-        condition = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Condition, condition, forceTargetType: CommonConversions.KnownTypes.Boolean);
+        var condition = await ConvertConditionAsync(node.Condition);
         var block = SyntaxFactory.Block(await ConvertStatementsAsync(node.Statements));
         ElseClauseSyntax elseClause = null;
 
@@ -585,8 +584,7 @@ public override async Task<SyntaxList<StatementSyntax>> VisitSingleLineIfStateme
 
     public override async Task<SyntaxList<StatementSyntax>> VisitMultiLineIfBlock(VBSyntax.MultiLineIfBlockSyntax node)
     {
-        var condition = await node.IfStatement.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
-        condition = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.IfStatement.Condition, condition, forceTargetType: CommonConversions.KnownTypes.Boolean);
+        var condition = await ConvertConditionAsync(node.IfStatement.Condition);
         var block = SyntaxFactory.Block(await ConvertStatementsAsync(node.Statements));
 
         var elseClause = await ConvertElseClauseAsync(node.ElseBlock);
@@ -604,11 +602,19 @@ public override async Task<SyntaxList<StatementSyntax>> VisitMultiLineIfBlock(VB
     private async Task<(ExpressionSyntax ElseIfCondition, BlockSyntax ElseBlock)> ConvertElseIfAsync(VBSyntax.ElseIfBlockSyntax elseIf)
     {
         var elseBlock = SyntaxFactory.Block(await ConvertStatementsAsync(elseIf.Statements));
-        var elseIfCondition = await elseIf.ElseIfStatement.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
-        elseIfCondition = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(elseIf.ElseIfStatement.Condition, elseIfCondition, forceTargetType: CommonConversions.KnownTypes.Boolean);
+        var elseIfCondition = await ConvertConditionAsync(elseIf.ElseIfStatement.Condition);
         return (elseIfCondition, elseBlock);
     }
 
+    /// <summary>
+    /// VB converts a condition to Boolean (e.g. Nothing to False), whereas C# requires it to already be bool
+    /// </summary>
+    private async Task<ExpressionSyntax> ConvertConditionAsync(VBSyntax.ExpressionSyntax vbCondition)
+    {
+        var condition = await vbCondition.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
+        return CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(vbCondition, condition, forceTargetType: CommonConversions.KnownTypes.Boolean);
+    }
+
     private async Task<ElseClauseSyntax> ConvertElseClauseAsync(VBSyntax.ElseBlockSyntax elseBlock)
     {
         if (elseBlock == null) return null;
@@ -1137,7 +1143,7 @@ async Task<CatchClauseSyntax> ConvertCatchBlockAsync(VBasic.Syntax.CatchBlockSyn
         async Task<CatchFilterClauseSyntax> ConvertCatchFilterClauseAsync(VBasic.Syntax.CatchFilterClauseSyntax node)
         {
             if (node == null) return null;
-            return SyntaxFactory.CatchFilterClause(await node.Filter.AcceptAsync<ExpressionSyntax>(_expressionVisitor));
+            return SyntaxFactory.CatchFilterClause(await ConvertConditionAsync(node.Filter));
         }
 
         async Task<FinallyClauseSyntax> ConvertFinallyBlockAsync(VBasic.Syntax.FinallyBlockSyntax node)
@@ -1182,7 +1188,7 @@ public override async Task<SyntaxList<StatementSyntax>> VisitUsingBlock(VBSyntax
     public override async Task<SyntaxList<StatementSyntax>> VisitWhileBlock(VBSyntax.WhileBlockSyntax node)
     {
         return SingleStatement(SyntaxFactory.WhileStatement(
-            await node.WhileStatement.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor),
+            await ConvertConditionAsync(node.WhileStatement.Condition),
             SyntaxFactory.Block(await ConvertStatementsAsync(node.Statements)).UnpackNonNestedBlock()
         ));
     }
@@ -1191,35 +1197,24 @@ public override async Task<SyntaxList<StatementSyntax>> VisitDoLoopBlock(VBSynta
     {
         var statements = SyntaxFactory.Block(await ConvertStatementsAsync(node.Statements)).UnpackNonNestedBlock();
 
-        if (node.DoStatement.WhileOrUntilClause != null) {
-            var stmt = node.DoStatement.WhileOrUntilClause;
-            if (stmt.WhileOrUntilKeyword.IsKind(VBasic.SyntaxKind.WhileKeyword))
-                return SingleStatement(SyntaxFactory.WhileStatement(
-                    await stmt.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor),
-                    statements
-                ));
-            return SingleStatement(SyntaxFactory.WhileStatement(
-                (await stmt.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor)).InvertCondition(),
-                statements
-            ));
+        if (node.DoStatement.WhileOrUntilClause is { } topClause) {
+            return SingleStatement(SyntaxFactory.WhileStatement(await ConvertLoopContinueConditionAsync(topClause), statements));
         }
 
-        var whileOrUntilStmt = node.LoopStatement.WhileOrUntilClause;
-        ExpressionSyntax conditionExpression;
-        bool isUntilStmt;
-        if (whileOrUntilStmt != null) {
-            conditionExpression = await whileOrUntilStmt.Condition.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
-            isUntilStmt = whileOrUntilStmt.WhileOrUntilKeyword.IsKind(VBasic.SyntaxKind.UntilKeyword);
-        } else {
-            conditionExpression = SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression);
-            isUntilStmt = false;
-        }
-
-        if (isUntilStmt) {
-            conditionExpression = conditionExpression.InvertCondition();
-        }
+        var bottomCondition = node.LoopStatement.WhileOrUntilClause is { } bottomClause
+            ? await ConvertLoopContinueConditionAsync(bottomClause)
+            : SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression);
+        return SingleStatement(SyntaxFactory.DoStatement(statements, bottomCondition));
+    }
 
-        return SingleStatement(SyntaxFactory.DoStatement(statements, conditionExpression));
+    /// <summary>
+    /// C# only has While loops, so an Until loop continues while its condition is false.
+    /// VB converts the Until condition to Boolean (Nothing to False) before negating it, so an Until loop continues while it's Nothing.
+    /// </summary>
+    private async Task<ExpressionSyntax> ConvertLoopContinueConditionAsync(VBSyntax.WhileOrUntilClauseSyntax clause)
+    {
+        var condition = await ConvertConditionAsync(clause.Condition);
+        return clause.WhileOrUntilKeyword.IsKind(VBasic.SyntaxKind.UntilKeyword) ? condition.InvertCondition() : condition;
     }
 
     public override async Task<SyntaxList<StatementSyntax>> VisitCallStatement(VBSyntax.CallStatementSyntax node)

CodeConverter/CSharp/TypeConversionAnalyzer.cs

@@ -1,4 +1,4 @@
-using ICSharpCode.CodeConverter.Util.FromRoslyn;
+using ICSharpCode.CodeConverter.Util.FromRoslyn;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.FindSymbols;
@@ -108,12 +108,11 @@ private ExpressionSyntax AddTypeConversion(VBSyntax.ExpressionSyntax vbNode, Exp
                 return addParenthesisIfNeeded ? vbNode.ParenthesizeIfPrecedenceCouldChange(csNode) : csNode;
             case TypeConversionKind.DestructiveCast:
             case TypeConversionKind.NonDestructiveCast:
-                return CreateCast(csNode, vbConvertedType);
+                return CreateCastOrConditionCast(vbNode, csNode, vbType, vbConvertedType);
             case TypeConversionKind.Conversion:
                 return AddExplicitConvertTo(vbNode, csNode, vbType, vbConvertedType);
             case TypeConversionKind.NullableBool:
-                return SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, csNode,
-                    LiteralConversions.GetLiteralExpression(true));
+                return csNode.EqualsTrue();
             case TypeConversionKind.StringToCharArray:
                 var memberAccessExpressionSyntax = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, csNode, ValidSyntaxFactory.IdentifierName(nameof(string.ToCharArray)));
                 return SyntaxFactory.InvocationExpression(memberAccessExpressionSyntax,
@@ -340,9 +339,8 @@ private bool TryAnalyzeCsConversion(VBasic.VisualBasicCompilation vbCompilation,
             typeConversionKind = TypeConversionKind.Conversion;
             return true;
         } 
-        else if (csConversion.IsNullable && csConvertedType.SpecialType == SpecialType.System_Boolean && vbNode.AlwaysHasBooleanTypeInCSharp() &&
-                 (vbNode is not VBSyntax.BinaryExpressionSyntax and not VBSyntax.UnaryExpressionSyntax || vbNode.IsKind(VBasic.SyntaxKind.AndExpression, VBasic.SyntaxKind.OrExpression, VBasic.SyntaxKind.ExclusiveOrExpression))) {
-            typeConversionKind = TypeConversionKind.NullableBool;
+        else if (csConversion.IsNullable && csConvertedType.SpecialType == SpecialType.System_Boolean && vbNode.AlwaysHasBooleanTypeInCSharp()) {
+            typeConversionKind = IsMadeBoolByNullableOperatorConversion(vbNode) ? TypeConversionKind.Identity : TypeConversionKind.NullableBool;
             return true;
         }
         else if (csConversion.IsExplicit) {
@@ -354,6 +352,16 @@ private bool TryAnalyzeCsConversion(VBasic.VisualBasicCompilation vbCompilation,
         return false;
     }
 
+    /// <summary>
+    /// In a condition, the nullable operator conversions already give bool (so no conversion is needed) for operators such as comparisons, Not, AndAlso and OrElse, but not for And, Or and Xor
+    /// </summary>
+    private static bool IsMadeBoolByNullableOperatorConversion(VBSyntax.ExpressionSyntax vbNode)
+    {
+        var unparenthesized = vbNode.SkipIntoParens();
+        return unparenthesized is VBSyntax.BinaryExpressionSyntax or VBSyntax.UnaryExpressionSyntax &&
+               !unparenthesized.IsKind(VBasic.SyntaxKind.AndExpression, VBasic.SyntaxKind.OrExpression, VBasic.SyntaxKind.ExclusiveOrExpression);
+    }
+
     private static bool IsExactTypeNumericLiteral(VBSyntax.ExpressionSyntax vbNode, ITypeSymbol vbConvertedType) => 
         vbNode is VBSyntax.LiteralExpressionSyntax literal &&
         LiteralConversions.ConvertLiteralNumericValueOrNull(literal.Token.Value, vbConvertedType) is {};
@@ -426,6 +434,39 @@ private ExpressionSyntax AddRoundInvocation(ExpressionSyntax csNode)
         return SyntaxFactory.InvocationExpression(memberAccess, arguments);
     }
 
+    /// <summary>
+    /// Converting Nothing to a non-nullable type throws in VB, except in a condition: there VB lifts the conversion to Boolean? and treats Nothing as False.
+    /// The default (zero) of every type convertible to Boolean also converts to False, so GetValueOrDefault matches that with a single evaluation.
+    /// </summary>
+    private static ExpressionSyntax GetUnderlyingValueToConvert(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNullableNode) =>
+        vbNode.AlwaysHasBooleanTypeInCSharp() ? csNullableNode.GetValueOrDefault() : csNullableNode.NullableGetValueExpression();
+
+    private ExpressionSyntax CreateCastOrConditionCast(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, ITypeSymbol vbType, ITypeSymbol vbConvertedType)
+    {
+        if (!vbNode.AlwaysHasBooleanTypeInCSharp()) return CreateCast(csNode, vbConvertedType);
+        if (GetNullableReturnTypeOfUserDefinedConversion(vbNode) is { } nullableBoolean) {
+            var operand = vbType.IsNullable() ? csNode.NullableGetValueExpression() : csNode;
+            return CreateCast(operand, nullableBoolean).EqualsTrue();
+        }
+        return IsNonBooleanNullable(vbType, vbConvertedType)
+            ? _vbNullableExpressionsConverter.ConvertWhenNotNullElseFalse(vbNode, csNode, value => CreateCast(value, vbConvertedType))
+            : CreateCast(csNode, vbConvertedType);
+    }
+
+    /// <summary>
+    /// In a condition, VB doesn't lift a user-defined CType returning Boolean? (a Nothing operand throws, whereas C# would lift it to give null),
+    /// but treats a Nothing result as False, like any other Boolean? condition
+    /// </summary>
+    private ITypeSymbol GetNullableReturnTypeOfUserDefinedConversion(VBSyntax.ExpressionSyntax vbNode) =>
+        VBasic.VisualBasicExtensions.GetConversion(_semanticModel, vbNode).MethodSymbol?.ReturnType is { } returnType && returnType.IsNullable() ? returnType : null;
+
+    /// <summary>
+    /// In a condition, VB lifts any other conversion to Boolean (e.g. a user-defined CType returning Boolean), so Nothing is False rather than throwing.
+    /// A Boolean? condition is excluded: the nullable Boolean logic already makes it bool.
+    /// </summary>
+    private static bool IsNonBooleanNullable(ITypeSymbol vbType, ITypeSymbol vbConvertedType) =>
+        vbType.IsNullable(out var underlyingType) && underlyingType.SpecialType != SpecialType.System_Boolean && !vbConvertedType.IsNullable();
+
     private ExpressionSyntax AddExplicitConvertTo(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, ITypeSymbol currentType, ITypeSymbol targetType)
     {
         var displayType = targetType.ToMinimalDisplayString(_semanticModel, vbNode.SpanStart);
@@ -443,7 +484,7 @@ invoke.Expression is MemberAccessExpressionSyntax expr &&
         currentType.IsNullable(out var nullableCurrentType);
         targetType.IsNullable(out var nullableTargetType);
         if (nullableCurrentType != null && nullableTargetType == null) {
-            csNode = csNode.NullableGetValueExpression();
+            csNode = GetUnderlyingValueToConvert(vbNode, csNode);
         }
         var typeNameForConversionMethod = nullableTargetType ?? targetType;
 

CodeConverter/CSharp/VbSyntaxNodeExtensions.cs

@@ -20,15 +20,34 @@ public static bool PrecedenceCouldChange(this VBasic.VisualBasicSyntaxNode node)
         return parentIsMemberAccessExpression || parentIsNonArgumentExpression && !parentIsBinaryExpression && !parentIsLambda && !parentIsParenthesis;
     }
 
+    public static bool IsOrderingComparison(this VBSyntax.ExpressionSyntax node) =>
+        node.IsKind(VBasic.SyntaxKind.LessThanExpression, VBasic.SyntaxKind.LessThanOrEqualExpression,
+            VBasic.SyntaxKind.GreaterThanExpression, VBasic.SyntaxKind.GreaterThanOrEqualExpression);
+
+    /// <summary>
+    /// Where VB binds a condition as Boolean (e.g. Nothing becomes False), the C# condition must be bool.
+    /// VB's binder passes the condition context through parentheses (isOperandOfConditionalBranch in Binder_Expressions.vb), so this does too.
+    /// </summary>
     public static bool AlwaysHasBooleanTypeInCSharp(this VBSyntax.ExpressionSyntax vbNode)
     {
-        var parent = vbNode.SkipOutOfParens()?.Parent;
+        var condition = vbNode.OutermostParenthesizedOrSelf();
 
-        return parent is VBSyntax.SingleLineIfStatementSyntax singleLine && singleLine.Condition == vbNode ||
-               parent is VBSyntax.IfStatementSyntax ifStatement && ifStatement.Condition == vbNode ||
-               parent is VBSyntax.ElseIfStatementSyntax elseIfStatement && elseIfStatement.Condition == vbNode ||
-               parent is VBSyntax.TernaryConditionalExpressionSyntax ternary && ternary.Condition == vbNode ||
-               parent is VBSyntax.WhereClauseSyntax or VBSyntax.PartitionWhileClauseSyntax;
+        return condition.Parent switch {
+            VBSyntax.SingleLineIfStatementSyntax singleLine => singleLine.Condition == condition,
+            VBSyntax.IfStatementSyntax ifStatement => ifStatement.Condition == condition,
+            VBSyntax.ElseIfStatementSyntax elseIfStatement => elseIfStatement.Condition == condition,
+            VBSyntax.TernaryConditionalExpressionSyntax ternary => ternary.Condition == condition,
+            VBSyntax.WhileStatementSyntax whileStatement => whileStatement.Condition == condition,
+            VBSyntax.WhileOrUntilClauseSyntax or VBSyntax.CatchFilterClauseSyntax => true,
+            VBSyntax.WhereClauseSyntax or VBSyntax.PartitionWhileClauseSyntax => true,
+            _ => false
+        };
+    }
+
+    public static VBSyntax.ExpressionSyntax OutermostParenthesizedOrSelf(this VBSyntax.ExpressionSyntax vbNode)
+    {
+        while (vbNode.Parent is VBSyntax.ParenthesizedExpressionSyntax parenthesized) vbNode = parenthesized;
+        return vbNode;
     }
 
     /// <summary>