From 32782f6b63643829b644de7ba67f842082fa0a33 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 30 Apr 2026 14:12:35 -0700 Subject: [PATCH] Track type inheritance chain assemblies in symbol-based mode (#144) In the symbol-based analysis path, only the immediate base type's containing assembly was credited via `TrackType(namedType.BaseType)`. The C# compiler validates the entire base-type chain plus implemented interfaces at type-check time -- CS0012 fires when *any* link's defining assembly is missing -- so any assembly along the chain must remain a reference. With `true`, the chain doesn't flow transitively, so the consumer must explicitly reference grandparent assemblies. Symbol-based RT, when handling `INamedTypeSymbol`, walked only the immediate `BaseType` and direct `Interfaces`, never recursing further up. So for `Consumer : Provider` where `Provider : ProviderDependency` in another assembly, the consumer's reference to `ProviderDependency` was wrongly flagged RT0002 removable. Fix: extend `TrackType`'s `INamedTypeSymbol` branch to walk the full `BaseType` chain and `AllInterfaces` collection. `AllInterfaces` is broader than `Interfaces` (includes transitively-implemented interfaces) and mirrors what the compiler validates. A `ConcurrentDictionary` keyed via `SymbolEqualityComparer.Default` gates the chain walk to break self-referential cycles such as `int -> AllInterfaces[IComparable] -> typeArg int -> ...` and to avoid redundant work. Adds seven regression tests that all fail without this change: - `UsedViaInheritedBaseType`: the canonical issue #144 scenario - `UsedViaImplementedInterface`: interface in the chain - `UsedViaMultiLevelInheritanceChain`: three-level A <- B <- C - `UsedViaInheritanceChainOnVariableType`: chain via parameter type - `UsedViaMixedBaseAndInterfaceChain`: base + interface, four asms - `UsedViaGenericConstraintBaseChain`: `where T : Provider` constraint - `UnrelatedReferenceNotMarkedByInheritance`: negative test ensuring the fix doesn't over-credit unrelated assemblies Verified against upstream repro https://github.com/olstakh/RT_Gap_Inheritance: `Consumer` builds with 0 RT warnings (was emitting RT0002 with v3.5.2). Different code path from #141 (qualifier tracking on inherited static member access), #142 (delegate parameter / return types), and #143 (override chains). Same family of "symbol traversal misses a chain" gaps; same reporter (olstakh). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Analyzer/ReferenceTrimmerAnalyzer.cs | 39 ++++++ src/Tests/AnalyzerTests.cs | 157 +++++++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/src/Analyzer/ReferenceTrimmerAnalyzer.cs b/src/Analyzer/ReferenceTrimmerAnalyzer.cs index a53dfc3..16e4c52 100644 --- a/src/Analyzer/ReferenceTrimmerAnalyzer.cs +++ b/src/Analyzer/ReferenceTrimmerAnalyzer.cs @@ -194,6 +194,16 @@ private static void InitializeSymbolBasedAnalysis( // callbacks short-circuit. A briefly stale read just means a few extra no-op lookups. int trackedCount = 0; + // Tracks named types whose inheritance chain (base types + transitively-implemented + // interfaces) has already been walked, to break self-referential cycles such as + // `int` → `IComparable` → typeArg `int` → ... and to avoid redundant work for + // types used many times. Use SymbolEqualityComparer to match Roslyn semantics for + // constructed generic types (e.g., distinct INamedTypeSymbol instances representing + // the same `List` are considered equal). +#pragma warning disable RS1024 // Compare symbols correctly (false positive: comparer is supplied explicitly) + var inheritanceWalked = new ConcurrentDictionary(SymbolEqualityComparer.Default); +#pragma warning restore RS1024 + void TrackAssembly(IAssemblySymbol? assembly) { if (trackedCount >= totalReferenceCount) @@ -247,6 +257,35 @@ void TrackType(ITypeSymbol? type) { TrackType(typeArg); } + + // When a named type is referenced, the C# compiler validates the entire + // inheritance chain at type-check time -- CS0012 fires when *any* base + // type or implemented interface is defined in an unreferenced assembly. + // Walk BaseType (recursively) and AllInterfaces (transitively) so every + // assembly along the chain is credited. AllInterfaces is broader than + // Interfaces and mirrors what the compiler actually validates. The + // visited-set guard breaks self-referential cycles (e.g. int -> + // IComparable -> typeArg int -> ...) and avoids redundant work. + if (inheritanceWalked.TryAdd(named, 0)) + { + for (INamedTypeSymbol? baseType = named.BaseType; baseType != null; baseType = baseType.BaseType) + { + TrackAssembly(baseType.ContainingAssembly); + foreach (ITypeSymbol typeArg in baseType.TypeArguments) + { + TrackType(typeArg); + } + } + + foreach (INamedTypeSymbol iface in named.AllInterfaces) + { + TrackAssembly(iface.ContainingAssembly); + foreach (ITypeSymbol typeArg in iface.TypeArguments) + { + TrackType(typeArg); + } + } + } } return; diff --git a/src/Tests/AnalyzerTests.cs b/src/Tests/AnalyzerTests.cs index cc553a8..f951ecb 100644 --- a/src/Tests/AnalyzerTests.cs +++ b/src/Tests/AnalyzerTests.cs @@ -698,6 +698,163 @@ public async Task UnrelatedReferenceNotMarkedByOverride() StringAssert.Contains(diagnostics[0].GetMessage(CultureInfo.InvariantCulture), "Unrelated"); } + [TestMethod] + public async Task UsedViaInheritedBaseType() + { + // The canonical issue #144 scenario: Consumer derives from a class in B, which itself + // derives from a class in A. With true, A + // does not flow transitively from B → Consumer, so Consumer must reference A directly. + // Without walking the BaseType chain we'd flag A as removable, but removing it produces + // CS0012 because the C# compiler validates the entire base-type chain. + var aAsm = EmitDependency( + "namespace Dep { public class ProviderDependency { public int Counter; } }", + assemblyName: "AAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class Provider : ProviderDependency { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference]); + var diagnostics = await RunAnalyzerAsync( + "class Consumer : Dep.Provider { }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj")]); + AssertNoDiagnostics(diagnostics); + } + + [TestMethod] + public async Task UsedViaImplementedInterface() + { + // Interface chain: A defines IFoo, B defines a class Foo : IFoo, Consumer derives from Foo. + // Consumer's reference to A is required because the compiler validates that Foo's + // implemented interface is reachable. Without walking the interface chain we'd miss A. + var aAsm = EmitDependency( + "namespace Dep { public interface IFoo { } }", + assemblyName: "AAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class Foo : IFoo { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference]); + var diagnostics = await RunAnalyzerAsync( + "class Consumer : Dep.Foo { }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj")]); + AssertNoDiagnostics(diagnostics); + } + + [TestMethod] + public async Task UsedViaMultiLevelInheritanceChain() + { + // Three-level base-type chain: A ← B ← C. Consumer references C and uses it as a base + // class. Every assembly along the chain must be credited because the C# compiler + // validates the full inheritance chain (CS0012 fires on any missing link). + var aAsm = EmitDependency( + "namespace Dep { public class A { } }", + assemblyName: "AAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class B : A { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference]); + var cAsm = EmitDependency( + "namespace Dep { public class C : B { } }", + assemblyName: "CAsm", + additionalReferences: [aAsm.Reference, bAsm.Reference]); + var diagnostics = await RunAnalyzerAsync( + "class Consumer : Dep.C { }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj"), + (cAsm.Reference, cAsm.Path, "ProjectReference", "../C/C.csproj")]); + AssertNoDiagnostics(diagnostics); + } + + [TestMethod] + public async Task UsedViaInheritanceChainOnVariableType() + { + // The chain must be walked even when the named type is encountered as a variable type + // (not as an explicit base in Consumer's own code). Declaring a parameter of type + // Dep.Provider still requires every assembly in Provider's inheritance chain. + var aAsm = EmitDependency( + "namespace Dep { public class ProviderDependency { } }", + assemblyName: "AAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class Provider : ProviderDependency { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference]); + var diagnostics = await RunAnalyzerAsync( + "class Consumer { void M(Dep.Provider p) { } }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj")]); + AssertNoDiagnostics(diagnostics); + } + + [TestMethod] + public async Task UsedViaMixedBaseAndInterfaceChain() + { + // Mixed scenario: B's class inherits from A's class and implements D's interface. + // Consumer derives from B's class. All four assemblies (A, B, D, and B itself via the + // direct base) must be credited. + var aAsm = EmitDependency( + "namespace Dep { public class BaseA { } }", + assemblyName: "AAsm"); + var dAsm = EmitDependency( + "namespace Dep { public interface IFromD { } }", + assemblyName: "DAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class Provider : BaseA, IFromD { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference, dAsm.Reference]); + var diagnostics = await RunAnalyzerAsync( + "class Consumer : Dep.Provider { }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj"), + (dAsm.Reference, dAsm.Path, "ProjectReference", "../D/D.csproj")]); + AssertNoDiagnostics(diagnostics); + } + + [TestMethod] + public async Task UsedViaGenericConstraintBaseChain() + { + // Generic constraint variant: T : Provider where Provider : ProviderDependency. + // Consumer's constraint forces the compiler to validate Provider's base chain, so A + // must remain a reference. + var aAsm = EmitDependency( + "namespace Dep { public class ProviderDependency { } }", + assemblyName: "AAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class Provider : ProviderDependency { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference]); + var diagnostics = await RunAnalyzerAsync( + "class Consumer where T : Dep.Provider { }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj")]); + AssertNoDiagnostics(diagnostics); + } + + [TestMethod] + public async Task UnrelatedReferenceNotMarkedByInheritance() + { + // Negative test: deriving from a type whose inheritance chain spans two assemblies + // should not credit an entirely unrelated assembly. Only the assemblies along the + // chain are required. + var aAsm = EmitDependency( + "namespace Dep { public class ProviderDependency { } }", + assemblyName: "AAsm"); + var bAsm = EmitDependency( + "namespace Dep { public class Provider : ProviderDependency { } }", + assemblyName: "BAsm", + additionalReferences: [aAsm.Reference]); + var unrelated = EmitDependency( + "namespace Other { public class Unused { } }", + assemblyName: "UnrelatedAsm"); + var diagnostics = await RunAnalyzerAsync( + "class Consumer : Dep.Provider { }", + [(aAsm.Reference, aAsm.Path, "ProjectReference", "../A/A.csproj"), + (bAsm.Reference, bAsm.Path, "ProjectReference", "../B/B.csproj"), + (unrelated.Reference, unrelated.Path, "ProjectReference", "../Unrelated/Unrelated.csproj")]); + Assert.AreEqual(1, diagnostics.Length); + Assert.AreEqual("RT0002", diagnostics[0].Id); + StringAssert.Contains(diagnostics[0].GetMessage(CultureInfo.InvariantCulture), "Unrelated"); + } + // ────────────────────────────────────────────────────────────────────── // Test infrastructure // ──────────────────────────────────────────────────────────────────────