Merge pull request #2764 from dotnet/feature/catch_isymbol_equals_wit…

…hout_comparer

Enforce that symbol comparison should use an equality comparer

src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs

@@ -1,8 +1,10 @@
 // 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.Linq;
 using System.Collections.Immutable;
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
+using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
 
@@ -16,6 +18,8 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer
         private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyDescription), CodeAnalysisDiagnosticsResources.ResourceManager, typeof(CodeAnalysisDiagnosticsResources));
 
         private static readonly string s_symbolTypeFullName = typeof(ISymbol).FullName;
+        private const string s_symbolEqualsName = nameof(ISymbol.Equals);
+        public const string SymbolEqualityComparerName = "Microsoft.CodeAnalysis.SymbolEqualityComparer";
 
         public static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
             DiagnosticIds.CompareSymbolsCorrectlyRuleId,
@@ -43,11 +47,29 @@ public override void Initialize(AnalysisContext context)
                     return;
                 }
 
-                context.RegisterOperationAction(context => HandleBinaryOperator(in context, symbolType), OperationKind.BinaryOperator);
+                // Check that the EqualityComparer exists and can be used, otherwise the Roslyn version
+                // being used it too low to need the change for method references
+                var operatorsToHandle = UseSymbolEqualityComparer(context.Compilation) ?
+                    new[] { OperationKind.BinaryOperator, OperationKind.Invocation } :
+                    new[] { OperationKind.BinaryOperator };
+
+                context.RegisterOperationAction(context => HandleOperation(in context, symbolType), operatorsToHandle);
             });
         }
 
-        private void HandleBinaryOperator(in OperationAnalysisContext context, INamedTypeSymbol symbolType)
+        private void HandleOperation(in OperationAnalysisContext context, INamedTypeSymbol symbolType)
+        {
+            if (context.Operation is IBinaryOperation)
+            {
+                HandleBinaryOperator(in context, symbolType);
+            }
+            else if (context.Operation is IInvocationOperation)
+            {
+                HandleInvocationOperation(in context, symbolType);
+            }
+        }
+
+        private static void HandleBinaryOperator(in OperationAnalysisContext context, INamedTypeSymbol symbolType)
         {
             var binary = (IBinaryOperation)context.Operation;
             if (binary.OperatorKind != BinaryOperatorKind.Equals && binary.OperatorKind != BinaryOperatorKind.NotEquals)
@@ -90,19 +112,32 @@ private void HandleBinaryOperator(in OperationAnalysisContext context, INamedTyp
             context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(Rule));
         }
 
-        private static bool IsSymbolType(IOperation operation, INamedTypeSymbol symbolType)
+        private static void HandleInvocationOperation(in OperationAnalysisContext context, INamedTypeSymbol symbolType)
         {
-            if (operation.Type is object)
+            var invocationOperation = (IInvocationOperation)context.Operation;
+            var method = invocationOperation.TargetMethod;
+            if (method.Name != s_symbolEqualsName)
             {
-                if (operation.Type.Equals(symbolType))
-                {
-                    return true;
-                }
+                return;
+            }
 
-                if (operation.Type.AllInterfaces.Contains(symbolType))
-                {
-                    return true;
-                }
+            if (invocationOperation.Instance != null && !IsSymbolType(invocationOperation.Instance, symbolType))
+            {
+                return;
+            }
+
+            var parameters = invocationOperation.Arguments;
+            if (parameters.All(p => IsSymbolType(p.Value, symbolType)))
+            {
+                context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(Rule));
+            }
+        }
+
+        private static bool IsSymbolType(IOperation operation, INamedTypeSymbol symbolType)
+        {
+            if (operation.Type is object && IsSymbolType(operation.Type, symbolType))
+            {
+                return true;
             }
 
             if (operation is IConversionOperation conversion)
@@ -113,6 +148,26 @@ private static bool IsSymbolType(IOperation operation, INamedTypeSymbol symbolTy
             return false;
         }
 
+        private static bool IsSymbolType(ITypeSymbol typeSymbol, INamedTypeSymbol symbolType)
+        {
+            if (typeSymbol == null)
+            {
+                return false;
+            }
+
+            if (typeSymbol.Equals(symbolType))
+            {
+                return true;
+            }
+
+            if (typeSymbol.AllInterfaces.Contains(symbolType))
+            {
+                return true;
+            }
+
+            return false;
+        }
+
         private static bool IsSymbolClassType(IOperation operation)
         {
             if (operation.Type is object)
@@ -146,5 +201,8 @@ private static bool IsExplicitCastToObject(IOperation operation)
 
             return conversion.Type?.SpecialType == SpecialType.System_Object;
         }
+
+        public static bool UseSymbolEqualityComparer(Compilation compilation)
+        => compilation.GetTypeByMetadataName(SymbolEqualityComparerName) is object;
     }
 }

src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs

@@ -2,6 +2,7 @@
 
 using System.Collections.Immutable;
 using System.Composition;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeActions;
@@ -41,27 +42,82 @@ private async Task<Document> ConvertToEqualsAsync(Document document, TextSpan so
             var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
             var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
             var expression = root.FindNode(sourceSpan, getInnermostNodeForTie: true);
-            if (!(semanticModel.GetOperation(expression, cancellationToken) is IBinaryOperation operation))
+            var rawOperation = semanticModel.GetOperation(expression, cancellationToken);
+
+            return rawOperation switch
+            {
+                IBinaryOperation binaryOperation => await ConvertToEqualsAsync(document, semanticModel, binaryOperation, cancellationToken).ConfigureAwait(false),
+                IInvocationOperation invocationOperation => await EnsureEqualsCorrectAsync(document, semanticModel, invocationOperation, cancellationToken).ConfigureAwait(false),
+                _ => document
+            };
+        }
+
+        private static async Task<Document> EnsureEqualsCorrectAsync(Document document, SemanticModel semanticModel, IInvocationOperation invocationOperation, CancellationToken cancellationToken)
+        {
+            if (!CompareSymbolsCorrectlyAnalyzer.UseSymbolEqualityComparer(semanticModel.Compilation))
             {
                 return document;
             }
 
             var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
             var generator = editor.Generator;
 
-            var replacement = generator.InvocationExpression(
-                generator.MemberAccessExpression(
-                    generator.TypeExpression(semanticModel.Compilation.GetSpecialType(SpecialType.System_Object)),
-                    nameof(object.Equals)),
-                operation.LeftOperand.Syntax.WithoutLeadingTrivia(),
-                operation.RightOperand.Syntax.WithoutTrailingTrivia());
-            if (operation.OperatorKind == BinaryOperatorKind.NotEquals)
+            if (invocationOperation.Instance is null)
+            {
+                var replacement = generator.InvocationExpression(
+                                    GetEqualityComparerDefaultEquals(generator),
+                                    invocationOperation.Arguments.Select(argument => argument.Syntax).ToImmutableArray());
+
+                editor.ReplaceNode(invocationOperation.Syntax, replacement.WithTriviaFrom(invocationOperation.Syntax));
+            }
+            else
+            {
+                var replacement = generator.AddParameters(invocationOperation.Syntax, new[] { GetEqualityComparerDefault(generator) });
+                editor.ReplaceNode(invocationOperation.Syntax, replacement.WithTriviaFrom(invocationOperation.Syntax));
+            }
+
+            return editor.GetChangedDocument();
+        }
+
+        private static async Task<Document> ConvertToEqualsAsync(Document document, SemanticModel semanticModel, IBinaryOperation binaryOperation, CancellationToken cancellationToken)
+        {
+
+            var expression = binaryOperation.Syntax;
+            var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+            var generator = editor.Generator;
+
+            var replacement = CompareSymbolsCorrectlyAnalyzer.UseSymbolEqualityComparer(semanticModel.Compilation) switch
+            {
+                true =>
+                    generator.InvocationExpression(
+                        GetEqualityComparerDefaultEquals(generator),
+                        binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(),
+                        binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia()),
+
+                false =>
+                    generator.InvocationExpression(
+                        generator.MemberAccessExpression(
+                            generator.TypeExpression(semanticModel.Compilation.GetSpecialType(SpecialType.System_Object)),
+                            nameof(object.Equals)),
+                        binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(),
+                        binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia())
+            };
+
+            if (binaryOperation.OperatorKind == BinaryOperatorKind.NotEquals)
             {
                 replacement = generator.LogicalNotExpression(replacement);
             }
 
             editor.ReplaceNode(expression, replacement.WithTriviaFrom(expression));
             return editor.GetChangedDocument();
         }
+
+        private static SyntaxNode GetEqualityComparerDefaultEquals(SyntaxGenerator generator)
+            => generator.MemberAccessExpression(
+                    GetEqualityComparerDefault(generator),
+                    nameof(object.Equals));
+
+        private static SyntaxNode GetEqualityComparerDefault(SyntaxGenerator generator)
+            => generator.MemberAccessExpression(generator.DottedName(CompareSymbolsCorrectlyAnalyzer.SymbolEqualityComparerName), "Default");
     }
 }

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Symboly by se měly porovnat podle rovnosti, ne podle identity.</target>
+        <target state="needs-review-translation">Symboly by se měly porovnat podle rovnosti, ne podle identity.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Symbole sollten hinsichtlich Gleichheit verglichen werden, nicht hinsichtlich Identität.</target>
+        <target state="needs-review-translation">Symbole sollten hinsichtlich Gleichheit verglichen werden, nicht hinsichtlich Identität.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Solo se debe realizar una comparación de igualdad de los símbolos, no de identidad.</target>
+        <target state="needs-review-translation">Solo se debe realizar una comparación de igualdad de los símbolos, no de identidad.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">La comparaison des symboles doit porter sur l'égalité, pas sur l'identité.</target>
+        <target state="needs-review-translation">La comparaison des symboles doit porter sur l'égalité, pas sur l'identité.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">È necessario confrontare i simboli per verificarne l'uguaglianza, non l'identità.</target>
+        <target state="needs-review-translation">È necessario confrontare i simboli per verificarne l'uguaglianza, non l'identità.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">シンボルは、ID ではなく等値で比較する必要があります。</target>
+        <target state="needs-review-translation">シンボルは、ID ではなく等値で比較する必要があります。</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">기호는 ID가 아니라 같은지 비교해야 합니다.</target>
+        <target state="needs-review-translation">기호는 ID가 아니라 같은지 비교해야 합니다.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Symbole powinny być porównywane pod kątem równości, a nie tożsamości.</target>
+        <target state="needs-review-translation">Symbole powinny być porównywane pod kątem równości, a nie tożsamości.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Os símbolos devem ser comparados quanto à igualdade, não à identidade.</target>
+        <target state="needs-review-translation">Os símbolos devem ser comparados quanto à igualdade, não à identidade.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Символы необходимо сравнивать на равенство, а не на тождественность.</target>
+        <target state="needs-review-translation">Символы необходимо сравнивать на равенство, а не на тождественность.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">Semboller kimlik değil eşitlik için karşılaştırılmalıdır.</target>
+        <target state="needs-review-translation">Semboller kimlik değil eşitlik için karşılaştırılmalıdır.</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">符号应比较相等性，而不是标识。</target>
+        <target state="needs-review-translation">符号应比较相等性，而不是标识。</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">

src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf

@@ -9,7 +9,7 @@
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyDescription">
         <source>Symbols should be compared for equality, not identity.</source>
-        <target state="translated">應比較符號是否相等，而非比較身分識別。</target>
+        <target state="needs-review-translation">應比較符號是否相等，而非比較身分識別。</target>
         <note />
       </trans-unit>
       <trans-unit id="CompareSymbolsCorrectlyMessage">