Merge pull request #5116 from NewellClark/#45552-use-string-equals-ov…

…er-string-compare

#45552 use string equals over string compare

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -14,6 +14,7 @@ CA1844 | Performance | Info | ProvideStreamMemoryBasedAsyncOverrides, [Documenta
 CA1845 | Performance | Info | UseSpanBasedStringConcat, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1845)
 CA1846 | Performance | Info | PreferAsSpanOverSubstring, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1846)
 CA2250 | Usage | Info | UseCancellationTokenThrowIfCancellationRequested, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2250)
+CA2251 | Usage | Hidden | UseStringEqualsOverStringCompare, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2251)
 
 ### Removed Rules
 

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

@@ -1635,4 +1635,16 @@
   <data name="DoNotUseWhenAllWithSingleTaskFix" xml:space="preserve">
     <value>Replace 'WhenAll' call with argument</value>
   </data>
+  <data name="UseStringEqualsOverStringCompareCodeFixTitle" xml:space="preserve">
+    <value>Use 'string.Equals'</value>
+  </data>
+  <data name="UseStringEqualsOverStringCompareDescription" xml:space="preserve">
+    <value>It is both clearer and likely faster to use 'string.Equals' instead of comparing the result of 'string.Compare' to zero.</value>
+  </data>
+  <data name="UseStringEqualsOverStringCompareMessage" xml:space="preserve">
+    <value>Use 'string.Equals' instead of comparing the result of 'string.Compare' to 0</value>
+  </data>
+  <data name="UseStringEqualsOverStringCompareTitle" xml:space="preserve">
+    <value>Use 'string.Equals'</value>
+  </data>
 </root>

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

@@ -0,0 +1,215 @@
+// 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;
+using System.Collections.Immutable;
+using System.Composition;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Analyzer.Utilities;
+using Analyzer.Utilities.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Editing;
+using Microsoft.CodeAnalysis.Operations;
+
+using Resx = Microsoft.NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources;
+using RequiredSymbols = Microsoft.NetCore.Analyzers.Runtime.UseStringEqualsOverStringCompare.RequiredSymbols;
+
+namespace Microsoft.NetCore.Analyzers.Runtime
+{
+    [ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
+    public sealed class UseStringEqualsOverStringCompareFixer : CodeFixProvider
+    {
+        public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(UseStringEqualsOverStringCompare.RuleId);
+
+        public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            var document = context.Document;
+            var token = context.CancellationToken;
+            var semanticModel = await document.GetSemanticModelAsync(token).ConfigureAwait(false);
+
+            _ = RequiredSymbols.TryGetSymbols(semanticModel.Compilation, out var symbols);
+            RoslynDebug.Assert(symbols is not null);
+
+            var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false);
+            var node = root.FindNode(context.Span, getInnermostNodeForTie: true);
+            if (semanticModel.GetOperation(node, token) is not IBinaryOperation violation)
+                return;
+
+            //  Get the replacer that applies to the reported violation.
+            var replacer = GetOperationReplacers(symbols).First(x => x.IsMatch(violation));
+
+            var codeAction = CodeAction.Create(
+                Resx.UseStringEqualsOverStringCompareCodeFixTitle,
+                CreateChangedDocument,
+                nameof(Resx.UseStringEqualsOverStringCompareCodeFixTitle));
+            context.RegisterCodeFix(codeAction, context.Diagnostics);
+            return;
+
+            //  Local functions
+
+            async Task<Document> CreateChangedDocument(CancellationToken token)
+            {
+                var editor = await DocumentEditor.CreateAsync(document, token).ConfigureAwait(false);
+                var replacementNode = replacer.CreateReplacementExpression(violation, editor.Generator);
+                editor.ReplaceNode(violation.Syntax, replacementNode);
+
+                return editor.GetChangedDocument();
+            }
+        }
+
+        public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+        private static ImmutableArray<OperationReplacer> GetOperationReplacers(RequiredSymbols symbols)
+        {
+            return ImmutableArray.Create<OperationReplacer>(
+                new StringStringCaseReplacer(symbols),
+                new StringStringBoolReplacer(symbols),
+                new StringStringStringComparisonReplacer(symbols));
+        }
+
+        /// <summary>
+        /// Base class for an object that generate the replacement code for a reported violation.
+        /// </summary>
+        private abstract class OperationReplacer
+        {
+            protected OperationReplacer(RequiredSymbols symbols)
+            {
+                Symbols = symbols;
+            }
+
+            protected RequiredSymbols Symbols { get; }
+
+            /// <summary>
+            /// Indicates whether the current <see cref="OperationReplacer"/> applies to the specified violation.
+            /// </summary>
+            /// <param name="violation">The <see cref="IBinaryOperation"/> at the location reported by the analyzer.</param>
+            /// <returns>True if the current <see cref="OperationReplacer"/> applies to the specified violation.</returns>
+            public abstract bool IsMatch(IBinaryOperation violation);
+
+            /// <summary>
+            /// Creates a replacement node for a violation that the current <see cref="OperationReplacer"/> applies to.
+            /// Asserts if the current <see cref="OperationReplacer"/> does not apply to the specified violation.
+            /// </summary>
+            /// <param name="violation">The <see cref="IBinaryOperation"/> obtained at the location reported by the analyzer.
+            /// <see cref="IsMatch(IBinaryOperation)"/> must return <see langword="true"/> for this operation.</param>
+            /// <param name="generator"></param>
+            /// <returns></returns>
+            public abstract SyntaxNode CreateReplacementExpression(IBinaryOperation violation, SyntaxGenerator generator);
+
+            protected SyntaxNode CreateEqualsMemberAccess(SyntaxGenerator generator)
+            {
+                var stringTypeExpression = generator.TypeExpressionForStaticMemberAccess(Symbols.StringType);
+                return generator.MemberAccessExpression(stringTypeExpression, nameof(string.Equals));
+            }
+
+            protected static IInvocationOperation GetInvocation(IBinaryOperation violation)
+            {
+                var result = UseStringEqualsOverStringCompare.GetInvocationFromEqualityCheckWithLiteralZero(violation);
+
+                RoslynDebug.Assert(result is not null);
+
+                return result;
+            }
+
+            protected static SyntaxNode InvertIfNotEquals(SyntaxNode stringEqualsInvocationExpression, IBinaryOperation equalsOrNotEqualsOperation, SyntaxGenerator generator)
+            {
+                return equalsOrNotEqualsOperation.OperatorKind is BinaryOperatorKind.NotEquals ?
+                    generator.LogicalNotExpression(stringEqualsInvocationExpression) :
+                    stringEqualsInvocationExpression;
+            }
+        }
+
+        /// <summary>
+        /// Replaces <see cref="string.Compare(string, string)"/> violations.
+        /// </summary>
+        private sealed class StringStringCaseReplacer : OperationReplacer
+        {
+            public StringStringCaseReplacer(RequiredSymbols symbols)
+                : base(symbols)
+            { }
+
+            public override bool IsMatch(IBinaryOperation violation) => UseStringEqualsOverStringCompare.IsStringStringCase(violation, Symbols);
+
+            public override SyntaxNode CreateReplacementExpression(IBinaryOperation violation, SyntaxGenerator generator)
+            {
+                RoslynDebug.Assert(IsMatch(violation));
+
+                var compareInvocation = GetInvocation(violation);
+                var equalsInvocationSyntax = generator.InvocationExpression(
+                    CreateEqualsMemberAccess(generator),
+                    compareInvocation.Arguments.GetArgumentsInParameterOrder().Select(x => x.Value.Syntax));
+
+                return InvertIfNotEquals(equalsInvocationSyntax, violation, generator);
+            }
+        }
+
+        /// <summary>
+        /// Replaces <see cref="string.Compare(string, string, bool)"/> violations.
+        /// </summary>
+        private sealed class StringStringBoolReplacer : OperationReplacer
+        {
+            public StringStringBoolReplacer(RequiredSymbols symbols)
+                : base(symbols)
+            { }
+
+            public override bool IsMatch(IBinaryOperation violation) => UseStringEqualsOverStringCompare.IsStringStringBoolCase(violation, Symbols);
+
+            public override SyntaxNode CreateReplacementExpression(IBinaryOperation violation, SyntaxGenerator generator)
+            {
+                RoslynDebug.Assert(IsMatch(violation));
+
+                var compareInvocation = GetInvocation(violation);
+
+                //  We know that the 'ignoreCase' argument in 'string.Compare(string, string, bool)' is a boolean literal
+                //  because we've asserted that 'IsMatch' returns true.
+                var ignoreCaseLiteral = (ILiteralOperation)compareInvocation.Arguments.GetArgumentForParameterAtIndex(2).Value;
+
+                //  If the violation contains a call to 'string.Compare(x, y, true)' then we
+                //  replace it with a call to 'string.Equals(x, y, StringComparison.CurrentCultureIgnoreCase)'.
+                //  If the violation contains a call to 'string.Compare(x, y, false)' then we
+                //  replace it with a call to 'string.Equals(x, y, StringComparison.CurrentCulture)'. 
+                var stringComparisonEnumMemberName = (bool)ignoreCaseLiteral.ConstantValue.Value ?
+                    nameof(StringComparison.CurrentCultureIgnoreCase) :
+                    nameof(StringComparison.CurrentCulture);
+                var stringComparisonMemberAccessSyntax = generator.MemberAccessExpression(
+                    generator.TypeExpressionForStaticMemberAccess(Symbols.StringComparisonType),
+                    stringComparisonEnumMemberName);
+
+                var equalsInvocationSyntax = generator.InvocationExpression(
+                    CreateEqualsMemberAccess(generator),
+                    compareInvocation.Arguments.GetArgumentForParameterAtIndex(0).Value.Syntax,
+                    compareInvocation.Arguments.GetArgumentForParameterAtIndex(1).Value.Syntax,
+                    stringComparisonMemberAccessSyntax);
+
+                return InvertIfNotEquals(equalsInvocationSyntax, violation, generator);
+            }
+        }
+
+        /// <summary>
+        /// Replaces <see cref="string.Compare(string, string, StringComparison)"/> violations.
+        /// </summary>
+        private sealed class StringStringStringComparisonReplacer : OperationReplacer
+        {
+            public StringStringStringComparisonReplacer(RequiredSymbols symbols)
+                : base(symbols)
+            { }
+
+            public override bool IsMatch(IBinaryOperation violation) => UseStringEqualsOverStringCompare.IsStringStringStringComparisonCase(violation, Symbols);
+
+            public override SyntaxNode CreateReplacementExpression(IBinaryOperation violation, SyntaxGenerator generator)
+            {
+                RoslynDebug.Assert(IsMatch(violation));
+
+                var invocation = GetInvocation(violation);
+                var equalsInvocationSyntax = generator.InvocationExpression(
+                    CreateEqualsMemberAccess(generator),
+                    invocation.Arguments.GetArgumentsInParameterOrder().Select(x => x.Value.Syntax));
+
+                return InvertIfNotEquals(equalsInvocationSyntax, violation, generator);
+            }
+        }
+    }
+}