From 08b0b6512b343d2d73544c36aff3558877d9f980 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Tue, 13 Aug 2019 18:07:33 -0700 Subject: [PATCH 01/10] Report a rule when the 'Equals' method is called with only ISymbol --- .../CompareSymbolsCorrectlyAnalyzer.cs | 65 +++++++++++++++---- 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index a1bc0630a3..b97fbb53b0 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -1,5 +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; +using System.Linq; using System.Collections.Immutable; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; @@ -16,6 +18,7 @@ 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 static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( DiagnosticIds.CompareSymbolsCorrectlyRuleId, @@ -43,11 +46,23 @@ public override void Initialize(AnalysisContext context) return; } - context.RegisterOperationAction(context => HandleBinaryOperator(in context, symbolType), OperationKind.BinaryOperator); + context.RegisterOperationAction(context => HandleOperation(in context, symbolType), OperationKind.BinaryOperator, OperationKind.MethodReference); }); } - private void HandleBinaryOperator(in OperationAnalysisContext context, INamedTypeSymbol symbolType) + private void HandleOperation(in OperationAnalysisContext context, INamedTypeSymbol symbolType) + { + if (context.Operation is IBinaryOperation) + { + HandleBinaryOperator(context, symbolType); + } + if (context.Operation is IMethodReferenceOperation) + { + HandleMethodReferenceOperation(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 +105,27 @@ private void HandleBinaryOperator(in OperationAnalysisContext context, INamedTyp context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(Rule)); } + private static void HandleMethodReferenceOperation(OperationAnalysisContext context, INamedTypeSymbol symbolType) + { + var methodReference = (IMethodReferenceOperation)context.Operation; + + if (methodReference.Instance != null && !IsSymbolType(methodReference.Instance, symbolType)) + { + return; + } + + var parameters = methodReference.Method.Parameters; + if (methodReference.Method.Name == s_symbolEqualsName && parameters.All(p => IsSymbolType(p.Type, symbolType))) + { + context.ReportDiagnostic(methodReference.Syntax.GetLocation().CreateDiagnostic(Rule)); + } + } + private static bool IsSymbolType(IOperation operation, INamedTypeSymbol symbolType) { if (operation.Type is object) { - if (operation.Type.Equals(symbolType)) - { - return true; - } - - if (operation.Type.AllInterfaces.Contains(symbolType)) - { - return true; - } + IsSymbolType(operation.Type, symbolType); } if (operation is IConversionOperation conversion) @@ -113,6 +136,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) From fcbe3de39d41275377eec45433ca1b34d1990c6b Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Wed, 14 Aug 2019 12:22:12 -0700 Subject: [PATCH 02/10] Update rule message to reflect new guidance --- .../Core/CodeAnalysisDiagnosticsResources.resx | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf | 4 ++-- .../Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf | 4 ++-- 14 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx b/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx index 8ef051914b..02bc4178d0 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx @@ -339,7 +339,7 @@ Upgrade MSBuildWorkspace - Symbols should be compared for equality, not identity. + Symbols should be compared for equality with an EqualityComparer. Compare symbols correctly diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf index fb5879538f..97d61acd47 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Symboly by se měly porovnat podle rovnosti, ne podle identity. + Symbols should be compared for equality with an EqualityComparer. + Symboly by se měly porovnat podle rovnosti, ne podle identity. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf index 847d030d6f..f0601ed50a 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Symbole sollten hinsichtlich Gleichheit verglichen werden, nicht hinsichtlich Identität. + Symbols should be compared for equality with an EqualityComparer. + Symbole sollten hinsichtlich Gleichheit verglichen werden, nicht hinsichtlich Identität. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf index ae11a9f18e..057ec1e653 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Solo se debe realizar una comparación de igualdad de los símbolos, no de identidad. + Symbols should be compared for equality with an EqualityComparer. + Solo se debe realizar una comparación de igualdad de los símbolos, no de identidad. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf index 0340cd7ae4..229a997ceb 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - La comparaison des symboles doit porter sur l'égalité, pas sur l'identité. + Symbols should be compared for equality with an EqualityComparer. + La comparaison des symboles doit porter sur l'égalité, pas sur l'identité. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf index b925c2d15f..2bee9c782f 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - È necessario confrontare i simboli per verificarne l'uguaglianza, non l'identità. + Symbols should be compared for equality with an EqualityComparer. + È necessario confrontare i simboli per verificarne l'uguaglianza, non l'identità. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf index 6e4a4c023f..b8d97e3be4 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - シンボルは、ID ではなく等値で比較する必要があります。 + Symbols should be compared for equality with an EqualityComparer. + シンボルは、ID ではなく等値で比較する必要があります。 diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf index 6e4762e995..9f9c4f529a 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - 기호는 ID가 아니라 같은지 비교해야 합니다. + Symbols should be compared for equality with an EqualityComparer. + 기호는 ID가 아니라 같은지 비교해야 합니다. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf index e04acaf2d2..622d8308b8 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Symbole powinny być porównywane pod kątem równości, a nie tożsamości. + Symbols should be compared for equality with an EqualityComparer. + Symbole powinny być porównywane pod kątem równości, a nie tożsamości. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf index cf0563d019..a4e0ca6f62 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Os símbolos devem ser comparados quanto à igualdade, não à identidade. + Symbols should be compared for equality with an EqualityComparer. + Os símbolos devem ser comparados quanto à igualdade, não à identidade. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf index 6f5ea23740..93bf6cd7ca 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Символы необходимо сравнивать на равенство, а не на тождественность. + Symbols should be compared for equality with an EqualityComparer. + Символы необходимо сравнивать на равенство, а не на тождественность. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf index 52bf4512cf..b9d48ab130 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - Semboller kimlik değil eşitlik için karşılaştırılmalıdır. + Symbols should be compared for equality with an EqualityComparer. + Semboller kimlik değil eşitlik için karşılaştırılmalıdır. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf index 467e677bfb..ee5ae5573e 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - 符号应比较相等性，而不是标识。 + Symbols should be compared for equality with an EqualityComparer. + 符号应比较相等性，而不是标识。 diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf index 87741bf033..b53d38388a 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf @@ -8,8 +8,8 @@ - Symbols should be compared for equality, not identity. - 應比較符號是否相等，而非比較身分識別。 + Symbols should be compared for equality with an EqualityComparer. + 應比較符號是否相等，而非比較身分識別。 From 69a84ba4eab54fd8e09a4f0111fbc68441a978fc Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Wed, 14 Aug 2019 14:03:19 -0700 Subject: [PATCH 03/10] Add missing return statement --- .../Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index b97fbb53b0..3f6a375616 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -125,7 +125,7 @@ private static bool IsSymbolType(IOperation operation, INamedTypeSymbol symbolTy { if (operation.Type is object) { - IsSymbolType(operation.Type, symbolType); + return IsSymbolType(operation.Type, symbolType); } if (operation is IConversionOperation conversion) From 7c91a285bff1bfd62bdeff302b1c60a7eae9022a Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Mon, 19 Aug 2019 14:20:24 -0700 Subject: [PATCH 04/10] Limit to only if SymbolEquivalenceComparer exists --- .../MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index 3f6a375616..5333888dca 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; @@ -19,6 +20,7 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer private static readonly string s_symbolTypeFullName = typeof(ISymbol).FullName; private const string s_symbolEqualsName = nameof(ISymbol.Equals); + private const string s_symbolEqualityComparerName = "Microsoft.CodeAnalysis.Shared.Utilities.SymbolEquivalenceComparer"; public static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( DiagnosticIds.CompareSymbolsCorrectlyRuleId, @@ -46,7 +48,14 @@ public override void Initialize(AnalysisContext context) return; } - context.RegisterOperationAction(context => HandleOperation(in context, symbolType), OperationKind.BinaryOperator, OperationKind.MethodReference); + // Check that the s_symbolEqualityComparerName exists and can be used, otherwise the Roslyn version + // being used it too low to need the change for method references + var symbolEqualityComparerType = context.Compilation.GetTypeByMetadataName(s_symbolEqualityComparerName); + var operatorsToHandle = symbolEqualityComparerType is null ? + new[] { OperationKind.BinaryOperator } : + new[] { OperationKind.BinaryOperator, OperationKind.MethodReference }; + + context.RegisterOperationAction(context => HandleOperation(in context, symbolType), operatorsToHandle); }); } From df8059de40ace64443883867890f529e5de534f5 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Tue, 20 Aug 2019 12:26:00 -0700 Subject: [PATCH 05/10] Fix logic change to be same as before change --- .../Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index 5333888dca..939b42c915 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -132,9 +132,9 @@ private static void HandleMethodReferenceOperation(OperationAnalysisContext cont private static bool IsSymbolType(IOperation operation, INamedTypeSymbol symbolType) { - if (operation.Type is object) + if (operation.Type is object && IsSymbolType(operation.Type, symbolType)) { - return IsSymbolType(operation.Type, symbolType); + return true; } if (operation is IConversionOperation conversion) From b2bc75000f705097dda11a10133fb98ad7e8e611 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Tue, 3 Sep 2019 10:42:58 -0700 Subject: [PATCH 06/10] Undo resx change --- .../Core/CodeAnalysisDiagnosticsResources.resx | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf | 2 +- .../Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx b/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx index 02bc4178d0..8ef051914b 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx @@ -339,7 +339,7 @@ Upgrade MSBuildWorkspace - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Compare symbols correctly diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf index 97d61acd47..ef3250a8c2 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Symboly by se měly porovnat podle rovnosti, ne podle identity. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf index f0601ed50a..6d6dd42622 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Symbole sollten hinsichtlich Gleichheit verglichen werden, nicht hinsichtlich Identität. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf index 057ec1e653..e3de66dd20 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Solo se debe realizar una comparación de igualdad de los símbolos, no de identidad. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf index 229a997ceb..3f5697d10a 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. La comparaison des symboles doit porter sur l'égalité, pas sur l'identité. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf index 2bee9c782f..c0b4a56043 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. È necessario confrontare i simboli per verificarne l'uguaglianza, non l'identità. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf index b8d97e3be4..995fc1c9c9 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. シンボルは、ID ではなく等値で比較する必要があります。 diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf index 9f9c4f529a..4604041ed6 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. 기호는 ID가 아니라 같은지 비교해야 합니다. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf index 622d8308b8..41f3d2894b 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Symbole powinny być porównywane pod kątem równości, a nie tożsamości. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf index a4e0ca6f62..ebb54c6299 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Os símbolos devem ser comparados quanto à igualdade, não à identidade. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf index 93bf6cd7ca..e753e2d126 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Символы необходимо сравнивать на равенство, а не на тождественность. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf index b9d48ab130..fe2fda20de 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. Semboller kimlik değil eşitlik için karşılaştırılmalıdır. diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf index ee5ae5573e..568f89eff0 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hans.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. 符号应比较相等性，而不是标识。 diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf index b53d38388a..3addadd2f1 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.zh-Hant.xlf @@ -8,7 +8,7 @@ - Symbols should be compared for equality with an EqualityComparer. + Symbols should be compared for equality, not identity. 應比較符號是否相等，而非比較身分識別。 From 911e3e07d1e42b95821ce7cb7749c50780004123 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Tue, 3 Sep 2019 13:24:21 -0700 Subject: [PATCH 07/10] Add code fix for equality comparer --- .../CompareSymbolsCorrectlyAnalyzer.cs | 5 +- .../Fixers/CompareSymbolsCorrectlyFix.cs | 54 +++++++++++++++++-- .../CompareSymbolsCorrectlyTests.cs | 2 +- 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index 939b42c915..6e10c4e197 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -1,6 +1,5 @@ // 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.Linq; using System.Collections.Immutable; using Analyzer.Utilities; @@ -20,7 +19,7 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer private static readonly string s_symbolTypeFullName = typeof(ISymbol).FullName; private const string s_symbolEqualsName = nameof(ISymbol.Equals); - private const string s_symbolEqualityComparerName = "Microsoft.CodeAnalysis.Shared.Utilities.SymbolEquivalenceComparer"; + public const string SymbolEqualityComparerName = "Microsoft.CodeAnalysis.Shared.Utilities.SymbolEquivalenceComparer"; public static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( DiagnosticIds.CompareSymbolsCorrectlyRuleId, @@ -50,7 +49,7 @@ public override void Initialize(AnalysisContext context) // Check that the s_symbolEqualityComparerName exists and can be used, otherwise the Roslyn version // being used it too low to need the change for method references - var symbolEqualityComparerType = context.Compilation.GetTypeByMetadataName(s_symbolEqualityComparerName); + var symbolEqualityComparerType = context.Compilation.GetTypeByMetadataName(SymbolEqualityComparerName); var operatorsToHandle = symbolEqualityComparerType is null ? new[] { OperationKind.BinaryOperator } : new[] { OperationKind.BinaryOperator, OperationKind.MethodReference }; diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs index 368670b9d2..b02e6f96d1 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs @@ -16,6 +16,8 @@ namespace Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.Fixers [Shared] public class CompareSymbolsCorrectlyFix : CodeFixProvider { + private const string s_equalityComparerIdentifier = "Microsoft.CodeAnalysis.SymbolEqualityComparer.Default"; + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(CompareSymbolsCorrectlyAnalyzer.Rule.Id); public override FixAllProvider GetFixAllProvider() @@ -41,7 +43,19 @@ private async Task 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), + IMethodReferenceOperation methodReferenceOperation => await EnsureEqualsCorrectAsync(document, semanticModel, methodReferenceOperation, cancellationToken).ConfigureAwait(false), + _ => document + }; + } + + private static async Task EnsureEqualsCorrectAsync(Document document, SemanticModel semanticModel, IMethodReferenceOperation methodReference, CancellationToken cancellationToken) + { + if (!UseEqualityComparer(semanticModel.Compilation)) { return document; } @@ -49,13 +63,40 @@ private async Task ConvertToEqualsAsync(Document document, TextSpan so var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); var generator = editor.Generator; + var replacement = generator.AddParameters(methodReference.Syntax, new[] { generator.IdentifierName(s_equalityComparerIdentifier) }); + editor.ReplaceNode(methodReference.Syntax, replacement.WithTriviaFrom(methodReference.Syntax)); + return editor.GetChangedDocument(); + } + + private static async Task 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 parameters = UseEqualityComparer(semanticModel.Compilation) switch + { + true => new[] { + binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(), + binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia(), + generator.IdentifierName(s_equalityComparerIdentifier) + }, + + false => + new[] { + binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(), + binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia() + } + }; + 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) + parameters); + + if (binaryOperation.OperatorKind == BinaryOperatorKind.NotEquals) { replacement = generator.LogicalNotExpression(replacement); } @@ -63,5 +104,10 @@ private async Task ConvertToEqualsAsync(Document document, TextSpan so editor.ReplaceNode(expression, replacement.WithTriviaFrom(expression)); return editor.GetChangedDocument(); } + + private static bool UseEqualityComparer(Compilation compilation) + { + return compilation.GetTypeByMetadataName(CompareSymbolsCorrectlyAnalyzer.SymbolEqualityComparerName) is object; + } } } diff --git a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs index 95d110012a..5de0f50e20 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs @@ -230,7 +230,7 @@ class TestClass {{ using Microsoft.CodeAnalysis; class TestClass {{ bool Method(Symbol x, {symbolType} y) {{ - return Equals(x, y); + return Equals(x, y, Microsoft.CodeAnalysis.SymbolEqualityComparer.Default); }} }} "; From 115349a9a08f2459292aa2361cfd797366f4b5ea Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Tue, 3 Sep 2019 16:26:07 -0700 Subject: [PATCH 08/10] update code fixer to handle when comparer should be used --- .../Fixers/CompareSymbolsCorrectlyFix.cs | 32 ++-- .../CompareSymbolsCorrectlyTests.cs | 140 +++++++++++++++++- 2 files changed, 154 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs index b02e6f96d1..c5e872a9ae 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs @@ -16,7 +16,7 @@ namespace Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.Fixers [Shared] public class CompareSymbolsCorrectlyFix : CodeFixProvider { - private const string s_equalityComparerIdentifier = "Microsoft.CodeAnalysis.SymbolEqualityComparer.Default"; + private const string s_equalityComparerIdentifier = "Microsoft.CodeAnalysis.SymbolEqualityComparer"; public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(CompareSymbolsCorrectlyAnalyzer.Rule.Id); @@ -75,27 +75,27 @@ private static async Task ConvertToEqualsAsync(Document document, Sema var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); var generator = editor.Generator; - var parameters = UseEqualityComparer(semanticModel.Compilation) switch + var replacement = UseEqualityComparer(semanticModel.Compilation) switch { - true => new[] { + true => + generator.InvocationExpression( + generator.MemberAccessExpression( + generator.MemberAccessExpression( + generator.DottedName(s_equalityComparerIdentifier), + "Default"), + nameof(object.Equals)), binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(), - binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia(), - generator.IdentifierName(s_equalityComparerIdentifier) - }, + binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia()), false => - new[] { + generator.InvocationExpression( + generator.MemberAccessExpression( + generator.TypeExpression(semanticModel.Compilation.GetSpecialType(SpecialType.System_Object)), + nameof(object.Equals)), binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(), - binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia() - } + binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia()) }; - var replacement = generator.InvocationExpression( - generator.MemberAccessExpression( - generator.TypeExpression(semanticModel.Compilation.GetSpecialType(SpecialType.System_Object)), - nameof(object.Equals)), - parameters); - if (binaryOperation.OperatorKind == BinaryOperatorKind.NotEquals) { replacement = generator.LogicalNotExpression(replacement); @@ -107,7 +107,7 @@ private static async Task ConvertToEqualsAsync(Document document, Sema private static bool UseEqualityComparer(Compilation compilation) { - return compilation.GetTypeByMetadataName(CompareSymbolsCorrectlyAnalyzer.SymbolEqualityComparerName) is object; + return compilation.GetTypeByMetadataName(s_equalityComparerIdentifier) is object; } } } diff --git a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs index 5de0f50e20..a70f494a66 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs @@ -145,6 +145,57 @@ End Operator End Class "; + private const string SymbolEqualityComparerStubVisualBasic = +@" +Imports System.Collections.Generic + +Namespace Microsoft + Namespace CodeAnalysis + Public Class SymbolEqualityComparer + Implements IEqualityComparer(Of ISymbol) + + Public Shared ReadOnly [Default] As SymbolEqualityComparer = New SymbolEqualityComparer() + + Private Sub New() + End Sub + + Public Function Equals(x As ISymbol, y As ISymbol) As Boolean Implements IEqualityComparer(Of ISymbol).Equals + Throw New System.NotImplementedException() + End Function + + Public Function GetHashCode(obj As ISymbol) As Integer Implements IEqualityComparer(Of ISymbol).GetHashCode + Throw New System.NotImplementedException() + End Function + End Class + End Namespace +End Namespace"; + + private const string SymbolEqualityComparerStubCSharp = +@" +using System.Collections.Generic; + +namespace Microsoft.CodeAnalysis +{ + public class SymbolEqualityComparer : IEqualityComparer + { + public static readonly SymbolEqualityComparer Default = new SymbolEqualityComparer(); + + private SymbolEqualityComparer() + { + } + + public bool Equals(ISymbol x, ISymbol y) + { + throw new System.NotImplementedException(); + } + + public int GetHashCode(ISymbol obj) + { + throw new System.NotImplementedException(); + } + } +}"; + [Theory] [InlineData(nameof(ISymbol))] [InlineData(nameof(INamedTypeSymbol))] @@ -162,11 +213,38 @@ class TestClass {{ using Microsoft.CodeAnalysis; class TestClass {{ bool Method({symbolType} x, {symbolType} y) {{ - return Equals(x, y); + return SymbolEqualityComparer.Default.Equals(x, y); }} }} "; + await new VerifyCS.Test + { + TestState = { Sources = { source, SymbolEqualityComparerStubCSharp } }, + FixedState = { Sources = { fixedSource, SymbolEqualityComparerStubCSharp } }, + }.RunAsync(); + } + [Theory] + [InlineData(nameof(ISymbol))] + [InlineData(nameof(INamedTypeSymbol))] + public async Task CompareTwoSymbolsEquals_NoComparer_CSharp(string symbolType) + { + var source = $@" +using Microsoft.CodeAnalysis; +class TestClass {{ + bool Method({symbolType} x, {symbolType} y) {{ + return [|x == y|]; + }} +}} +"; + var fixedSource = $@" +using Microsoft.CodeAnalysis; +class TestClass {{ + bool Method({symbolType} x, {symbolType} y) {{ + return SymbolEqualityComparer.Default.Equals(x, y); + }} +}} +"; await VerifyCS.VerifyCodeFixAsync(source, fixedSource); } @@ -230,7 +308,36 @@ class TestClass {{ using Microsoft.CodeAnalysis; class TestClass {{ bool Method(Symbol x, {symbolType} y) {{ - return Equals(x, y, Microsoft.CodeAnalysis.SymbolEqualityComparer.Default); + return SymbolEqualityComparer.Default.Equals(x, y); + }} +}} +"; + + await new VerifyCS.Test + { + TestState = { Sources = { source, MinimalSymbolImplementationCSharp, SymbolEqualityComparerStubCSharp } }, + FixedState = { Sources = { fixedSource, MinimalSymbolImplementationCSharp, SymbolEqualityComparerStubCSharp } }, + }.RunAsync(); + } + + [Theory] + [InlineData(nameof(ISymbol))] + [InlineData(nameof(INamedTypeSymbol))] + public async Task CompareSymbolImplementationWithInterface_NoComparer_CSharp(string symbolType) + { + var source = $@" +using Microsoft.CodeAnalysis; +class TestClass {{ + bool Method(Symbol x, {symbolType} y) {{ + return [|x == y|]; + }} +}} +"; + var fixedSource = $@" +using Microsoft.CodeAnalysis; +class TestClass {{ + bool Method(Symbol x, {symbolType} y) {{ + return Equals(x, y); }} }} "; @@ -297,6 +404,35 @@ End Class "; var fixedSource = $@" Imports Microsoft.CodeAnalysis +Class TestClass + Function Method(x As {symbolType}, y As {symbolType}) As Boolean + Return SymbolEqualityComparer.Default.Equals(x, y) + End Function +End Class +"; + + await new VerifyVB.Test + { + TestState = { Sources = { source, SymbolEqualityComparerStubVisualBasic } }, + FixedState = { Sources = { fixedSource, SymbolEqualityComparerStubVisualBasic } }, + }.RunAsync(); + } + + [Theory] + [InlineData(nameof(ISymbol))] + [InlineData(nameof(INamedTypeSymbol))] + public async Task CompareTwoSymbolsEquals_NoComparer_VisualBasic(string symbolType) + { + var source = $@" +Imports Microsoft.CodeAnalysis +Class TestClass + Function Method(x As {symbolType}, y As {symbolType}) As Boolean + Return [|x Is y|] + End Function +End Class +"; + var fixedSource = $@" +Imports Microsoft.CodeAnalysis Class TestClass Function Method(x As {symbolType}, y As {symbolType}) As Boolean Return Equals(x, y) From ca364109086effdb57f33db9ecc90a81088f2d78 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Wed, 4 Sep 2019 11:19:54 -0700 Subject: [PATCH 09/10] Fix test --- .../UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs index a70f494a66..d137900982 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs @@ -241,7 +241,7 @@ class TestClass {{ using Microsoft.CodeAnalysis; class TestClass {{ bool Method({symbolType} x, {symbolType} y) {{ - return SymbolEqualityComparer.Default.Equals(x, y); + return Equals(x, y); }} }} "; From 3bc68a102d947bffc81b219e32bac4222484e932 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 26 Sep 2019 12:31:32 -0700 Subject: [PATCH 10/10] Add test for Equal -> EqualityComparer --- .../CompareSymbolsCorrectlyAnalyzer.cs | 37 +++++++++------ .../Fixers/CompareSymbolsCorrectlyFix.cs | 46 +++++++++++-------- .../CompareSymbolsCorrectlyTests.cs | 30 ++++++++++++ 3 files changed, 80 insertions(+), 33 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index 6e10c4e197..6ecc1217d5 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -19,7 +19,7 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer private static readonly string s_symbolTypeFullName = typeof(ISymbol).FullName; private const string s_symbolEqualsName = nameof(ISymbol.Equals); - public const string SymbolEqualityComparerName = "Microsoft.CodeAnalysis.Shared.Utilities.SymbolEquivalenceComparer"; + public const string SymbolEqualityComparerName = "Microsoft.CodeAnalysis.SymbolEqualityComparer"; public static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( DiagnosticIds.CompareSymbolsCorrectlyRuleId, @@ -47,12 +47,11 @@ public override void Initialize(AnalysisContext context) return; } - // Check that the s_symbolEqualityComparerName exists and can be used, otherwise the Roslyn version + // 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 symbolEqualityComparerType = context.Compilation.GetTypeByMetadataName(SymbolEqualityComparerName); - var operatorsToHandle = symbolEqualityComparerType is null ? - new[] { OperationKind.BinaryOperator } : - new[] { OperationKind.BinaryOperator, OperationKind.MethodReference }; + var operatorsToHandle = UseSymbolEqualityComparer(context.Compilation) ? + new[] { OperationKind.BinaryOperator, OperationKind.Invocation } : + new[] { OperationKind.BinaryOperator }; context.RegisterOperationAction(context => HandleOperation(in context, symbolType), operatorsToHandle); }); @@ -62,11 +61,11 @@ private void HandleOperation(in OperationAnalysisContext context, INamedTypeSymb { if (context.Operation is IBinaryOperation) { - HandleBinaryOperator(context, symbolType); + HandleBinaryOperator(in context, symbolType); } - if (context.Operation is IMethodReferenceOperation) + else if (context.Operation is IInvocationOperation) { - HandleMethodReferenceOperation(context, symbolType); + HandleInvocationOperation(in context, symbolType); } } @@ -113,19 +112,24 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(Rule)); } - private static void HandleMethodReferenceOperation(OperationAnalysisContext context, INamedTypeSymbol symbolType) + private static void HandleInvocationOperation(in OperationAnalysisContext context, INamedTypeSymbol symbolType) { - var methodReference = (IMethodReferenceOperation)context.Operation; + var invocationOperation = (IInvocationOperation)context.Operation; + var method = invocationOperation.TargetMethod; + if (method.Name != s_symbolEqualsName) + { + return; + } - if (methodReference.Instance != null && !IsSymbolType(methodReference.Instance, symbolType)) + if (invocationOperation.Instance != null && !IsSymbolType(invocationOperation.Instance, symbolType)) { return; } - var parameters = methodReference.Method.Parameters; - if (methodReference.Method.Name == s_symbolEqualsName && parameters.All(p => IsSymbolType(p.Type, symbolType))) + var parameters = invocationOperation.Arguments; + if (parameters.All(p => IsSymbolType(p.Value, symbolType))) { - context.ReportDiagnostic(methodReference.Syntax.GetLocation().CreateDiagnostic(Rule)); + context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(Rule)); } } @@ -197,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; } } diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs index c5e872a9ae..986faaba52 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs +++ b/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; @@ -16,8 +17,6 @@ namespace Microsoft.CodeAnalysis.Analyzers.MetaAnalyzers.Fixers [Shared] public class CompareSymbolsCorrectlyFix : CodeFixProvider { - private const string s_equalityComparerIdentifier = "Microsoft.CodeAnalysis.SymbolEqualityComparer"; - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(CompareSymbolsCorrectlyAnalyzer.Rule.Id); public override FixAllProvider GetFixAllProvider() @@ -48,14 +47,14 @@ private async Task ConvertToEqualsAsync(Document document, TextSpan so return rawOperation switch { IBinaryOperation binaryOperation => await ConvertToEqualsAsync(document, semanticModel, binaryOperation, cancellationToken).ConfigureAwait(false), - IMethodReferenceOperation methodReferenceOperation => await EnsureEqualsCorrectAsync(document, semanticModel, methodReferenceOperation, cancellationToken).ConfigureAwait(false), + IInvocationOperation invocationOperation => await EnsureEqualsCorrectAsync(document, semanticModel, invocationOperation, cancellationToken).ConfigureAwait(false), _ => document }; } - private static async Task EnsureEqualsCorrectAsync(Document document, SemanticModel semanticModel, IMethodReferenceOperation methodReference, CancellationToken cancellationToken) + private static async Task EnsureEqualsCorrectAsync(Document document, SemanticModel semanticModel, IInvocationOperation invocationOperation, CancellationToken cancellationToken) { - if (!UseEqualityComparer(semanticModel.Compilation)) + if (!CompareSymbolsCorrectlyAnalyzer.UseSymbolEqualityComparer(semanticModel.Compilation)) { return document; } @@ -63,8 +62,20 @@ private static async Task EnsureEqualsCorrectAsync(Document document, var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); var generator = editor.Generator; - var replacement = generator.AddParameters(methodReference.Syntax, new[] { generator.IdentifierName(s_equalityComparerIdentifier) }); - editor.ReplaceNode(methodReference.Syntax, replacement.WithTriviaFrom(methodReference.Syntax)); + 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(); } @@ -75,15 +86,11 @@ private static async Task ConvertToEqualsAsync(Document document, Sema var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); var generator = editor.Generator; - var replacement = UseEqualityComparer(semanticModel.Compilation) switch + var replacement = CompareSymbolsCorrectlyAnalyzer.UseSymbolEqualityComparer(semanticModel.Compilation) switch { - true => + true => generator.InvocationExpression( - generator.MemberAccessExpression( - generator.MemberAccessExpression( - generator.DottedName(s_equalityComparerIdentifier), - "Default"), - nameof(object.Equals)), + GetEqualityComparerDefaultEquals(generator), binaryOperation.LeftOperand.Syntax.WithoutLeadingTrivia(), binaryOperation.RightOperand.Syntax.WithoutTrailingTrivia()), @@ -105,9 +112,12 @@ private static async Task ConvertToEqualsAsync(Document document, Sema return editor.GetChangedDocument(); } - private static bool UseEqualityComparer(Compilation compilation) - { - return compilation.GetTypeByMetadataName(s_equalityComparerIdentifier) is object; - } + 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"); } } diff --git a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs index d137900982..cd236a79a4 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/UnitTests/MetaAnalyzers/CompareSymbolsCorrectlyTests.cs @@ -508,5 +508,35 @@ End Class await VerifyVB.VerifyAnalyzerAsync(source); } + + + [Theory] + [InlineData(nameof(ISymbol))] + [InlineData(nameof(INamedTypeSymbol))] + public async Task CompareSymbolImplementationWithInterface_EqualsComparison_CSharp(string symbolType) + { + var source = $@" +using Microsoft.CodeAnalysis; +class TestClass {{ + bool Method(ISymbol x, {symbolType} y) {{ + return [|Equals(x, y)|]; + }} +}} +"; + var fixedSource = $@" +using Microsoft.CodeAnalysis; +class TestClass {{ + bool Method(ISymbol x, {symbolType} y) {{ + return SymbolEqualityComparer.Default.Equals(x, y); + }} +}} +"; + + await new VerifyCS.Test + { + TestState = { Sources = { source, SymbolEqualityComparerStubCSharp } }, + FixedState = { Sources = { fixedSource, SymbolEqualityComparerStubCSharp } }, + }.RunAsync(); + } } }