From aba7cf1bdb0040d22e11407f266cdd469e9db2ec Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Wed, 17 Apr 2024 17:32:52 +0300 Subject: [PATCH 1/2] Avoid direct descriptor comparison, so the fix can be shown --- .../CompareSymbolsCorrectlyAnalyzer.cs | 18 ++++++---- .../Fixers/CompareSymbolsCorrectlyFix.cs | 36 ++++++++++--------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index f0e3be553e..f3a99ee10f 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.Linq; @@ -26,7 +27,9 @@ 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_HashCodeCombineName = "Combine"; + public const string SymbolEqualityComparerName = "Microsoft.CodeAnalysis.SymbolEqualityComparer"; + public const string RulePropertyName = "Rule"; public static readonly DiagnosticDescriptor EqualityRule = new( DiagnosticIds.CompareSymbolsCorrectlyRuleId, @@ -142,7 +145,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN return; } - context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(EqualityRule)); + context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(EqualityRule, MakeProperties(nameof(EqualityRule)))); } private static void HandleInvocationOperation( @@ -163,7 +166,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN // without the correct arguments if (IsSymbolType(invocationOperation.Instance, symbolType)) { - context.ReportDiagnostic(invocationOperation.CreateDiagnostic(GetHashCodeRule)); + context.ReportDiagnostic(invocationOperation.CreateDiagnostic(GetHashCodeRule, MakeProperties(nameof(GetHashCode)))); } break; @@ -174,7 +177,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN var parameters = invocationOperation.Arguments; if (parameters.All(p => IsSymbolType(p.Value, symbolType))) { - context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(EqualityRule)); + context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(EqualityRule, MakeProperties(nameof(EqualityRule)))); } } @@ -187,7 +190,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN systemHashCodeType.Equals(method.ContainingType, SymbolEqualityComparer.Default) && invocationOperation.Arguments.Any(arg => IsSymbolType(arg.Value, symbolType))) { - context.ReportDiagnostic(invocationOperation.CreateDiagnostic(GetHashCodeRule)); + context.ReportDiagnostic(invocationOperation.CreateDiagnostic(GetHashCodeRule, MakeProperties(nameof(GetHashCodeRule)))); } break; @@ -199,7 +202,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN IsBehavingOnSymbolType(method, symbolType) && !invocationOperation.Arguments.Any(arg => IsSymbolType(arg.Value, iEqualityComparer))) { - context.ReportDiagnostic(invocationOperation.CreateDiagnostic(CollectionRule)); + context.ReportDiagnostic(invocationOperation.CreateDiagnostic(CollectionRule, MakeProperties(nameof(CollectionRule)))); } break; @@ -249,7 +252,7 @@ static bool IsBehavingOnSymbolType(IMethodSymbol? method, INamedTypeSymbol symbo IsSymbolType(createdType.TypeArguments[0], symbolType) && !objectCreation.Arguments.Any(arg => IsSymbolType(arg.Value, iEqualityComparerType))) { - context.ReportDiagnostic(objectCreation.CreateDiagnostic(CollectionRule)); + context.ReportDiagnostic(objectCreation.CreateDiagnostic(CollectionRule, MakeProperties(nameof(CollectionRule)))); } } @@ -354,6 +357,9 @@ void AddOrUpdate(string methodName, INamedTypeSymbol typeSymbol) } } + private static ImmutableDictionary MakeProperties(string rule) + => ImmutableDictionary.CreateRange([new KeyValuePair(RulePropertyName, rule)]); + public static bool UseSymbolEqualityComparer(Compilation compilation) => compilation.GetOrCreateTypeByMetadataName(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 fe9a8fc585..4a1a007df6 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs @@ -32,23 +32,27 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) { foreach (var diagnostic in context.Diagnostics) { - if (diagnostic.Descriptor == CompareSymbolsCorrectlyAnalyzer.EqualityRule) + if (diagnostic.Properties.TryGetValue(CompareSymbolsCorrectlyAnalyzer.RulePropertyName, out var rule)) { - context.RegisterCodeFix( - CodeAction.Create( - CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyCodeFix, - cancellationToken => ConvertToEqualsAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken), - equivalenceKey: nameof(CompareSymbolsCorrectlyFix)), - diagnostic); - } - else if (diagnostic.Descriptor == CompareSymbolsCorrectlyAnalyzer.CollectionRule) - { - context.RegisterCodeFix( - CodeAction.Create( - CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyCodeFix, - cancellationToken => CallOverloadWithEqualityComparerAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken), - equivalenceKey: nameof(CompareSymbolsCorrectlyFix)), - diagnostic); + switch (rule) + { + case nameof(CompareSymbolsCorrectlyAnalyzer.EqualityRule): + context.RegisterCodeFix( + CodeAction.Create( + CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyCodeFix, + cancellationToken => ConvertToEqualsAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken), + equivalenceKey: nameof(CompareSymbolsCorrectlyFix)), + diagnostic); + break; + case nameof(CompareSymbolsCorrectlyAnalyzer.CollectionRule): + context.RegisterCodeFix( + CodeAction.Create( + CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyCodeFix, + cancellationToken => CallOverloadWithEqualityComparerAsync(context.Document, diagnostic.Location.SourceSpan, cancellationToken), + equivalenceKey: nameof(CompareSymbolsCorrectlyFix)), + diagnostic); + break; + } } } From 37a5a329ddd7591820fa5f84f88679128f0f3d18 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Mon, 29 Apr 2024 19:13:57 +0300 Subject: [PATCH 2/2] Feedback and some refactorings --- .../CompareSymbolsCorrectlyAnalyzer.cs | 36 ++++++++++++------- .../Fixers/CompareSymbolsCorrectlyFix.cs | 4 +-- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs index f3a99ee10f..e2e30d64b1 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/CompareSymbolsCorrectlyAnalyzer.cs @@ -31,7 +31,11 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer public const string SymbolEqualityComparerName = "Microsoft.CodeAnalysis.SymbolEqualityComparer"; public const string RulePropertyName = "Rule"; - public static readonly DiagnosticDescriptor EqualityRule = new( + public const string EqualityRuleName = "EqualityRule"; + public const string GetHashCodeRuleName = "GetHashCodeRule"; + public const string CollectionRuleName = "CollectionRule"; + + private static readonly DiagnosticDescriptor s_equalityRule = new( DiagnosticIds.CompareSymbolsCorrectlyRuleId, s_localizableTitle, s_localizableMessage, @@ -41,7 +45,7 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer description: s_localizableDescription, customTags: WellKnownDiagnosticTagsExtensions.Telemetry); - public static readonly DiagnosticDescriptor GetHashCodeRule = new( + private static readonly DiagnosticDescriptor s_getHashCodeRule = new( DiagnosticIds.CompareSymbolsCorrectlyRuleId, s_localizableTitle, s_localizableMessage, @@ -51,7 +55,7 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer description: CreateLocalizableResourceString(nameof(CompareSymbolsCorrectlyDescriptionGetHashCode)), customTags: WellKnownDiagnosticTagsExtensions.Telemetry); - public static readonly DiagnosticDescriptor CollectionRule = new( + private static readonly DiagnosticDescriptor s_collectionRule = new( DiagnosticIds.CompareSymbolsCorrectlyRuleId, s_localizableTitle, s_localizableMessage, @@ -61,7 +65,16 @@ public class CompareSymbolsCorrectlyAnalyzer : DiagnosticAnalyzer description: s_localizableDescription, customTags: WellKnownDiagnosticTagsExtensions.Telemetry); - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(EqualityRule); + private static readonly ImmutableDictionary s_EqualityRuleProperties = + ImmutableDictionary.CreateRange([new KeyValuePair(RulePropertyName, EqualityRuleName)]); + + private static readonly ImmutableDictionary s_GetHashCodeRuleProperties = + ImmutableDictionary.CreateRange([new KeyValuePair(RulePropertyName, GetHashCodeRuleName)]); + + private static readonly ImmutableDictionary s_CollectionRuleProperties = + ImmutableDictionary.CreateRange([new KeyValuePair(RulePropertyName, CollectionRuleName)]); + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(s_equalityRule); public override void Initialize(AnalysisContext context) { @@ -145,7 +158,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN return; } - context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(EqualityRule, MakeProperties(nameof(EqualityRule)))); + context.ReportDiagnostic(binary.Syntax.GetLocation().CreateDiagnostic(s_equalityRule, s_EqualityRuleProperties)); } private static void HandleInvocationOperation( @@ -166,7 +179,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN // without the correct arguments if (IsSymbolType(invocationOperation.Instance, symbolType)) { - context.ReportDiagnostic(invocationOperation.CreateDiagnostic(GetHashCodeRule, MakeProperties(nameof(GetHashCode)))); + context.ReportDiagnostic(invocationOperation.CreateDiagnostic(s_getHashCodeRule, s_GetHashCodeRuleProperties)); } break; @@ -177,7 +190,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN var parameters = invocationOperation.Arguments; if (parameters.All(p => IsSymbolType(p.Value, symbolType))) { - context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(EqualityRule, MakeProperties(nameof(EqualityRule)))); + context.ReportDiagnostic(invocationOperation.Syntax.GetLocation().CreateDiagnostic(s_equalityRule, s_EqualityRuleProperties)); } } @@ -190,7 +203,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN systemHashCodeType.Equals(method.ContainingType, SymbolEqualityComparer.Default) && invocationOperation.Arguments.Any(arg => IsSymbolType(arg.Value, symbolType))) { - context.ReportDiagnostic(invocationOperation.CreateDiagnostic(GetHashCodeRule, MakeProperties(nameof(GetHashCodeRule)))); + context.ReportDiagnostic(invocationOperation.CreateDiagnostic(s_getHashCodeRule, s_GetHashCodeRuleProperties)); } break; @@ -202,7 +215,7 @@ private static void HandleBinaryOperator(in OperationAnalysisContext context, IN IsBehavingOnSymbolType(method, symbolType) && !invocationOperation.Arguments.Any(arg => IsSymbolType(arg.Value, iEqualityComparer))) { - context.ReportDiagnostic(invocationOperation.CreateDiagnostic(CollectionRule, MakeProperties(nameof(CollectionRule)))); + context.ReportDiagnostic(invocationOperation.CreateDiagnostic(s_collectionRule, s_CollectionRuleProperties)); } break; @@ -252,7 +265,7 @@ static bool IsBehavingOnSymbolType(IMethodSymbol? method, INamedTypeSymbol symbo IsSymbolType(createdType.TypeArguments[0], symbolType) && !objectCreation.Arguments.Any(arg => IsSymbolType(arg.Value, iEqualityComparerType))) { - context.ReportDiagnostic(objectCreation.CreateDiagnostic(CollectionRule, MakeProperties(nameof(CollectionRule)))); + context.ReportDiagnostic(objectCreation.CreateDiagnostic(s_collectionRule, s_CollectionRuleProperties)); } } @@ -357,9 +370,6 @@ void AddOrUpdate(string methodName, INamedTypeSymbol typeSymbol) } } - private static ImmutableDictionary MakeProperties(string rule) - => ImmutableDictionary.CreateRange([new KeyValuePair(RulePropertyName, rule)]); - public static bool UseSymbolEqualityComparer(Compilation compilation) => compilation.GetOrCreateTypeByMetadataName(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 4a1a007df6..1114be3838 100644 --- a/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs +++ b/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/Fixers/CompareSymbolsCorrectlyFix.cs @@ -36,7 +36,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) { switch (rule) { - case nameof(CompareSymbolsCorrectlyAnalyzer.EqualityRule): + case CompareSymbolsCorrectlyAnalyzer.EqualityRuleName: context.RegisterCodeFix( CodeAction.Create( CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyCodeFix, @@ -44,7 +44,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) equivalenceKey: nameof(CompareSymbolsCorrectlyFix)), diagnostic); break; - case nameof(CompareSymbolsCorrectlyAnalyzer.CollectionRule): + case CompareSymbolsCorrectlyAnalyzer.CollectionRuleName: context.RegisterCodeFix( CodeAction.Create( CodeAnalysisDiagnosticsResources.CompareSymbolsCorrectlyCodeFix,