Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement correct string comparisons for VB #309

Merged
merged 2 commits into from May 7, 2019
Merged

Implement correct string comparisons for VB #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7462e7c
Implement correct string comparisons for VB
mrmonday May 3, 2019
d108f30
Cleaner comparison handling for string literals
mrmonday May 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 43 additions & 3 deletions ICSharpCode.CodeConverter/CSharp/NodesVisitor.cs
View file
Open in desktop
Expand Up @@ -37,6 +37,7 @@ class NodesVisitor : VBasic.VisualBasicSyntaxVisitor<CSharpSyntaxNode>
private static readonly Type ExtensionAttributeType = typeof(ExtensionAttribute);
private readonly TypeConversionAnalyzer _typeConversionAnalyzer;
public CommentConvertingNodesVisitor TriviaConvertingVisitor { get; }
private bool _optionCompareText = false;

private CommonConversions CommonConversions { get; }

Expand Down Expand Up @@ -106,6 +107,9 @@ public override CSharpSyntaxNode VisitCompilationUnit(VBSyntax.CompilationUnitSy
var options = (VBasic.VisualBasicCompilationOptions)_semanticModel.Compilation.Options;
var importsClauses = options.GlobalImports.Select(gi => gi.Clause).Concat(node.Imports.SelectMany(imp => imp.ImportsClauses)).ToList();

_optionCompareText = node.Options.Any(x => x.NameKeyword.ValueText.Equals("Compare", StringComparison.OrdinalIgnoreCase) &&
x.ValueKeyword.ValueText.Equals("Text", StringComparison.OrdinalIgnoreCase));

var attributes = SyntaxFactory.List(node.Attributes.SelectMany(a => a.AttributeLists).SelectMany(ConvertAttribute));
var sourceAndConverted = node.Members.Select(m => (Source: m, Converted: ConvertMember(m))).ToReadOnlyCollection();
var convertedMembers = string.IsNullOrEmpty(options.RootNamespace)
Expand Down Expand Up @@ -1674,10 +1678,11 @@ public override CSharpSyntaxNode VisitBinaryExpression(VBSyntax.BinaryExpression
var lhs = _typeConversionAnalyzer.AddExplicitConversion(node.Left, (ExpressionSyntax)node.Left.Accept(TriviaConvertingVisitor));
var rhs = _typeConversionAnalyzer.AddExplicitConversion(node.Right, (ExpressionSyntax)node.Right.Accept(TriviaConvertingVisitor));

var stringType = _semanticModel.Compilation.GetTypeByMetadataName("System.String");
var lhsTypeInfo = _semanticModel.GetTypeInfo(node.Left);
var rhsTypeInfo = _semanticModel.GetTypeInfo(node.Right);

if (node.IsKind(VBasic.SyntaxKind.ConcatenateExpression)) {
var stringType = _semanticModel.Compilation.GetTypeByMetadataName("System.String");
var lhsTypeInfo = _semanticModel.GetTypeInfo(node.Left);
var rhsTypeInfo = _semanticModel.GetTypeInfo(node.Right);
if (lhsTypeInfo.Type.SpecialType != SpecialType.System_String &&
lhsTypeInfo.ConvertedType.SpecialType != SpecialType.System_String &&
rhsTypeInfo.Type.SpecialType != SpecialType.System_String &&
Expand All @@ -1686,13 +1691,48 @@ public override CSharpSyntaxNode VisitBinaryExpression(VBSyntax.BinaryExpression
}
}

if (node.IsKind(VBasic.SyntaxKind.EqualsExpression, VBasic.SyntaxKind.NotEqualsExpression)) {
if (lhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String &&
rhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String) {
bool lhsEmpty = lhs is LiteralExpressionSyntax les &&
(les.IsKind(SyntaxKind.NullLiteralExpression) ||
(les.IsKind(SyntaxKind.StringLiteralExpression) && string.IsNullOrEmpty(les.Token.ValueText)));
bool lhsLiteral = lhs is LiteralExpressionSyntax && lhs.IsKind(SyntaxKind.StringLiteralExpression);
bool rhsEmpty = rhs is LiteralExpressionSyntax res &&
(res.IsKind(SyntaxKind.NullLiteralExpression) ||
(res.IsKind(SyntaxKind.StringLiteralExpression) && string.IsNullOrEmpty(res.Token.ValueText)));
bool rhsLiteral = rhs is LiteralExpressionSyntax && rhs.IsKind(SyntaxKind.StringLiteralExpression);

if (lhsEmpty || rhsEmpty) {
var arg = lhsEmpty ? rhs : lhs;
var nullOrEmpty = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.IdentifierName("string"), SyntaxFactory.IdentifierName("IsNullOrEmpty")),
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[] { SyntaxFactory.Argument(arg) })));
return node.IsKind(VBasic.SyntaxKind.EqualsExpression) ? (CSharpSyntaxNode)nullOrEmpty : SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, nullOrEmpty);
} else if (!_optionCompareText && (lhsLiteral || rhsLiteral)) {
// If either side is a literal, and we're in binary comparison mode, we can use normal comparison logic
} else {
_extraUsingDirectives.Add("Microsoft.VisualBasic.CompilerServices");
var textCompare = SyntaxFactory.Argument(SyntaxFactory.NameColon("TextCompare"),
default(SyntaxToken),
_optionCompareText ? SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression) : SyntaxFactory.LiteralExpression(SyntaxKind.FalseLiteralExpression));
var compareString = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.IdentifierName("Operators"), SyntaxFactory.IdentifierName("CompareString")),
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[] { SyntaxFactory.Argument(lhs), SyntaxFactory.Argument(rhs), textCompare })));
lhs = compareString;
rhs = SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(0));
}
}
}

if (node.IsKind(VBasic.SyntaxKind.ExponentiateExpression,
VBasic.SyntaxKind.ExponentiateAssignmentStatement)) {
return SyntaxFactory.InvocationExpression(
SyntaxFactory.ParseExpression($"{nameof(Math)}.{nameof(Math.Pow)}"),
ExpressionSyntaxExtensions.CreateArgList(lhs, rhs));
}


var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken(TokenContext.Local);
var op = SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind));

Expand Down
7 changes: 7 additions & 0 deletions TestData/SelfVerifyingTests/VBToCS/ExpressionTests.vb
View file
Open in desktop
Expand Up @@ -24,6 +24,13 @@ Module Program
Assert.Equal(x, 3.5D)
End Sub

<Fact>
Public Sub TestStringComparison()
Dim s1 As String = Nothing
Dim s2 As String = ""
Assert.True(s1 = s2)
End Sub

<Fact>
Sub TestCat()
Dim vBoolean As Boolean = Nothing
Expand Down
103 changes: 100 additions & 3 deletions Tests/CSharp/ExpressionTests.cs
View file
Open in desktop
Expand Up @@ -642,6 +642,103 @@ private void TestMethod()
}");
}

[Fact]
public void StringCompare()
{
TestConversionVisualBasicToCSharpWithoutComments(@"Public Class Class1
Sub Foo()
Dim s1 As String = Nothing
Dim s2 As String = """"
If s1 <> s2 Then
Throw New Exception()
End If
If s1 = ""something"" Then
Throw New Exception()
End If
If ""something"" = s1 Then
Throw New Exception()
End If
If s1 = Nothing Then
'
End If
If s1 = """" Then
'
End If
End Sub
End Class", @"using System;
using Microsoft.VisualBasic.CompilerServices;

public class Class1
{
public void Foo()
{
string s1 = null;
string s2 = """";
if (Operators.CompareString(s1, s2, TextCompare: false) != 0)
throw new Exception();
if (s1 == ""something"")
throw new Exception();
if (""something"" == s1)
throw new Exception();
if (string.IsNullOrEmpty(s1))
{
}
if (string.IsNullOrEmpty(s1))
{
}
}
}");
}

[Fact]
public void StringCompareText()
{
TestConversionVisualBasicToCSharpWithoutComments(@"Option Compare Text
Public Class Class1
Sub Foo()
Dim s1 As String = Nothing
Dim s2 As String = """"
If s1 <> s2 Then
Throw New Exception()
End If
If s1 = ""something"" Then
Throw New Exception()
End If
If ""something"" = s1 Then
Throw New Exception()
End If
If s1 = Nothing Then
'
End If
If s1 = """" Then
'
End If
End Sub
End Class", @"using System;
using Microsoft.VisualBasic.CompilerServices;

public class Class1
{
public void Foo()
{
string s1 = null;
string s2 = """";
if (Operators.CompareString(s1, s2, TextCompare: true) != 0)
throw new Exception();
if (Operators.CompareString(s1, ""something"", TextCompare: true) == 0)
throw new Exception();
if (Operators.CompareString(""something"", s1, TextCompare: true) == 0)
throw new Exception();
if (string.IsNullOrEmpty(s1))
{
}
if (string.IsNullOrEmpty(s1))
{
}
}
}");
}

[Fact]
public void StringConcatPrecedence()
{
Expand Down Expand Up @@ -912,7 +1009,7 @@ End Sub
{
private void TestMethod(string str)
{
bool result = (str == """") ? true : false;
bool result = (string.IsNullOrEmpty(str)) ? true : false;
}
}");
}
Expand All @@ -928,7 +1025,7 @@ End Sub
{
private void TestMethod(string str)
{
bool result = !((str == """") ? true : false);
bool result = !((string.IsNullOrEmpty(str)) ? true : false);
}
}");
}
Expand All @@ -944,7 +1041,7 @@ End Sub
{
private void TestMethod(string str)
{
int result = 5 - ((str == """") ? 1 : 2);
int result = 5 - ((string.IsNullOrEmpty(str)) ? 1 : 2);
}
}");
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/CSharp/MemberTests.cs
View file
Open in desktop
Expand Up @@ -175,7 +175,7 @@ public string Y
{
set
{
if (value != """")
if (!string.IsNullOrEmpty(value))
Y = """";
else
_y = """";
Expand Down
8 changes: 5 additions & 3 deletions Tests/CSharp/StatementTests.cs
View file
Open in desktop
Expand Up @@ -732,19 +732,21 @@ End If
Next
Return -1
End Function
End Class", @"class TestClass
End Class", @"using Microsoft.VisualBasic.CompilerServices;

class TestClass
{
public static int FindTextInCol(string w, int pTitleRow, int startCol, string needle)
{
var loopTo = w.Length;
for (int c = startCol; c <= loopTo; c++)
{
if (needle == """")
if (string.IsNullOrEmpty(needle))
{
if (string.IsNullOrWhiteSpace(w[c].ToString()))
return c;
}
else if (w[c].ToString() == needle)
else if (Operators.CompareString(w[c].ToString(), needle, TextCompare: false) == 0)
return c;
}
return -1;
Expand Down