Use existing method to invert condition - fixes #226

I believe this will generally yield more readable code than parenthesizing the whole expression

Author: GrahamTheCoder

ICSharpCode.CodeConverter/CSharp/MethodBodyVisitor.cs

@@ -635,7 +635,7 @@ public override SyntaxList<StatementSyntax> VisitDoLoopBlock(VBSyntax.DoLoopBloc
                             statements
                         ));
                     return SingleStatement(SyntaxFactory.WhileStatement(
-                        SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, (ExpressionSyntax)stmt.Condition.Accept(_nodesVisitor)),
+                        ((ExpressionSyntax)stmt.Condition.Accept(_nodesVisitor)).InvertCondition(),
                         statements
                     ));
                 }
@@ -652,7 +652,7 @@ public override SyntaxList<StatementSyntax> VisitDoLoopBlock(VBSyntax.DoLoopBloc
                 }
 
                 if (isUntilStmt) {
-                    conditionExpression = SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, conditionExpression);
+                    conditionExpression = conditionExpression.InvertCondition();
                 }
 
                 return SingleStatement(SyntaxFactory.DoStatement(statements, conditionExpression));

ICSharpCode.CodeConverter/CSharp/NodesVisitor.cs

@@ -1337,11 +1337,7 @@ public override CSharpSyntaxNode VisitTypeOfExpression(VBSyntax.TypeOfExpression
                     (ExpressionSyntax)node.Expression.Accept(TriviaConvertingVisitor),
                     (TypeSyntax)node.Type.Accept(TriviaConvertingVisitor)
                 );
-                if (node.IsKind(VBasic.SyntaxKind.TypeOfIsNotExpression))
-                    return SyntaxFactory.PrefixUnaryExpression(
-                            SyntaxKind.LogicalNotExpression,
-                            SyntaxFactory.ParenthesizedExpression(expr));
-                return expr;
+                return node.IsKind(VBasic.SyntaxKind.TypeOfIsNotExpression) ? expr.InvertCondition() : expr;
             }
 
             public override CSharpSyntaxNode VisitUnaryExpression(VBSyntax.UnaryExpressionSyntax node)
@@ -1350,18 +1346,13 @@ public override CSharpSyntaxNode VisitUnaryExpression(VBSyntax.UnaryExpressionSy
                 if (node.IsKind(VBasic.SyntaxKind.AddressOfExpression))
                     return expr;
                 var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken(TokenContext.Local);
+                SyntaxKind csTokenKind = CSharpUtil.GetExpressionOperatorTokenKind(kind);
                 return SyntaxFactory.PrefixUnaryExpression(
                     kind,
-                    SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind)),
-                    MaybeParenthesize(expr, kind)
+                    SyntaxFactory.Token(csTokenKind),
+                    expr.AddParensIfRequired()
                 );
             }
-            private ExpressionSyntax MaybeParenthesize(ExpressionSyntax expr, SyntaxKind operatorKind)
-            {
-                return expr is BinaryExpressionSyntax && operatorKind == SyntaxKind.LogicalNotExpression
-                    ? SyntaxFactory.ParenthesizedExpression(expr)
-                    : expr;
-            }
 
             public override CSharpSyntaxNode VisitBinaryExpression(VBSyntax.BinaryExpressionSyntax node)
             {

ICSharpCode.CodeConverter/Util/CSharpUtil.cs

@@ -13,7 +13,7 @@ static class CSharpUtil
         /// Inverts a boolean condition. Note: The condition object can be frozen (from AST) it's cloned internally.
         /// </summary>
         /// <param name="condition">The condition to invert.</param>
-        public static ExpressionSyntax InvertCondition(ExpressionSyntax condition)
+        public static ExpressionSyntax InvertCondition(this ExpressionSyntax condition)
         {
             return InvertConditionInternal(condition);
         }
@@ -51,8 +51,7 @@ static ExpressionSyntax InvertConditionInternal(ExpressionSyntax condition)
                 return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, SyntaxFactory.ParenthesizedExpression(condition));
             }
 
-            if (condition is ConditionalExpressionSyntax) {
-                var cEx = condition as ConditionalExpressionSyntax;
+            if (condition is ConditionalExpressionSyntax cEx) {
                 return cEx.WithCondition(InvertCondition(cEx.Condition));
             }
 
@@ -63,7 +62,7 @@ static ExpressionSyntax InvertConditionInternal(ExpressionSyntax condition)
                     return SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression);
             }
 
-            return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, AddParensIfRequired(condition, false));
+            return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, condition.AddParensIfRequired());
         }
 
         public static SyntaxKind GetExpressionOperatorTokenKind(SyntaxKind op)
@@ -133,7 +132,7 @@ public static SyntaxKind GetExpressionOperatorTokenKind(SyntaxKind op)
         /// When negating an expression this is required, otherwise you would end up with
         /// a or b -> !a or b
         /// </summary>
-        public static ExpressionSyntax AddParensIfRequired(ExpressionSyntax expression, bool parenthesesRequiredForUnaryExpressions = true)
+        public static ExpressionSyntax AddParensIfRequired(this ExpressionSyntax expression, bool parenthesesRequiredForUnaryExpressions = false)
         {
             if ((expression is BinaryExpressionSyntax) ||
                 (expression is AssignmentExpressionSyntax) ||

Tests/CSharp/StatementTests.cs

@@ -760,6 +760,31 @@ private void TestMethod()
 }");
         }
 
+        [Fact]
+        public void UntilStatement()
+        {
+            //Bug: comment on statement in do loop gets moved to end of conditional
+            TestConversionVisualBasicToCSharpWithoutComments(@"Class TestClass
+    Private Sub TestMethod()
+        Dim charIndex As Integer
+        ' allow only digits and letters
+        Do
+            charIndex = rand.Next(48, 123)
+        Loop Until (charIndex >= 48 AndAlso charIndex <= 57) OrElse (charIndex >= 65 AndAlso charIndex <= 90) OrElse (charIndex >= 97 AndAlso charIndex <= 122)
+    End Sub
+End Class", @"class TestClass
+{
+    private void TestMethod()
+    {
+        int charIndex;
+        // allow only digits and letters
+        do
+            charIndex = rand.Next(48, 123);
+        while ((charIndex < 48 || charIndex > 57) && (charIndex < 65 || charIndex > 90) && (charIndex < 97 || charIndex > 122));
+    }
+}");
+        }
+
         [Fact]
         public void SimpleDoStatement()
         {