Skip to content

Commit 91867c2

Browse files
Merge remote-tracking branch 'mrmonday/308-string-comparisons'
2 parents 999b7b3 + d108f30 commit 91867c2

5 files changed

Lines changed: 156 additions & 10 deletions

File tree

ICSharpCode.CodeConverter/CSharp/NodesVisitor.cs

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class NodesVisitor : VBasic.VisualBasicSyntaxVisitor<CSharpSyntaxNode>
3737
private static readonly Type ExtensionAttributeType = typeof(ExtensionAttribute);
3838
private readonly TypeConversionAnalyzer _typeConversionAnalyzer;
3939
public CommentConvertingNodesVisitor TriviaConvertingVisitor { get; }
40+
private bool _optionCompareText = false;
4041

4142
private CommonConversions CommonConversions { get; }
4243

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

110+
_optionCompareText = node.Options.Any(x => x.NameKeyword.ValueText.Equals("Compare", StringComparison.OrdinalIgnoreCase) &&
111+
x.ValueKeyword.ValueText.Equals("Text", StringComparison.OrdinalIgnoreCase));
112+
109113
var attributes = SyntaxFactory.List(node.Attributes.SelectMany(a => a.AttributeLists).SelectMany(ConvertAttribute));
110114
var sourceAndConverted = node.Members.Select(m => (Source: m, Converted: ConvertMember(m))).ToReadOnlyCollection();
111115
var convertedMembers = string.IsNullOrEmpty(options.RootNamespace)
@@ -1675,10 +1679,11 @@ public override CSharpSyntaxNode VisitBinaryExpression(VBSyntax.BinaryExpression
16751679
var lhs = _typeConversionAnalyzer.AddExplicitConversion(node.Left, (ExpressionSyntax)node.Left.Accept(TriviaConvertingVisitor));
16761680
var rhs = _typeConversionAnalyzer.AddExplicitConversion(node.Right, (ExpressionSyntax)node.Right.Accept(TriviaConvertingVisitor));
16771681

1682+
var stringType = _semanticModel.Compilation.GetTypeByMetadataName("System.String");
1683+
var lhsTypeInfo = _semanticModel.GetTypeInfo(node.Left);
1684+
var rhsTypeInfo = _semanticModel.GetTypeInfo(node.Right);
1685+
16781686
if (node.IsKind(VBasic.SyntaxKind.ConcatenateExpression)) {
1679-
var stringType = _semanticModel.Compilation.GetTypeByMetadataName("System.String");
1680-
var lhsTypeInfo = _semanticModel.GetTypeInfo(node.Left);
1681-
var rhsTypeInfo = _semanticModel.GetTypeInfo(node.Right);
16821687
if (lhsTypeInfo.Type.SpecialType != SpecialType.System_String &&
16831688
lhsTypeInfo.ConvertedType.SpecialType != SpecialType.System_String &&
16841689
rhsTypeInfo.Type.SpecialType != SpecialType.System_String &&
@@ -1687,13 +1692,48 @@ public override CSharpSyntaxNode VisitBinaryExpression(VBSyntax.BinaryExpression
16871692
}
16881693
}
16891694

1695+
if (node.IsKind(VBasic.SyntaxKind.EqualsExpression, VBasic.SyntaxKind.NotEqualsExpression)) {
1696+
if (lhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String &&
1697+
rhsTypeInfo.ConvertedType?.SpecialType == SpecialType.System_String) {
1698+
bool lhsEmpty = lhs is LiteralExpressionSyntax les &&
1699+
(les.IsKind(SyntaxKind.NullLiteralExpression) ||
1700+
(les.IsKind(SyntaxKind.StringLiteralExpression) && string.IsNullOrEmpty(les.Token.ValueText)));
1701+
bool lhsLiteral = lhs is LiteralExpressionSyntax && lhs.IsKind(SyntaxKind.StringLiteralExpression);
1702+
bool rhsEmpty = rhs is LiteralExpressionSyntax res &&
1703+
(res.IsKind(SyntaxKind.NullLiteralExpression) ||
1704+
(res.IsKind(SyntaxKind.StringLiteralExpression) && string.IsNullOrEmpty(res.Token.ValueText)));
1705+
bool rhsLiteral = rhs is LiteralExpressionSyntax && rhs.IsKind(SyntaxKind.StringLiteralExpression);
1706+
1707+
if (lhsEmpty || rhsEmpty) {
1708+
var arg = lhsEmpty ? rhs : lhs;
1709+
var nullOrEmpty = SyntaxFactory.InvocationExpression(
1710+
SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.IdentifierName("string"), SyntaxFactory.IdentifierName("IsNullOrEmpty")),
1711+
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[] { SyntaxFactory.Argument(arg) })));
1712+
return node.IsKind(VBasic.SyntaxKind.EqualsExpression) ? (CSharpSyntaxNode)nullOrEmpty : SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, nullOrEmpty);
1713+
} else if (!_optionCompareText && (lhsLiteral || rhsLiteral)) {
1714+
// If either side is a literal, and we're in binary comparison mode, we can use normal comparison logic
1715+
} else {
1716+
_extraUsingDirectives.Add("Microsoft.VisualBasic.CompilerServices");
1717+
var textCompare = SyntaxFactory.Argument(SyntaxFactory.NameColon("TextCompare"),
1718+
default(SyntaxToken),
1719+
_optionCompareText ? SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression) : SyntaxFactory.LiteralExpression(SyntaxKind.FalseLiteralExpression));
1720+
var compareString = SyntaxFactory.InvocationExpression(
1721+
SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.IdentifierName("Operators"), SyntaxFactory.IdentifierName("CompareString")),
1722+
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[] { SyntaxFactory.Argument(lhs), SyntaxFactory.Argument(rhs), textCompare })));
1723+
lhs = compareString;
1724+
rhs = SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(0));
1725+
}
1726+
}
1727+
}
1728+
16901729
if (node.IsKind(VBasic.SyntaxKind.ExponentiateExpression,
16911730
VBasic.SyntaxKind.ExponentiateAssignmentStatement)) {
16921731
return SyntaxFactory.InvocationExpression(
16931732
SyntaxFactory.ParseExpression($"{nameof(Math)}.{nameof(Math.Pow)}"),
16941733
ExpressionSyntaxExtensions.CreateArgList(lhs, rhs));
16951734
}
16961735

1736+
16971737
var kind = VBasic.VisualBasicExtensions.Kind(node).ConvertToken(TokenContext.Local);
16981738
var op = SyntaxFactory.Token(CSharpUtil.GetExpressionOperatorTokenKind(kind));
16991739

TestData/SelfVerifyingTests/VBToCS/ExpressionTests.vb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ Module Program
2424
Assert.Equal(x, 3.5D)
2525
End Sub
2626

27+
<Fact>
28+
Public Sub TestStringComparison()
29+
Dim s1 As String = Nothing
30+
Dim s2 As String = ""
31+
Assert.True(s1 = s2)
32+
End Sub
33+
2734
<Fact>
2835
Sub TestCat()
2936
Dim vBoolean As Boolean = Nothing

Tests/CSharp/ExpressionTests.cs

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,103 @@ private void TestMethod()
722722
}");
723723
}
724724

725+
[Fact]
726+
public void StringCompare()
727+
{
728+
TestConversionVisualBasicToCSharpWithoutComments(@"Public Class Class1
729+
Sub Foo()
730+
Dim s1 As String = Nothing
731+
Dim s2 As String = """"
732+
If s1 <> s2 Then
733+
Throw New Exception()
734+
End If
735+
If s1 = ""something"" Then
736+
Throw New Exception()
737+
End If
738+
If ""something"" = s1 Then
739+
Throw New Exception()
740+
End If
741+
If s1 = Nothing Then
742+
'
743+
End If
744+
If s1 = """" Then
745+
'
746+
End If
747+
End Sub
748+
End Class", @"using System;
749+
using Microsoft.VisualBasic.CompilerServices;
750+
751+
public class Class1
752+
{
753+
public void Foo()
754+
{
755+
string s1 = null;
756+
string s2 = """";
757+
if (Operators.CompareString(s1, s2, TextCompare: false) != 0)
758+
throw new Exception();
759+
if (s1 == ""something"")
760+
throw new Exception();
761+
if (""something"" == s1)
762+
throw new Exception();
763+
if (string.IsNullOrEmpty(s1))
764+
{
765+
}
766+
if (string.IsNullOrEmpty(s1))
767+
{
768+
}
769+
}
770+
}");
771+
}
772+
773+
[Fact]
774+
public void StringCompareText()
775+
{
776+
TestConversionVisualBasicToCSharpWithoutComments(@"Option Compare Text
777+
Public Class Class1
778+
Sub Foo()
779+
Dim s1 As String = Nothing
780+
Dim s2 As String = """"
781+
If s1 <> s2 Then
782+
Throw New Exception()
783+
End If
784+
If s1 = ""something"" Then
785+
Throw New Exception()
786+
End If
787+
If ""something"" = s1 Then
788+
Throw New Exception()
789+
End If
790+
If s1 = Nothing Then
791+
'
792+
End If
793+
If s1 = """" Then
794+
'
795+
End If
796+
End Sub
797+
End Class", @"using System;
798+
using Microsoft.VisualBasic.CompilerServices;
799+
800+
public class Class1
801+
{
802+
public void Foo()
803+
{
804+
string s1 = null;
805+
string s2 = """";
806+
if (Operators.CompareString(s1, s2, TextCompare: true) != 0)
807+
throw new Exception();
808+
if (Operators.CompareString(s1, ""something"", TextCompare: true) == 0)
809+
throw new Exception();
810+
if (Operators.CompareString(""something"", s1, TextCompare: true) == 0)
811+
throw new Exception();
812+
if (string.IsNullOrEmpty(s1))
813+
{
814+
}
815+
if (string.IsNullOrEmpty(s1))
816+
{
817+
}
818+
}
819+
}");
820+
}
821+
725822
[Fact]
726823
public void StringConcatPrecedence()
727824
{
@@ -992,7 +1089,7 @@ End Sub
9921089
{
9931090
private void TestMethod(string str)
9941091
{
995-
bool result = (str == """") ? true : false;
1092+
bool result = (string.IsNullOrEmpty(str)) ? true : false;
9961093
}
9971094
}");
9981095
}
@@ -1008,7 +1105,7 @@ End Sub
10081105
{
10091106
private void TestMethod(string str)
10101107
{
1011-
bool result = !((str == """") ? true : false);
1108+
bool result = !((string.IsNullOrEmpty(str)) ? true : false);
10121109
}
10131110
}");
10141111
}
@@ -1024,7 +1121,7 @@ End Sub
10241121
{
10251122
private void TestMethod(string str)
10261123
{
1027-
int result = 5 - ((str == """") ? 1 : 2);
1124+
int result = 5 - ((string.IsNullOrEmpty(str)) ? 1 : 2);
10281125
}
10291126
}");
10301127
}

Tests/CSharp/MemberTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public string Y
175175
{
176176
set
177177
{
178-
if (value != """")
178+
if (!string.IsNullOrEmpty(value))
179179
Y = """";
180180
else
181181
_y = """";

Tests/CSharp/StatementTests.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -732,19 +732,21 @@ End If
732732
Next
733733
Return -1
734734
End Function
735-
End Class", @"class TestClass
735+
End Class", @"using Microsoft.VisualBasic.CompilerServices;
736+
737+
class TestClass
736738
{
737739
public static int FindTextInCol(string w, int pTitleRow, int startCol, string needle)
738740
{
739741
var loopTo = w.Length;
740742
for (int c = startCol; c <= loopTo; c++)
741743
{
742-
if (needle == """")
744+
if (string.IsNullOrEmpty(needle))
743745
{
744746
if (string.IsNullOrWhiteSpace(w[c].ToString()))
745747
return c;
746748
}
747-
else if (w[c].ToString() == needle)
749+
else if (Operators.CompareString(w[c].ToString(), needle, TextCompare: false) == 0)
748750
return c;
749751
}
750752
return -1;

0 commit comments

Comments
 (0)