Address CA1862 cases to diagnose and to fix and improve messages

src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpRecommendCaseInsensitiveStringComparisonFixer.cs

@@ -17,70 +17,92 @@ namespace Microsoft.NetCore.CSharp.Analyzers.Performance
     [ExportCodeFixProvider(LanguageNames.CSharp), Shared]
     public sealed class CSharpRecommendCaseInsensitiveStringComparisonFixer : RecommendCaseInsensitiveStringComparisonFixer
     {
-        protected override IEnumerable<SyntaxNode> GetNewArgumentsForInvocation(SyntaxGenerator generator, string caseChangingApproachValue, IInvocationOperation mainInvocationOperation,
-            INamedTypeSymbol stringComparisonType, out SyntaxNode? mainInvocationInstance)
+        protected override IEnumerable<SyntaxNode> GetNewArgumentsForInvocation(SyntaxGenerator generator,
+            string caseChangingApproachValue, IInvocationOperation mainInvocationOperation,
+            INamedTypeSymbol stringType, INamedTypeSymbol stringComparisonType,
+            string? leftOffendingMethod, string? rightOffendingMethod, out SyntaxNode? mainInvocationInstance)
         {
-            List<SyntaxNode> arguments = new();
-            bool isAnyArgumentNamed = false;
-
             InvocationExpressionSyntax invocationExpression = (InvocationExpressionSyntax)mainInvocationOperation.Syntax;
 
-            bool isChangingCaseInArgument = false;
             mainInvocationInstance = null;
 
             if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression)
             {
-                ExpressionSyntax internalExpression = memberAccessExpression.Expression is ParenthesizedExpressionSyntax parenthesizedExpression ?
-                    parenthesizedExpression.Expression :
-                    memberAccessExpression.Expression;
+                ExpressionSyntax internalExpression = memberAccessExpression.Expression;
+                while (internalExpression is ParenthesizedExpressionSyntax parenthesizedExpression)
+                {
+                    internalExpression = parenthesizedExpression.Expression;
+                }
 
-                if (internalExpression is InvocationExpressionSyntax internalInvocationExpression &&
-                    internalInvocationExpression.Expression is MemberAccessExpressionSyntax internalMemberAccessExpression)
+                if (leftOffendingMethod != null &&
+                    internalExpression is InvocationExpressionSyntax internalInvocationExpression &&
+                    internalInvocationExpression.Expression is MemberAccessExpressionSyntax internalMemberAccessExpression &&
+                    internalMemberAccessExpression.Name.Identifier.Text == leftOffendingMethod)
                 {
+                    // We know we have an instance invocation that is an offending method, retrieve just the instance
                     mainInvocationInstance = internalMemberAccessExpression.Expression;
                 }
                 else
                 {
                     mainInvocationInstance = memberAccessExpression.Expression;
-                    isChangingCaseInArgument = true;
                 }
             }
 
-            foreach (ArgumentSyntax node in invocationExpression.ArgumentList.Arguments)
-            {
-                string? argumentName = node.NameColon?.Name.Identifier.ValueText;
-                isAnyArgumentNamed |= argumentName != null;
+            List<SyntaxNode> arguments = new();
+            bool isAnyArgumentNamed = false;
 
-                ExpressionSyntax argumentExpression = node.Expression is ParenthesizedExpressionSyntax argumentParenthesizedExpression ?
-                    argumentParenthesizedExpression.Expression :
-                    node.Expression;
+            foreach (IArgumentOperation arg in mainInvocationOperation.Arguments)
+            {
+                SyntaxNode newArgumentNode;
 
-                MemberAccessExpressionSyntax? argumentMemberAccessExpression = null;
-                if (argumentExpression is InvocationExpressionSyntax argumentInvocationExpression)
+                // When accessing the main invocation operation arguments, the bottom operation is retrieved
+                // so we need to go up until we find the actual argument syntax ancestor, and skip through any
+                // parenthesized syntax nodes
+                SyntaxNode actualArgumentNode = arg.Syntax;
+                while (actualArgumentNode is not ArgumentSyntax)
                 {
-                    argumentMemberAccessExpression = argumentInvocationExpression.Expression as MemberAccessExpressionSyntax;
+                    actualArgumentNode = actualArgumentNode.Parent!;
                 }
 
-                SyntaxNode newArgumentNode;
-                if (isChangingCaseInArgument)
+                string? argumentName = ((ArgumentSyntax)actualArgumentNode).NameColon?.Name.Identifier.ValueText;
+                isAnyArgumentNamed |= argumentName != null;
+
+                // The arguments could be named and out of order, so we need to detect the string parameter
+                // and remove the offending invocation if there is one
+                if (rightOffendingMethod != null && arg.Parameter?.Type?.Name == stringType.Name)
                 {
-                    if (argumentMemberAccessExpression != null)
+                    ExpressionSyntax? desiredExpression = null;
+                    if (arg.Syntax is ArgumentSyntax argumentExpression)
+                    {
+                        desiredExpression = argumentExpression.Expression;
+                        while (desiredExpression is ParenthesizedExpressionSyntax parenthesizedExpression)
+                        {
+                            desiredExpression = parenthesizedExpression.Expression;
+                        }
+                    }
+                    else if (arg.Syntax is InvocationExpressionSyntax argumentInvocationExpression)
+                    {
+                        desiredExpression = argumentInvocationExpression;
+                    }
+
+                    if (desiredExpression is InvocationExpressionSyntax invocation &&
+                        invocation.Expression is MemberAccessExpressionSyntax argumentMemberAccessExpression)
                     {
                         newArgumentNode = argumentName == RCISCAnalyzer.StringParameterName ?
-                            generator.Argument(RCISCAnalyzer.StringParameterName, RefKind.None, argumentMemberAccessExpression.Expression) :
-                            generator.Argument(argumentMemberAccessExpression.Expression);
+                                generator.Argument(RCISCAnalyzer.StringParameterName, RefKind.None, argumentMemberAccessExpression.Expression) :
+                                generator.Argument(argumentMemberAccessExpression.Expression);
                     }
                     else
                     {
-                        newArgumentNode = node;
+                        newArgumentNode = arg.Syntax;
                     }
                 }
                 else
                 {
-                    newArgumentNode = node;
+                    newArgumentNode = arg.Syntax;
                 }
 
-                arguments.Add(newArgumentNode.WithTriviaFrom(node));
+                arguments.Add(newArgumentNode.WithTriviaFrom(arg.Syntax));
             }
 
             Debug.Assert(mainInvocationInstance != null);

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -2010,19 +2010,19 @@ Widening and user defined conversions are not supported with generic types.</val
     <value>Prefer an 'IsEmpty' check rather than using 'Any()', both for clarity and for performance</value>
   </data>
   <data name="RecommendCaseInsensitiveStringComparerStringComparisonCodeFixTitle" xml:space="preserve">
-    <value>Use the 'string.{0}(string, StringComparison)' overload</value>
+    <value>Consider using the 'string.{0}(string, StringComparison)' overload</value>
   </data>
   <data name="RecommendCaseInsensitiveStringEqualsCodeFixTitle" xml:space="preserve">
-    <value>Use 'string.Equals(string, StringComparison)'</value>
+    <value>Consider using '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>
+    <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. Switching to using 'StringComparer' might cause subtle changes in behavior, so it's important to conduct thorough testing after applying the suggestion. Additionally, if a culturally sensitive comparison is not required, consider using 'StringComparer.OrdinalIgnoreCase'.</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>
+    <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. Switching to using an overload that takes a 'StringComparison' might cause subtle changes in behavior, so it's important to conduct thorough testing after applying the suggestion. Additionally, if a culturally sensitive comparison is not required, consider using 'StringComparison.OrdinalIgnoreCase'.</value>
   </data>
     <data name="RecommendCaseInsensitiveStringEqualsMessage" xml:space="preserve">
     <value>Prefer using 'string.Equals(string, StringComparison)' to perform a case-insensitive comparison</value>
@@ -2031,7 +2031,7 @@ Widening and user defined conversions are not supported with generic types.</val
     <value>Prefer the 'StringComparison' method overloads to perform case-insensitive string comparisons</value>
   </data>
   <data name="RecommendCaseInsensitiveStringComparisonDescription" xml:space="preserve">
-    <value>Avoid calling 'ToLower', 'ToUpper', 'ToLowerInvariant' and 'ToUpperInvariant' to perform case-insensitive string comparisons because they lead to an allocation. Instead, prefer calling the method overloads of 'Contains', 'IndexOf' and 'StartsWith' that take a 'StringComparison' enum value to perform case-insensitive comparisons.</value>
+    <value>Avoid calling 'ToLower', 'ToUpper', 'ToLowerInvariant' and 'ToUpperInvariant' to perform case-insensitive string comparisons because they lead to an allocation. Instead, prefer calling the method overloads of 'Contains', 'IndexOf' and 'StartsWith' that take a 'StringComparison' enum value to perform case-insensitive comparisons. Switching to using an overload that takes a 'StringComparison' might cause subtle changes in behavior, so it's important to conduct thorough testing after applying the suggestion. Additionally, if a culturally sensitive comparison is not required, consider using 'StringComparison.OrdinalIgnoreCase'.</value>
   </data>
   <data name="RecommendCaseInsensitiveStringComparisonMessage" xml:space="preserve">
     <value>Prefer the string comparison method overload of '{0}' that takes a 'StringComparison' enum value to perform a case-insensitive comparison</value>