Merge pull request #902 from Noah1989/issue-897

Fix Issue #897 (VB -> C#: variable is initialized inside loop)

CodeConverter/CSharp/HoistedDefaultInitializedLoopVariable.cs

@@ -0,0 +1,24 @@
+using System.Diagnostics;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace ICSharpCode.CodeConverter.CSharp;
+
+internal class HoistedDefaultInitializedLoopVariable : IHoistedNode
+{
+    public string OriginalVariableName { get; }
+    public string Id { get; }
+    public ExpressionSyntax Initializer { get; }
+    public TypeSyntax Type { get; }
+    public bool Nested { get; }
+
+    public HoistedDefaultInitializedLoopVariable(string originalVariableName, ExpressionSyntax initializer, TypeSyntax type, bool nested)
+    {
+        Debug.Assert(initializer is DefaultExpressionSyntax);
+        OriginalVariableName = originalVariableName;
+        Id = $"ph{Guid.NewGuid():N}";
+        Initializer = initializer;
+        Type = type;
+        Nested = nested;
+    }
+
+}

CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs

@@ -123,7 +123,22 @@ public override async Task<SyntaxList<StatementSyntax>> VisitLocalDeclarationSta
                     _perScopeState.HoistToTopLevel(new HoistedFieldFromVbStaticVariable(methodName, variable.Identifier.Text, initializeValue, decl.Declaration.Type, isVbShared));
                 }
             } else {
-                declarations.AddRange(localDeclarationStatementSyntaxs);
+                foreach (var decl in localDeclarationStatementSyntaxs) {
+                    if (_perScopeState.IsInsideLoop() &&
+                        decl.Declaration.Variables.All(
+                            x => x.Initializer?.Value is DefaultExpressionSyntax
+                        )) {
+                        foreach (var variable in decl.Declaration.Variables) {
+                            _perScopeState.Hoist(new HoistedDefaultInitializedLoopVariable(
+                                variable.Identifier.Text, 
+                                variable.Initializer?.Value, 
+                                decl.Declaration.Type,
+                                _perScopeState.IsInsideNestedLoop()));
+                        }
+                    } else {
+                        declarations.Add(decl);
+                    }
+                }
             }
             var localFunctions = methods.Cast<LocalFunctionStatementSyntax>();
             declarations.AddRange(localFunctions);

CodeConverter/CSharp/PerScopeState.cs

@@ -40,16 +40,42 @@ public T Hoist<T>(T additionalLocal) where T : IHoistedNode
         return additionalLocal;
     }
 
-    public void HoistToTopLevel<T>(T additionalField) where T : IHoistedNode
+    public T HoistToParent<T>(T additionalLocal) where T : IHoistedNode
+    {
+        _hoistedNodesPerScope.ElementAt(1).Add(additionalLocal);
+        return additionalLocal;
+    }
+
+    private readonly VBasic.SyntaxKind[] loopKinds = {
+        VBasic.SyntaxKind.DoKeyword, 
+        VBasic.SyntaxKind.ForKeyword,
+        VBasic.SyntaxKind.WhileKeyword
+    };
+    public bool IsInsideLoop()
+    {
+        return _hoistedNodesPerScope.Skip(1).Any(x => loopKinds.Contains(x.ExitableKind));
+    }
+    public bool IsInsideNestedLoop()
+    {
+        return _hoistedNodesPerScope.Skip(1).Count(x => loopKinds.Contains(x.ExitableKind)) > 1;
+    }
+
+    public T HoistToTopLevel<T>(T additionalField) where T : IHoistedNode
     {
         _hoistedNodesPerScope.Last().Add(additionalField);
+        return additionalField;
     }
 
     public IReadOnlyCollection<AdditionalDeclaration> GetDeclarations()
     {
         return _hoistedNodesPerScope.Peek().OfType<AdditionalDeclaration>().ToArray();
     }
 
+    public IReadOnlyCollection<HoistedDefaultInitializedLoopVariable> GetDefaultInitializedLoopVariables()
+    {
+        return _hoistedNodesPerScope.Peek().OfType<HoistedDefaultInitializedLoopVariable>().ToArray();
+    }
+
     private StatementSyntax[] GetPostStatements()
     {
         var scopeState = _hoistedNodesPerScope.Peek();
@@ -83,12 +109,30 @@ public async Task<SyntaxList<StatementSyntax>> CreateLocalsAsync(VBasic.VisualBa
     {
         var preDeclarations = new List<StatementSyntax>();
         var postAssignments = new List<StatementSyntax>();
+        var newNames = new Dictionary<string, string>();
+
+        foreach (var variable in GetDefaultInitializedLoopVariables()) {
+            if (IsInsideLoop()) {
+                newNames.Add(variable.OriginalVariableName, variable.Id);
+                HoistToParent(variable);
+            } else {
+                string name;
+                if (variable.Nested) {
+                    newNames.Add(variable.Id, NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, variable.OriginalVariableName));
+                    name = newNames[variable.Id];
+                } else {
+                    name = variable.OriginalVariableName;
+                }
+                var decl = 
+                    CommonConversions.CreateVariableDeclarationAndAssignment(name,
+                    variable.Initializer, variable.Type);
+                preDeclarations.Add(CS.SyntaxFactory.LocalDeclarationStatement(decl));
+            }
+        }
 
         var additionalDeclarationInfo = GetDeclarations();
-        var newNames = additionalDeclarationInfo.ToDictionary(l => l.Id, l =>
-            NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, l.Prefix)
-        );
         foreach (var additionalLocal in additionalDeclarationInfo) {
+            newNames.Add(additionalLocal.Id, NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, additionalLocal.Prefix)); 
             var decl = CommonConversions.CreateVariableDeclarationAndAssignment(newNames[additionalLocal.Id],
                 additionalLocal.Initializer, additionalLocal.Type);
             preDeclarations.Add(CS.SyntaxFactory.LocalDeclarationStatement(decl));

Tests/CSharp/StatementTests/LoopStatementTests.cs

@@ -1,4 +1,5 @@
-using System.Threading.Tasks;
+using System;
+using System.Threading.Tasks;
 using ICSharpCode.CodeConverter.Tests.TestRunners;
 using Xunit;
 
@@ -417,6 +418,102 @@ private static void Main()
         Console.WriteLine(""Press any key to exit."");
         Console.ReadKey();
     }
+}");
+    }
+
+    [Fact]
+    public async Task ForWithVariableDeclarationIssue897Async()
+    {
+        await TestConversionVisualBasicToCSharpAsync(@"Class TestClass
+    Private Sub TestMethod()
+        For i = 1 To 2
+            Dim b As Boolean
+            Console.WriteLine(b)
+            b = True
+        Next
+    End Sub
+End Class", @"using System;
+
+internal partial class TestClass
+{
+    private void TestMethod()
+    {
+        var b = default(bool);
+        for (int i = 1; i <= 2; i++)
+        {
+            Console.WriteLine(b);
+            b = true;
+        }
+    }
+}");
+    }
+
+    [Fact]
+    public async Task NestedLoopsWithVariableDeclarationIssue897Async()
+    {
+        await TestConversionVisualBasicToCSharpAsync(@"Class TestClass
+    Private Sub TestMethod()
+        Dim i=1
+        Do
+            Dim b As Integer
+            b  +=1
+            Console.WriteLine(""b={0}"", b)
+            For j = 1 To 3
+                Dim c As Integer
+                c  +=1
+                Console.WriteLine(""c1={0}"", c)
+            Next
+            For j = 1 To 3
+                Dim c As Integer
+                c +=1
+                Console.WriteLine(""c2={0}"", c)
+            Next
+            Dim k=1
+            Do while k <= 3
+                Dim c As Integer
+                c +=1
+                Console.WriteLine(""c3={0}"", c)
+                k+=1
+            Loop
+        i += 1
+        Loop Until i > 3
+    End Sub
+End Class", @"using System;
+
+internal partial class TestClass
+{
+    private void TestMethod()
+    {
+        int i = 1;
+        var b = default(int);
+        var c1 = default(int);
+        var c2 = default(int);
+        var c3 = default(int);
+        do
+        {
+            b += 1;
+            Console.WriteLine(""b={0}"", b);
+            for (int j = 1; j <= 3; j++)
+            {
+                c1 += 1;
+                Console.WriteLine(""c1={0}"", c1);
+            }
+            for (int j = 1; j <= 3; j++)
+            {
+                c2 += 1;
+                Console.WriteLine(""c2={0}"", c2);
+            }
+            int k = 1;
+            while (k <= 3)
+            {
+                c3 += 1;
+                Console.WriteLine(""c3={0}"", c3);
+                k += 1;
+            }
+            i += 1;
+        }
+        while (i <= 3);
+    }
 }");
     }
 }

Tests/TestData/SelfVerifyingTests/VBToCS/VariableScopeTests.vb

@@ -0,0 +1,57 @@
+Imports System
+Imports System.Linq
+Imports System.Xml.Linq
+Imports Xunit
+
+Public Class VariableScopeTests
+
+    <Fact>
+    Sub TestDeclarationInsideForLoop()
+        For i = 1 To 2
+            Dim b As Boolean
+            If i = 1 Then
+                Assert.False(b)
+            Else
+                Assert.True(b)
+            End If
+
+            b = True
+            Assert.True(b)
+        Next
+    End Sub
+
+    <Fact>
+    Sub TestDeclarationsInsideNestedLoops()
+        Dim results As New List(Of String)
+        Dim i = 1
+        Do
+            Dim b As Integer
+            b += 1
+            results.Add("b=" & b)
+            For j = 1 To 3
+                Dim c As Integer
+                c += 1
+                results.Add("c1=" & c)
+            Next
+            For j = 1 To 3
+                Dim c As Integer
+                c += 1
+                results.Add("c2=" & c)
+            Next
+            Dim k = 1
+            Do While k <= 3
+                Dim c As Integer
+                c += 1
+                results.Add("c3=" & c)
+                k += 1
+            Loop
+            i += 1
+        Loop Until i > 3
+        Assert.Equal(
+            "b=1, c1=1, c1=2, c1=3, c2=1, c2=2, c2=3, c3=1, c3=2, c3=3, " &
+            "b=2, c1=4, c1=5, c1=6, c2=4, c2=5, c2=6, c3=4, c3=5, c3=6, " &
+            "b=3, c1=7, c1=8, c1=9, c2=7, c2=8, c2=9, c3=7, c3=8, c3=9",
+            String.Join(", ", results))
+    End Sub
+
+End Class