Skip to content

Commit

Permalink
Merge pull request #1001 from philippejer/issues-998-1000
Browse files Browse the repository at this point in the history
Fix Issue #998 and #1000
  • Loading branch information
GrahamTheCoder authored Apr 15, 2023
2 parents f23e1df + 4aa4fdc commit 900df70
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private IEnumerable<VariableDeclarationSyntax> HoistVariablesBeforeLoopWhenNeede
if (!assignedBeforeRead) {
_perScopeState.Hoist(new HoistedDefaultInitializedLoopVariable(
csVariable.Identifier.Text,
// e.g. "b As Boolean" has no intializer but can turn into "var b = default(bool)"
// e.g. "b As Boolean" has no initializer but can turn into "var b = default(bool)"
csVariable.Initializer?.Value,
variablesDecl.Decl.Type,
_perScopeState.IsInsideNestedLoop()));
Expand Down
27 changes: 15 additions & 12 deletions CodeConverter/CSharp/PerScopeState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,20 @@ public T HoistToParent<T>(T additionalLocal) where T : IHoistedNode
return additionalLocal;
}

private readonly VBasic.SyntaxKind[] loopKinds = {
VBasic.SyntaxKind.DoKeyword,
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));
return _hoistedNodesPerScope.Skip(1).Any(x => _loopKinds.Contains(x.ExitableKind));
}

public bool IsInsideNestedLoop()
{
return _hoistedNodesPerScope.Skip(1).Count(x => loopKinds.Contains(x.ExitableKind)) > 1;
return _hoistedNodesPerScope.Skip(1).Count(x => _loopKinds.Contains(x.ExitableKind)) > 1;
}

public T HoistToTopLevel<T>(T additionalField) where T : IHoistedNode
Expand Down Expand Up @@ -113,18 +115,19 @@ public async Task<SyntaxList<StatementSyntax>> CreateLocalsAsync(VBasic.VisualBa

foreach (var variable in GetDefaultInitializedLoopVariables()) {
if (IsInsideLoop()) {
newNames.Add(variable.OriginalVariableName, variable.Id);
if (variable.Nested) {
newNames.Add(variable.OriginalVariableName, variable.Id);
}
HoistToParent(variable);
} else {
string name;
// The variable comes from the VB scope, only check for conflict with other hoisted definitions
string name = NameGenerator.GenerateUniqueVariableName(generatedNames, variable.OriginalVariableName);
if (variable.Nested) {
newNames.Add(variable.Id, NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, variable.OriginalVariableName));
name = newNames[variable.Id];
} else {
name = variable.OriginalVariableName;
newNames.Add(variable.Id, name);
} else if (name != variable.OriginalVariableName) {
newNames.Add(variable.OriginalVariableName, name);
}
var decl =
CommonConversions.CreateVariableDeclarationAndAssignment(name,
var decl = CommonConversions.CreateVariableDeclarationAndAssignment(name,
variable.Initializer, variable.Type);
preDeclarations.Add(CS.SyntaxFactory.LocalDeclarationStatement(decl));
}
Expand Down
7 changes: 7 additions & 0 deletions CodeConverter/Common/NameGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,11 @@ private static List<int> GetScopeStarts(VBasic.VisualBasicSyntaxNode node)
return node.GetAncestorOrThis<VBSyntax.StatementSyntax>().DescendantNodesAndSelf()
.OfType<VBSyntax.StatementSyntax>().Select(n => n.SpanStart).ToList();
}

public static string GenerateUniqueVariableName(HashSet<string> generatedNames, string variableNameBase)
{
string uniqueName = GenerateUniqueName(variableNameBase, string.Empty, n => !generatedNames.Contains(n));
generatedNames.Add(uniqueName);
return uniqueName;
}
}
90 changes: 80 additions & 10 deletions Tests/CSharp/StatementTests/LoopStatementTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -687,18 +687,18 @@ Dim b As Integer
For j = 1 To 3
Dim c As Integer
c +=1
Console.WriteLine(""c1={0}"", c)
Console.WriteLine(""c={0}"", c)
Next
For j = 1 To 3
Dim c As Integer
c +=1
Console.WriteLine(""c2={0}"", c)
Console.WriteLine(""c1={0}"", c)
Next
Dim k=1
Do while k <= 3
Dim c As Integer
c +=1
Console.WriteLine(""c3={0}"", c)
Console.WriteLine(""c2={0}"", c)
k+=1
Loop
i += 1
Expand All @@ -712,34 +712,104 @@ private void TestMethod()
{
int i = 1;
var b = default(int);
var c = 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);
c += 1;
Console.WriteLine(""c={0}"", c);
}
for (int j = 1; j <= 3; j++)
{
c2 += 1;
Console.WriteLine(""c2={0}"", c2);
c1 += 1;
Console.WriteLine(""c1={0}"", c1);
}
int k = 1;
while (k <= 3)
{
c3 += 1;
Console.WriteLine(""c3={0}"", c3);
c2 += 1;
Console.WriteLine(""c2={0}"", c2);
k += 1;
}
i += 1;
}
while (i <= 3);
}
}");
}

[Fact]
public async Task ForWithVariableDeclarationIssue1000Async()
{
await TestConversionVisualBasicToCSharpAsync(@"Class TestClass
Private Sub TestMethod()
For i = 1 To 2
Dim b As Boolean
Console.WriteLine(b)
b = True
Next
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;
}
var b1 = default(bool);
for (int i = 1; i <= 2; i++)
{
Console.WriteLine(b1);
b1 = true;
}
}
}");
}

[Fact]
public async Task ForWithVariableDeclarationIssue998Async()
{
await TestConversionVisualBasicToCSharpAsync(@"Class TestClass
Private Sub TestMethod(someCondition As Boolean)
For j = 1 To 2
If someCondition Then
Dim b As Boolean
Console.WriteLine(b)
b = True
End If
Next
End Sub
End Class", @"using System;
internal partial class TestClass
{
private void TestMethod(bool someCondition)
{
var b = default(bool);
for (int j = 1; j <= 2; j++)
{
if (someCondition)
{
Console.WriteLine(b);
b = true;
}
}
}
}");
}
}

0 comments on commit 900df70

Please sign in to comment.