Merge pull request #3686 from Youssef1313/patch-4

Fix TestForEmptyStringsUsingStringLength.Fixer when empty literal is used

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLength.Fixer.cs

@@ -5,9 +5,10 @@
 
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.Formatting;
+using System.Threading;
 
 namespace Microsoft.NetCore.Analyzers.Runtime
 {
@@ -37,7 +38,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
             SemanticModel model = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
 
-            FixResolution? resolution = TryGetFixResolution(binaryExpressionSyntax, model);
+            FixResolution? resolution = TryGetFixResolution(binaryExpressionSyntax, model, context.CancellationToken);
 
             if (resolution != null)
             {
@@ -55,18 +56,18 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
             }
         }
 
-        private FixResolution? TryGetFixResolution(SyntaxNode binaryExpressionSyntax, SemanticModel model)
+        private FixResolution? TryGetFixResolution(SyntaxNode binaryExpressionSyntax, SemanticModel model, CancellationToken cancellationToken)
         {
             bool isEqualsOperator = IsEqualsOperator(binaryExpressionSyntax);
             SyntaxNode leftOperand = GetLeftOperand(binaryExpressionSyntax);
             SyntaxNode rightOperand = GetRightOperand(binaryExpressionSyntax);
 
-            if (ContainsSystemStringEmpty(leftOperand, model))
+            if (ContainsSystemStringEmpty(leftOperand, model) || ContainsEmptyStringLiteral(leftOperand, model, cancellationToken))
             {
                 return new FixResolution(binaryExpressionSyntax, rightOperand, isEqualsOperator);
             }
 
-            if (ContainsSystemStringEmpty(rightOperand, model))
+            if (ContainsSystemStringEmpty(rightOperand, model) || ContainsEmptyStringLiteral(rightOperand, model, cancellationToken))
             {
                 return new FixResolution(binaryExpressionSyntax, leftOperand, isEqualsOperator);
             }
@@ -135,6 +136,10 @@ private async Task<Document> ConvertToStringLengthComparison(CodeFixContext cont
             return editor.GetChangedDocument();
         }
 
+        private static bool ContainsEmptyStringLiteral(SyntaxNode node, SemanticModel model, CancellationToken cancellationToken)
+            => model.GetConstantValue(node, cancellationToken) is Optional<object> optionalValue &&
+            optionalValue.HasValue && optionalValue.Value is string value && value.Length == 0;
+
         protected abstract SyntaxNode GetBinaryExpression(SyntaxNode node);
         protected abstract bool IsEqualsOperator(SyntaxNode node);
         protected abstract bool IsNotEqualsOperator(SyntaxNode node);
@@ -155,4 +160,4 @@ public FixResolution(SyntaxNode binaryExpressionSyntax, SyntaxNode comparisonOpe
             }
         }
     }
-}
+}

src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/TestForEmptyStringsUsingStringLengthTests.Fixer.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
+using Test.Utilities;
 using Xunit;
 using VerifyCS = Test.Utilities.CSharpCodeFixVerifier<
     Microsoft.NetCore.Analyzers.Runtime.TestForEmptyStringsUsingStringLengthAnalyzer,
@@ -14,6 +15,132 @@ namespace Microsoft.NetCore.Analyzers.Runtime.UnitTests
     public class TestForEmptyStringsUsingStringLengthFixerTests
     {
         private const int c_StringLengthCodeActionIndex = 1;
+
+        [Fact, WorkItem(3686, "https://github.com/dotnet/roslyn-analyzers/pull/3686")]
+        public async Task CA1820_FixTestEmptyStringsUsingIsNullOrEmpty_WhenStringIsLiteral()
+        {
+            await VerifyCS.VerifyCodeFixAsync(@"
+public class A
+{
+    public bool Compare(string s)
+    {
+        return [|s == """"|];
+    }
+
+    public bool CompareEmptyIsLeft(string s)
+    {
+        return [|"""" == s|];
+    }
+}
+", @"
+public class A
+{
+    public bool Compare(string s)
+    {
+        return string.IsNullOrEmpty(s);
+    }
+
+    public bool CompareEmptyIsLeft(string s)
+    {
+        return string.IsNullOrEmpty(s);
+    }
+}
+");
+            await VerifyVB.VerifyCodeFixAsync(@"
+Public Class A
+    Public Function Compare(s As String) As Boolean
+        Return [|s = """"|]
+    End Function
+
+    Public Function CompareEmptyIsLeft(s As String) As Boolean
+        Return [|"""" = s|]
+    End Function
+End Class
+", @"
+Public Class A
+    Public Function Compare(s As String) As Boolean
+        Return String.IsNullOrEmpty(s)
+    End Function
+
+    Public Function CompareEmptyIsLeft(s As String) As Boolean
+        Return String.IsNullOrEmpty(s)
+    End Function
+End Class
+");
+        }
+
+        [Fact]
+        public async Task CA1820_FixTestEmptyStringsUsingStringLength_WhenStringIsLiteral()
+        {
+            await new VerifyCS.Test
+            {
+                TestState =
+                {
+                    Sources =
+                    {
+                        @"
+public class A
+{
+    public bool Compare(string s)
+    {
+        return [|s == """"|];
+    }
+}
+",
+                    },
+                },
+                FixedState =
+                {
+                    Sources =
+                    {
+                        @"
+public class A
+{
+    public bool Compare(string s)
+    {
+        return s.Length == 0;
+    }
+}
+",
+                    },
+                },
+                CodeActionIndex = c_StringLengthCodeActionIndex,
+                CodeActionEquivalenceKey = "TestForEmptyStringCorrectlyUsingStringLength",
+            }.RunAsync();
+
+            await new VerifyVB.Test
+            {
+                TestState =
+                {
+                    Sources =
+                    {
+                        @"
+Public Class A
+    Public Function Compare(s As String) As Boolean
+        Return [|s = """"|]
+    End Function
+End Class
+",
+                    },
+                },
+                FixedState =
+                {
+                    Sources =
+                    {
+                        @"
+Public Class A
+    Public Function Compare(s As String) As Boolean
+        Return s.Length = 0
+    End Function
+End Class
+",
+                    },
+                },
+                CodeActionIndex = c_StringLengthCodeActionIndex,
+                CodeActionEquivalenceKey = "TestForEmptyStringCorrectlyUsingStringLength",
+            }.RunAsync();
+        }
+
         [Fact]
         public async Task CA1820_FixTestEmptyStringsUsingIsNullOrEmpty()
         {
@@ -877,4 +1004,4 @@ End Class
             }.RunAsync();
         }
     }
-}
+}