improvement(analyzer): enhance handling of public members in nested classes

DecSmith42 · DecSmith42 · commit a73f80f66d63 · 2026-01-23T20:09:47.000+10:00
Add new test cases to validate proper handling of public members within nested internal and public classes. Refactor `AT0001_PublicMemberShouldBeAnnotatedWithPublicAPIAnalyzer` to improve performance and readability by caching configuration for valid attributes. Streamline logic for effective public access checks and recursive attribute validation. Update `_usings.cs` to include `System.Collections.Concurrent` for the newly introduced caching mechanism.
diff --git a/DecSm.Analyzers.PublicApiSurface.Tests/AT0001_PublicMemberShouldBeAnnotatedWithPublicAPIAnalyzerTests.cs b/DecSm.Analyzers.PublicApiSurface.Tests/AT0001_PublicMemberShouldBeAnnotatedWithPublicAPIAnalyzerTests.cs
@@ -237,6 +237,43 @@ public static class MyClass
         await Verifier.VerifyAnalyzerAsync(text, Configure, expected);
     }
 
+    [Fact]
+    public async Task PublicMemberInNestedInternalClass_NoDiagnostic()
+    {
+        const string text = """
+                            public class Outer
+                            {
+                                internal class Inner
+                                {
+                                    public void MyMethod() { }
+                                }
+                            }
+                            """;
+
+        var expected = Verifier
+            .Diagnostic()
+            .WithSpan(1, 14, 1, 19)
+            .WithArguments("Outer");
+
+        await Verifier.VerifyAnalyzerAsync(text, Configure, expected);
+    }
+
+    [Fact]
+    public async Task PublicMemberInPublicClassInInternalClass_NoDiagnostic()
+    {
+        const string text = """
+                            internal class Outer
+                            {
+                                public class Inner
+                                {
+                                    public void MyMethod() { }
+                                }
+                            }
+                            """;
+
+        await Verifier.VerifyAnalyzerAsync(text, Configure);
+    }
+
     [Fact]
     public async Task CustomAttribute_WhenConfigured_NoDiagnostic()
     {
diff --git a/DecSm.Analyzers.PublicApiSurface/AT0001_PublicMemberShouldBeAnnotatedWithPublicAPIAnalyzer.cs b/DecSm.Analyzers.PublicApiSurface/AT0001_PublicMemberShouldBeAnnotatedWithPublicAPIAnalyzer.cs
@@ -37,132 +37,124 @@ public class DSPA0001_PublicMemberShouldBeAnnotatedWithPublicAPIAnalyzer : Diagn
 
     public override void Initialize(AnalysisContext context)
     {
-        // You must call this method to avoid analyzing generated code.
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
-
-        // You must call this method to enable the Concurrent Execution.
         context.EnableConcurrentExecution();
 
-        // Register for symbol actions
-        context.RegisterSymbolAction(AnalyzeSymbol,
-            SymbolKind.NamedType,
-            SymbolKind.Method,
-            SymbolKind.Property,
-            SymbolKind.Field,
-            SymbolKind.Event);
+        context.RegisterCompilationStartAction(compilationContext =>
+        {
+            var configCache = new ConcurrentDictionary<string, ImmutableHashSet<string>>(StringComparer.Ordinal);
+            compilationContext.RegisterSymbolAction(symbolContext => AnalyzeSymbol(symbolContext, configCache),
+                SymbolKind.NamedType,
+                SymbolKind.Method,
+                SymbolKind.Property,
+                SymbolKind.Field,
+                SymbolKind.Event);
+        });
     }
 
-    private static void AnalyzeSymbol(SymbolAnalysisContext context)
+    private static void AnalyzeSymbol(SymbolAnalysisContext context, ConcurrentDictionary<string, ImmutableHashSet<string>> configCache)
     {
         var symbol = context.Symbol;
 
-        // Only check public symbols
-        if (symbol.DeclaredAccessibility != Accessibility.Public)
-            return;
-
-        // If public, check if containing type is also public
-        if (symbol.ContainingType?.DeclaredAccessibility is not Accessibility.Public and not null)
+        if (symbol.IsImplicitlyDeclared)
             return;
 
-        var optionsProvider = context.Options.AnalyzerConfigOptionsProvider;
-
-        optionsProvider.GlobalOptions.TryGetValue("dotnet_code_quality.DecSm_Analyzers_ValidPublicApiAttributes",
-            out var values);
+        if (symbol is IMethodSymbol methodSymbol)
+        {
+            if (methodSymbol.MethodKind is MethodKind.Constructor or MethodKind.StaticConstructor or
+                MethodKind.PropertyGet or MethodKind.PropertySet or
+                MethodKind.EventAdd or MethodKind.EventRemove or MethodKind.EventRaise)
+                return;
 
-        var options = GeneratorOptions.Parse(values);
+            if (methodSymbol.IsOverride)
+                return;
+        }
 
-        var tree = symbol.Locations.FirstOrDefault()
-            ?.SourceTree;
+        if (!IsEffectivelyPublic(symbol))
+            return;
 
-        if (tree != null &&
-            optionsProvider
-                .GetOptions(tree)
-                .TryGetValue("dotnet_code_quality.DecSm_Analyzers_ValidPublicApiAttributes", out var fileValues))
-            options = GeneratorOptions.Parse(fileValues);
+        var validAttributes = GetValidAttributes(context, configCache);
 
-        var validAttributes = options.ValidAttributes.ToImmutableHashSet();
+        if (HasValidAttributeRecursive(symbol, validAttributes))
+            return;
 
-        // Skip if the symbol itself has [PublicAPI]
-        if (HasValidAttribute(symbol, validAttributes))
+        if (symbol.Name == "" || validAttributes.Contains(symbol.Name))
             return;
 
-        // Skip if the containing type has [PublicAPI]
-        var containingType = symbol.ContainingType;
+        context.ReportDiagnostic(Diagnostic.Create(Rule, symbol.Locations[0], symbol.Name));
+    }
 
-        while (containingType != null)
+    private static bool IsEffectivelyPublic(ISymbol symbol)
+    {
+        var current = symbol;
+        while (current != null)
         {
-            if (HasValidAttribute(containingType, validAttributes))
-                return;
+            if (current.DeclaredAccessibility != Accessibility.Public)
+                return false;
 
-            containingType = containingType.ContainingType;
+            current = current.ContainingType;
         }
 
-        // Special case: Implicitly declared symbols (like default constructors)
-        if (symbol.IsImplicitlyDeclared)
-            return;
-
-        // Special case: .ctor methods
-        if (symbol is { Kind: SymbolKind.Method, Name: ".ctor" })
-            return;
+        return true;
+    }
 
-        // Special case: Nameless symbols
-        // Also don't flag the Public API attributes themselves
-        if (symbol.Name == "" ||
-            IsValidAttributeName(symbol.Name, validAttributes) ||
-            symbol.Name == "PublicAPI" ||
-            symbol.Name == "PublicAPIAttribute")
-            return;
+    private static ImmutableHashSet<string> GetValidAttributes(SymbolAnalysisContext context, ConcurrentDictionary<string, ImmutableHashSet<string>> cache)
+    {
+        var optionsProvider = context.Options.AnalyzerConfigOptionsProvider;
+        var tree = context.Symbol.Locations.Length > 0 ? context.Symbol.Locations[0].SourceTree : null;
 
-        if (symbol is IMethodSymbol methodSymbol)
+        if (tree != null && optionsProvider.GetOptions(tree).TryGetValue("dotnet_code_quality.DecSm_Analyzers_ValidPublicApiAttributes", out var treeValues))
         {
-            // Skip property/event accessors, as we check the property/event itself
-            if (methodSymbol.MethodKind is MethodKind.PropertyGet
-                or MethodKind.PropertySet
-                or MethodKind.EventAdd
-                or MethodKind.EventRemove
-                or MethodKind.EventRaise)
-                return;
-
-            if (methodSymbol.IsOverride)
-                return;
+            return ParseAndCache(treeValues, cache);
         }
 
-        context.ReportDiagnostic(Diagnostic.Create(Rule, symbol.Locations[0], symbol.Name));
+        optionsProvider.GlobalOptions.TryGetValue("dotnet_code_quality.DecSm_Analyzers_ValidPublicApiAttributes", out var globalValues);
+        return ParseAndCache(globalValues ?? string.Empty, cache);
     }
 
-    private static bool IsValidAttributeName(string name, ImmutableHashSet<string> attributeNames)
+    private static ImmutableHashSet<string> ParseAndCache(string values, ConcurrentDictionary<string, ImmutableHashSet<string>> cache)
     {
-        if (attributeNames.Contains(name))
-            return true;
-
-        return name.EndsWith("Attribute", StringComparison.Ordinal) &&
-               name.Length > 9 &&
-               attributeNames.Contains(name.Substring(0, name.Length - 9));
+        return cache.GetOrAdd(values, v =>
+        {
+            var builder = ImmutableHashSet.CreateBuilder<string>(StringComparer.Ordinal);
+            builder.Add("PublicAPI");
+            builder.Add("PublicAPIAttribute");
+
+            if (!string.IsNullOrWhiteSpace(v))
+            {
+                var parts = v.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
+                foreach (var part in parts)
+                {
+                    var trimmed = part.Trim();
+                    if (string.IsNullOrWhiteSpace(trimmed))
+                        continue;
+
+                    builder.Add(trimmed);
+                    if (!trimmed.EndsWith("Attribute", StringComparison.Ordinal))
+                    {
+                        builder.Add(trimmed + "Attribute");
+                    }
+                }
+            }
+
+            return builder.ToImmutable();
+        });
     }
 
-    private static bool HasValidAttribute(ISymbol symbol, ImmutableHashSet<string> attributeNames) =>
-        symbol
-            .GetAttributes()
-            .Any(attr => attr.AttributeClass != null && IsValidAttributeName(attr.AttributeClass.Name, attributeNames));
-}
+    private static bool HasValidAttributeRecursive(ISymbol symbol, ImmutableHashSet<string> validAttributes)
+    {
+        var current = symbol;
+        while (current != null)
+        {
+            foreach (var attribute in current.GetAttributes())
+            {
+                if (attribute.AttributeClass != null && validAttributes.Contains(attribute.AttributeClass.Name))
+                    return true;
+            }
 
-public sealed record GeneratorOptions(string[] ValidAttributes)
-{
-    public string[] ValidAttributes { get; } = ValidAttributes;
+            current = current.ContainingType;
+        }
 
-    public static GeneratorOptions Parse(string? s)
-    {
-        if (string.IsNullOrWhiteSpace(s))
-            return new(["PublicAPI"]);
-
-        var results = s!
-            .Split(',')
-            .Select(x => x.Trim())
-            .Where(x => !string.IsNullOrWhiteSpace(x))
-            .Append("PublicAPI")
-            .Distinct()
-            .ToArray();
-
-        return new(results);
+        return false;
     }
 }
diff --git a/DecSm.Analyzers.PublicApiSurface/_usings.cs b/DecSm.Analyzers.PublicApiSurface/_usings.cs
@@ -1,5 +1,5 @@
 ﻿global using System;
+global using System.Collections.Concurrent;
 global using System.Collections.Immutable;
-global using System.Linq;
 global using Microsoft.CodeAnalysis;
 global using Microsoft.CodeAnalysis.Diagnostics;
