From 10fe1ea0ee793c65f268925c337f5ba45faf1c09 Mon Sep 17 00:00:00 2001 From: Dominik Baran Date: Tue, 12 Apr 2022 23:38:22 +0200 Subject: [PATCH] Use string.IsNullOrEmpty when comparing string to string.Empty --- CHANGELOG.md | 1 + .../CSharp/VisualBasicEqualityComparison.cs | 28 +++++++++++--- .../CSharp/ExpressionTests/ExpressionTests.cs | 37 +++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e344eac3d..45fd23de9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * Convert immediately executed lambdas without causing a compiler error [#869](https://github.com/icsharpcode/CodeConverter/issues/869) * Fix binary expressions for nullable types in VB->C# conversion [#840](https://github.com/icsharpcode/CodeConverter/issues/840) +* Use string.IsNullOrEmpty when comparing string to string.Empty [#874] (https://github.com/icsharpcode/CodeConverter/issues/874) ### C# -> VB diff --git a/CodeConverter/CSharp/VisualBasicEqualityComparison.cs b/CodeConverter/CSharp/VisualBasicEqualityComparison.cs index b08050abc..54ffc458c 100644 --- a/CodeConverter/CSharp/VisualBasicEqualityComparison.cs +++ b/CodeConverter/CSharp/VisualBasicEqualityComparison.cs @@ -84,7 +84,8 @@ public static RequiredType GetObjectEqualityType(params TypeInfo[] typeInfos) private static bool IsNonEmptyStringLiteral(VBSyntax.ExpressionSyntax vbExpr) { - return vbExpr.SkipIntoParens().IsKind(VBSyntaxKind.StringLiteralExpression) && vbExpr is VBSyntax.LiteralExpressionSyntax literal && !IsEmptyString(literal); + vbExpr = vbExpr.SkipIntoParens(); + return vbExpr.IsKind(VBSyntaxKind.StringLiteralExpression) && vbExpr is VBSyntax.LiteralExpressionSyntax literal && !IsEmptyString(literal); } public ExpressionSyntax VbCoerceToNonNullString(VBSyntax.ExpressionSyntax vbNode, ExpressionSyntax csNode, TypeInfo typeInfo, bool knownNotNull = false) @@ -97,8 +98,8 @@ public ExpressionSyntax VbCoerceToNonNullString(VBSyntax.ExpressionSyntax vbNode return csNode; } - csNode = typeInfo.Type.SpecialType == SpecialType.System_String - ? SyntaxFactory.ParenthesizedExpression(Coalesce(csNode, EmptyStringExpression())) + csNode = typeInfo.Type?.SpecialType == SpecialType.System_String + ? Coalesce(csNode, EmptyStringExpression()).AddParens() : Coalesce(csNode, EmptyCharArrayExpression()); return VbCoerceToString(csNode, typeInfo); @@ -106,7 +107,7 @@ public ExpressionSyntax VbCoerceToNonNullString(VBSyntax.ExpressionSyntax vbNode public static ExpressionSyntax VbCoerceToString(ExpressionSyntax csNode, TypeInfo typeInfo) { - return typeInfo.Type.SpecialType switch + return typeInfo.Type?.SpecialType switch { SpecialType.System_String => csNode, SpecialType.System_Char => SyntaxFactory.InvocationExpression(ValidSyntaxFactory.MemberAccess(csNode, nameof(ToString))), @@ -188,11 +189,26 @@ public bool TryConvertToNullOrEmptyCheck(VBSyntax.BinaryExpressionSyntax node, E private bool IsNothingOrEmpty(VBSyntax.ExpressionSyntax expressionSyntax) { + expressionSyntax = expressionSyntax.SkipIntoParens(); + if (expressionSyntax is VBSyntax.LiteralExpressionSyntax les && - (les.IsKind(VBSyntaxKind.NothingLiteralExpression) || - IsEmptyString(les))) return true; + (les.IsKind(VBSyntaxKind.NothingLiteralExpression) || IsEmptyString(les))) { + return true; + } + + bool IsStringEmptyExpression() => + expressionSyntax is VBSyntax.MemberAccessExpressionSyntax memberAccess && memberAccess.IsKind(VBSyntaxKind.SimpleMemberAccessExpression) && + memberAccess.Expression is VBSyntax.PredefinedTypeSyntax predefinedType && predefinedType.Keyword.IsKind(VBSyntaxKind.StringKeyword) && + memberAccess.Name.Identifier.ValueText.Equals("Empty", StringComparison.OrdinalIgnoreCase); + + if (IsStringEmptyExpression()) { + return true; + } + var constantAnalysis = _semanticModel.GetConstantValue(expressionSyntax); return constantAnalysis.HasValue && (constantAnalysis.Value == null || constantAnalysis.Value as string == ""); + + } private static bool IsEmptyString(VBSyntax.LiteralExpressionSyntax les) diff --git a/Tests/CSharp/ExpressionTests/ExpressionTests.cs b/Tests/CSharp/ExpressionTests/ExpressionTests.cs index 3776c997b..dda07e04b 100644 --- a/Tests/CSharp/ExpressionTests/ExpressionTests.cs +++ b/Tests/CSharp/ExpressionTests/ExpressionTests.cs @@ -6,6 +6,43 @@ namespace ICSharpCode.CodeConverter.Tests.CSharp.ExpressionTests; public class ExpressionTests : ConverterTestBase { + [Fact] + public async Task ComparingStringsUsesCoerceToNonNullOnlyWhenNeededAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Class TestClass + Private Sub TestMethod(a as String) + Dim result = a = ("""") + result = """" = a + result = a = (String.Empty) + result = String.Empty = a + result = a = (Nothing) + result = Nothing = a + result = a Is Nothing + result = a IsNot Nothing + result = a = a + result = a = (""test"") + result = ""test"" = a + End Sub +End Class", @" +internal partial class TestClass +{ + private void TestMethod(string a) + { + bool result = string.IsNullOrEmpty(a); + result = string.IsNullOrEmpty(a); + result = string.IsNullOrEmpty(a); + result = string.IsNullOrEmpty(a); + result = string.IsNullOrEmpty(a); + result = string.IsNullOrEmpty(a); + result = a is null; + result = a is not null; + result = (a ?? """") == (a ?? """"); + result = a == ""test""; + result = ""test"" == a; + } +}"); + } + [Fact] public async Task ConversionOfNotUsesParensIfNeededAsync() {