From b9bc28c6bee75a273442c758cb95112eb97f1717 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sun, 20 Aug 2023 11:52:44 +0300 Subject: [PATCH] Refactor PropertiesShouldNotBeWriteOnlyAnalyzer (CA1044) --- .../PropertiesShouldNotBeWriteOnly.cs | 31 ++++++++++--------- .../Compiler/Extensions/ISymbolExtensions.cs | 6 ++-- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/PropertiesShouldNotBeWriteOnly.cs b/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/PropertiesShouldNotBeWriteOnly.cs index b5ac031148..e5279043f5 100644 --- a/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/PropertiesShouldNotBeWriteOnly.cs +++ b/src/NetAnalyzers/Core/Microsoft.CodeQuality.Analyzers/ApiDesignGuidelines/PropertiesShouldNotBeWriteOnly.cs @@ -57,40 +57,41 @@ public override void Initialize(AnalysisContext context) /// private static void AnalyzeSymbol(SymbolAnalysisContext context) { - if (context.Symbol is not IPropertySymbol property) - { - return; - } + var property = (IPropertySymbol)context.Symbol; // not raising a violation for when: // property is overridden because the issue can only be fixed in the base type // property is the implementation of any interface member - if (property.IsOverride || property.IsImplementationOfAnyInterfaceMember()) + if (property.IsOverride || GetRule(property) is not DiagnosticDescriptor descriptor || property.IsImplementationOfAnyInterfaceMember()) { return; } Debug.Assert(context.Options.MatchesConfiguredVisibility(MakeMoreAccessibleRule, property, context.Compilation) == context.Options.MatchesConfiguredVisibility(AddGetterRule, property, context.Compilation)); + Debug.Assert(descriptor == MakeMoreAccessibleRule || descriptor == AddGetterRule); + // Only analyze externally visible properties by default + if (context.Options.MatchesConfiguredVisibility(descriptor, property, context.Compilation)) + { + context.ReportDiagnostic(property.CreateDiagnostic(descriptor, property.Name)); + } + } + + private static DiagnosticDescriptor? GetRule(IPropertySymbol property) + { // We handled the non-CA1044 cases earlier. Now, we handle CA1044 cases // If there is no getter then it is not accessible if (property.IsWriteOnly) { - // Only analyze externally visible properties by default - if (context.Options.MatchesConfiguredVisibility(AddGetterRule, property, context.Compilation)) - { - context.ReportDiagnostic(property.CreateDiagnostic(AddGetterRule, property.Name)); - } + return AddGetterRule; } // Otherwise if there is a setter, check for its relative accessibility else if (!property.IsReadOnly && (property.GetMethod!.DeclaredAccessibility < property.SetMethod!.DeclaredAccessibility)) { - // Only analyze externally visible properties by default - if (context.Options.MatchesConfiguredVisibility(MakeMoreAccessibleRule, property, context.Compilation)) - { - context.ReportDiagnostic(property.CreateDiagnostic(MakeMoreAccessibleRule, property.Name)); - } + return MakeMoreAccessibleRule; } + + return null; } } } diff --git a/src/Utilities/Compiler/Extensions/ISymbolExtensions.cs b/src/Utilities/Compiler/Extensions/ISymbolExtensions.cs index 91eec55546..7f9c14395c 100644 --- a/src/Utilities/Compiler/Extensions/ISymbolExtensions.cs +++ b/src/Utilities/Compiler/Extensions/ISymbolExtensions.cs @@ -563,17 +563,17 @@ public static ISymbol GetOverriddenMember(this ISymbol symbol) /// public static bool IsImplementationOfAnyExplicitInterfaceMember([NotNullWhen(returnValue: true)] this ISymbol? symbol) { - if (symbol is IMethodSymbol methodSymbol && methodSymbol.ExplicitInterfaceImplementations.Any()) + if (symbol is IMethodSymbol methodSymbol && !methodSymbol.ExplicitInterfaceImplementations.IsEmpty) { return true; } - if (symbol is IPropertySymbol propertySymbol && propertySymbol.ExplicitInterfaceImplementations.Any()) + if (symbol is IPropertySymbol propertySymbol && !propertySymbol.ExplicitInterfaceImplementations.IsEmpty) { return true; } - if (symbol is IEventSymbol eventSymbol && eventSymbol.ExplicitInterfaceImplementations.Any()) + if (symbol is IEventSymbol eventSymbol && !eventSymbol.ExplicitInterfaceImplementations.IsEmpty) { return true; }