Recommend using Equals with StringComparison instead of string.ToLowe…

…r() == otherString.ToLower() (dotnet#6720) * Analyzer: Use string.Format with CultureInfo for title. * Almost, only needs to handle inequality * Fix inequality case, fix trivia, still need to add C# test * Add C# tests for trivia and string method * Add tests for diagnostic-only, no fix, for Equals that don't match variant/invariant. * Use ValueTuple in tests * Register operation actions separately. * Fix recent nullability changes * Use simplified valuetuple creation in tests * Add extra rule and resource messages for the 'string.Method() == string.Method()' comparison case. * msbuild /t:pack * Address suggestion
amiru3f · Jul 26, 2023 · 175a259 · 175a259
1 parent d2cfc7a
commit 175a259
Show file tree

Hide file tree

Showing 24 changed files with 749 additions and 181 deletions.
diff --git a/...soft.NetCore.Analyzers/Performance/CSharpRecommendCaseInsensitiveStringComparisonFixer.cs b/...soft.NetCore.Analyzers/Performance/CSharpRecommendCaseInsensitiveStringComparisonFixer.cs
@@ -17,30 +17,27 @@ namespace Microsoft.NetCore.CSharp.Analyzers.Performance
     [ExportCodeFixProvider(LanguageNames.CSharp), Shared]
     public sealed class CSharpRecommendCaseInsensitiveStringComparisonFixer : RecommendCaseInsensitiveStringComparisonFixer
     {
-        protected override List<SyntaxNode> GetNewArguments(SyntaxGenerator generator, IInvocationOperation mainInvocationOperation,
+        protected override IEnumerable<SyntaxNode> GetNewArgumentsForInvocation(SyntaxGenerator generator, string caseChangingApproachValue, IInvocationOperation mainInvocationOperation,
             INamedTypeSymbol stringComparisonType, out SyntaxNode? mainInvocationInstance)
         {
             List<SyntaxNode> arguments = new();
             bool isAnyArgumentNamed = false;
 
             InvocationExpressionSyntax invocationExpression = (InvocationExpressionSyntax)mainInvocationOperation.Syntax;
 
-            string? caseChangingApproachName = null;
             bool isChangingCaseInArgument = false;
-
             mainInvocationInstance = null;
 
             if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression)
             {
-                var internalExpression = memberAccessExpression.Expression is ParenthesizedExpressionSyntax parenthesizedExpression ?
+                ExpressionSyntax internalExpression = memberAccessExpression.Expression is ParenthesizedExpressionSyntax parenthesizedExpression ?
                     parenthesizedExpression.Expression :
                     memberAccessExpression.Expression;
 
                 if (internalExpression is InvocationExpressionSyntax internalInvocationExpression &&
                     internalInvocationExpression.Expression is MemberAccessExpressionSyntax internalMemberAccessExpression)
                 {
                     mainInvocationInstance = internalMemberAccessExpression.Expression;
-                    caseChangingApproachName = GetCaseChangingApproach(internalMemberAccessExpression.Name.Identifier.ValueText);
                 }
                 else
                 {
@@ -59,10 +56,9 @@ protected override List<SyntaxNode> GetNewArguments(SyntaxGenerator generator, I
                     node.Expression;
 
                 MemberAccessExpressionSyntax? argumentMemberAccessExpression = null;
-                if (argumentExpression is InvocationExpressionSyntax argumentInvocationExpression &&
-                    (argumentMemberAccessExpression = argumentInvocationExpression.Expression as MemberAccessExpressionSyntax) != null)
+                if (argumentExpression is InvocationExpressionSyntax argumentInvocationExpression)
                 {
-                    caseChangingApproachName = GetCaseChangingApproach(argumentMemberAccessExpression.Name.Identifier.ValueText);
+                    argumentMemberAccessExpression = argumentInvocationExpression.Expression as MemberAccessExpressionSyntax;
                 }
 
                 SyntaxNode newArgumentNode;
@@ -84,18 +80,23 @@ protected override List<SyntaxNode> GetNewArguments(SyntaxGenerator generator, I
                     newArgumentNode = node;
                 }
 
-                arguments.Add(newArgumentNode);
+                arguments.Add(newArgumentNode.WithTriviaFrom(node));
             }
 
-            Debug.Assert(caseChangingApproachName != null);
             Debug.Assert(mainInvocationInstance != null);
 
-            SyntaxNode stringComparisonArgument = GetNewStringComparisonArgument(generator,
-                stringComparisonType, caseChangingApproachName!, isAnyArgumentNamed);
+            SyntaxNode stringComparisonArgument = GetNewStringComparisonArgument(generator, stringComparisonType, caseChangingApproachValue, isAnyArgumentNamed);
 
             arguments.Add(stringComparisonArgument);
 
             return arguments;
         }
+
+        protected override IEnumerable<SyntaxNode> GetNewArgumentsForBinary(SyntaxGenerator generator, SyntaxNode rightNode, SyntaxNode typeMemberAccess) =>
+            new List<SyntaxNode>()
+            {
+                generator.Argument(rightNode),
+                generator.Argument(typeMemberAccess)
+            };
     }
 }
diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx
@@ -2066,14 +2066,20 @@ Widening and user defined conversions are not supported with generic types.</val
   <data name="RecommendCaseInsensitiveStringComparerStringComparisonCodeFixTitle" xml:space="preserve">
     <value>Use the 'string.{0}(string, StringComparison)' overload</value>
   </data>
-  <data name="RecommendCaseInsensitiveStringComparerTitle" xml:space="preserve">
-    <value>Prefer using 'StringComparer' to perform case-insensitive string comparisons</value>
+  <data name="RecommendCaseInsensitiveStringEqualsCodeFixTitle" xml:space="preserve">
+    <value>Use 'string.Equals(string, StringComparison)'</value>
   </data>
   <data name="RecommendCaseInsensitiveStringComparerDescription" xml:space="preserve">
     <value>Avoid calling 'ToLower', 'ToUpper', 'ToLowerInvariant' and 'ToUpperInvariant' to perform case-insensitive string comparisons when using 'CompareTo', because they lead to an allocation. Instead, use 'StringComparer' to perform case-insensitive comparisons.</value>
   </data>
   <data name="RecommendCaseInsensitiveStringComparerMessage" xml:space="preserve">
     <value>Prefer using 'StringComparer' to perform a case-insensitive comparison</value>
+  </data>
+    <data name="RecommendCaseInsensitiveStringEqualsDescription" xml:space="preserve">
+    <value>Avoid calling 'ToLower', 'ToUpper', 'ToLowerInvariant' and 'ToUpperInvariant' to perform case-insensitive string comparisons, as in 'string.ToLower() == string.ToLower()', because they lead to an allocation. Instead, use 'string.Equals(string, StringComparison)' to perform case-insensitive comparisons.</value>
+  </data>
+    <data name="RecommendCaseInsensitiveStringEqualsMessage" xml:space="preserve">
+    <value>Prefer using 'string.Equals(string, StringComparison)' to perform a case-insensitive comparison</value>
   </data>
   <data name="RecommendCaseInsensitiveStringComparisonTitle" xml:space="preserve">
     <value>Prefer the 'StringComparison' method overloads to perform case-insensitive string comparisons</value>