From a9b0baa5c15eb46543bc357e2020d40eab2dd103 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Fri, 24 Jul 2026 23:17:08 +0200 Subject: [PATCH 1/3] docs: add AI plan for bundled annotation type reachability Signed-off-by: Kenny Pflug --- ...81-bundled-annotation-type-reachability.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 ai-plans/0181-bundled-annotation-type-reachability.md diff --git a/ai-plans/0181-bundled-annotation-type-reachability.md b/ai-plans/0181-bundled-annotation-type-reachability.md new file mode 100644 index 0000000..17507fb --- /dev/null +++ b/ai-plans/0181-bundled-annotation-type-reachability.md @@ -0,0 +1,37 @@ +# Reachability-Driven Emission of Bundled Annotation Types + +## Rationale + +`ReSharperAnnotations.cs` is copied into the export as an all-or-nothing unit, so `IncludeJetBrainsAnnotations` is a per-file switch while every annotation-removal option is a per-attribute switch. Enabling the option to obtain `NoEnumerationAttribute` therefore also emits `ContractAnnotationAttribute` and the JetBrains `NotNullAttribute`, neither of which the generated code references once `RemoveContractAnnotations` is enabled. The same wholesale copying applies to the other bundled support types: six of the nine `System.Diagnostics.CodeAnalysis` polyfill attributes are unreferenced in every configuration, which is roughly ninety lines of dead code in the committed single file. + +Decide emission of bundled support types by reachability from the annotation usages that survive cleanup, so `IncludeJetBrainsAnnotations` and the polyfill inclusion options mean "provide the support types this export needs" instead of "provide all of them". This keeps the export as deliberate as its whitelist already suggests without introducing per-attribute configuration options. + +## Acceptance Criteria + +- [ ] Bundled support types are emitted only when the generated file references them, directly or transitively, for every combination of the annotation-removal and inclusion options and for whitelisted exports whose retained assertions no longer use an annotation. +- [ ] The JetBrains `NotNullAttribute` is emitted only as a dependency of `ContractAnnotationAttribute`, never because of the `[NotNull]` usages on assertion parameters. +- [ ] When no JetBrains annotation type survives, the `JetBrains.Annotations` namespace, its license banner, and the `using JetBrains.Annotations;` directive are all absent from the generated file. +- [ ] The `NotNullAttribute` alias directive is emitted exactly when the JetBrains `NotNullAttribute` is part of the export. +- [ ] The export behaves as before when `IncludeJetBrainsAnnotations` is `false`, including the configuration that imports the JetBrains namespace from an externally supplied package. +- [ ] Exports whose bundled types were trimmed still compile through the generated-file build validation. +- [ ] The committed `Light.GuardClauses.SingleFile.cs` is regenerated, and its only change is the removal of the unreferenced code-analysis polyfill attributes; the JetBrains annotation types and all guard-clause code are byte-identical. +- [ ] Automated tests cover the trimming decisions for the relevant option combinations, including the configuration reported in issue #181. +- [ ] The source-inclusion documentation describes emission of bundled support types as reachability-driven, and documents that importing the JetBrains namespace without bundling its types requires the consuming project to supply them. + +## Technical Details + +Add an `AnnotationReachabilityTrimmer` that transforms the merged `CompilationUnitSyntax` after `CleanupStep.RemoveAnnotations` and before the final `NormalizeWhitespace`. Running after cleanup is essential and is why this cannot live in `SourceReachabilityAnalyzer`: that analyzer inspects the original source files, where every removable attribute usage is still present. + +The trimmer operates on a candidate set of bundled support types rather than on all unreferenced types, because public API types must never be trimmed for lack of references. The candidates are the JetBrains annotation types, `ValidatedNotNullAttribute`, and the emitted `System.Diagnostics.CodeAnalysis` and `System.Runtime.CompilerServices` polyfill types. Two of them reach the merged tree as always-processed source files that whitelist mode exempts from reachability analysis, the other two as literal blocks injected by the merger, so the candidate set has to be identified in the final tree rather than derived from either mechanism. + +Resolve references with Roslyn symbol analysis rather than name matching. Create a syntax tree from the final root and a compilation over it, reusing the metadata references that `SourceReachabilityAnalyzer` builds today, then bind every `AttributeSyntax` outside the candidate declarations to its containing type symbol. This resolves the `NotNullAttribute` alias exactly, and the `ContractAnnotationAttribute` to JetBrains `NotNullAttribute` edge falls out of the same closure instead of being special-cased: inside `namespace JetBrains.Annotations`, the namespace member wins over the compilation-unit alias. Note that the root reached at that point in the pipeline does not belong to a syntax tree that reflects it, so the tree must be recreated with the export's parse options. + +When an attribute cannot be bound, retain every candidate whose simple name matches. A degraded binding must produce a slightly larger file, never one that fails to compile. + +Reachability is established from attribute usages only. Documentation `cref` references are invisible to the trimmer, so a retained type must never document a trimmed one; the four `cref` references among the candidates are self-references that disappear with their own declaration, and `GenerateDocumentationFile` is enabled for consuming projects, where a dangling `cref` would surface as a warning. + +Derive the JetBrains using directives from the trimmed set only while `IncludeJetBrainsAnnotations` is `true`. When it is `false`, `IncludeJetBrainsAnnotationsUsing` keeps its current meaning so consumers can supply the annotations externally, as established in [0179](0179-source-export-annotation-cleanup.md). Removing the namespace declaration while leaving its import in place is a compile error, so the two decisions must stay coupled. The JetBrains license banner is leading trivia of the `namespace` token and is removed together with the namespace node. + +Cover the JetBrains trimming decisions with focused tests for the default configuration, `RemoveContractAnnotations` alone, both JetBrains removal options together, and `IncludeJetBrainsAnnotations=false`. Add an end-to-end build validation for the configuration from issue #181 — `IncludeJetBrainsAnnotations`, `IncludeJetBrainsAnnotationsUsing`, `RemoveContractAnnotations` and `RemoveValidatedNotNull` enabled, `RemoveNoEnumeration` and `IncludeValidatedNotNullAttribute` disabled — asserting that `NoEnumerationAttribute` survives while the other JetBrains types and `ValidatedNotNullAttribute` do not. Cover polyfill trimming with a `netstandard2.0` export that retains the referenced attributes and drops the unreferenced ones. + +Default settings keep all three JetBrains annotation types, so the `verify-single-file` CI job protects that half of the change. The polyfill trimming deliberately changes the committed single file; the removed types are `internal`, so no consumer contract is affected. From e6db5a3aebebe59b69c8b0b8d32371103e525bf9 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Fri, 24 Jul 2026 23:43:29 +0200 Subject: [PATCH 2/3] chore: implement annotation reachability via Roslyn on Source Export Signed-off-by: Kenny Pflug --- Light.GuardClauses.SingleFile.cs | 128 ------ ...81-bundled-annotation-type-reachability.md | 18 +- docs/source-code-inclusion.md | 16 +- .../AnnotationReachabilityTrimmerTests.cs | 192 +++++++++ .../GeneratedFileBuildValidatorTests.cs | 32 ++ .../GeneratedSourceFile.cs | 49 +++ .../SourceFileMergerFrameworkTests.cs | 6 +- .../AnnotationReachabilityTrimmer.cs | 365 ++++++++++++++++++ .../CleanupStep.cs | 16 +- .../SourceFileMerger.cs | 4 + .../SourceReachabilityAnalyzer.cs | 86 +++-- 11 files changed, 731 insertions(+), 181 deletions(-) create mode 100644 tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs create mode 100644 tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs create mode 100644 tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs diff --git a/Light.GuardClauses.SingleFile.cs b/Light.GuardClauses.SingleFile.cs index 1234e8d..dfe120b 100644 --- a/Light.GuardClauses.SingleFile.cs +++ b/Light.GuardClauses.SingleFile.cs @@ -14658,36 +14658,6 @@ internal sealed class NoEnumerationAttribute : Attribute namespace System.Diagnostics.CodeAnalysis { - /// - /// Specifies that is allowed as an input even if the - /// corresponding type disallows it. - /// - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] - internal sealed class AllowNullAttribute : Attribute - { - /// - /// Initializes a new instance of the class. - /// - public AllowNullAttribute() - { - } - } - - /// - /// Specifies that is disallowed as an input even if the - /// corresponding type allows it. - /// - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] - internal sealed class DisallowNullAttribute : Attribute - { - /// - /// Initializes a new instance of the class. - /// - public DisallowNullAttribute() - { - } - } - /// /// Specifies that a method that will never return under any circumstance. /// @@ -14702,76 +14672,6 @@ public DoesNotReturnAttribute() } } - /// - /// Specifies that the method will not return if the associated - /// parameter is passed the specified value. - /// - [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] - internal sealed class DoesNotReturnIfAttribute : Attribute - { - /// - /// Gets the condition parameter value. - /// Code after the method is considered unreachable by diagnostics if the argument - /// to the associated parameter matches this value. - /// - public bool ParameterValue { get; } - - /// - /// Initializes a new instance of the - /// class with the specified parameter value. - /// - /// - /// The condition parameter value. - /// Code after the method is considered unreachable by diagnostics if the argument - /// to the associated parameter matches this value. - /// - public DoesNotReturnIfAttribute(bool parameterValue) - { - ParameterValue = parameterValue; - } - } - - /// - /// Specifies that an output may be even if the - /// corresponding type disallows it. - /// - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] - internal sealed class MaybeNullAttribute : Attribute - { - /// - /// Initializes a new instance of the class. - /// - public MaybeNullAttribute() - { - } - } - - /// - /// Specifies that when a method returns , - /// the parameter may be even if the corresponding type disallows it. - /// - [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] - internal sealed class MaybeNullWhenAttribute : Attribute - { - /// - /// Gets the return value condition. - /// If the method returns this value, the associated parameter may be . - /// - public bool ReturnValue { get; } - - /// - /// Initializes the attribute with the specified return value condition. - /// - /// - /// The return value condition. - /// If the method returns this value, the associated parameter may be . - /// - public MaybeNullWhenAttribute(bool returnValue) - { - ReturnValue = returnValue; - } - } - /// /// Specifies that an output is not even if the /// corresponding type allows it. @@ -14787,34 +14687,6 @@ public NotNullAttribute() } } - /// - /// Specifies that the output will be non- if the - /// named parameter is non-. - /// - [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, AllowMultiple = true, Inherited = false)] - internal sealed class NotNullIfNotNullAttribute : Attribute - { - /// - /// Gets the associated parameter name. - /// The output will be non- if the argument to the - /// parameter specified is non-. - /// - public string ParameterName { get; } - - /// - /// Initializes the attribute with the associated parameter name. - /// - /// - /// The associated parameter name. - /// The output will be non- if the argument to the - /// parameter specified is non-. - /// - public NotNullIfNotNullAttribute(string parameterName) - { - ParameterName = parameterName; - } - } - /// /// Specifies that when a method returns , /// the parameter will not be even if the corresponding type allows it. diff --git a/ai-plans/0181-bundled-annotation-type-reachability.md b/ai-plans/0181-bundled-annotation-type-reachability.md index 17507fb..1b3f3db 100644 --- a/ai-plans/0181-bundled-annotation-type-reachability.md +++ b/ai-plans/0181-bundled-annotation-type-reachability.md @@ -8,15 +8,15 @@ Decide emission of bundled support types by reachability from the annotation usa ## Acceptance Criteria -- [ ] Bundled support types are emitted only when the generated file references them, directly or transitively, for every combination of the annotation-removal and inclusion options and for whitelisted exports whose retained assertions no longer use an annotation. -- [ ] The JetBrains `NotNullAttribute` is emitted only as a dependency of `ContractAnnotationAttribute`, never because of the `[NotNull]` usages on assertion parameters. -- [ ] When no JetBrains annotation type survives, the `JetBrains.Annotations` namespace, its license banner, and the `using JetBrains.Annotations;` directive are all absent from the generated file. -- [ ] The `NotNullAttribute` alias directive is emitted exactly when the JetBrains `NotNullAttribute` is part of the export. -- [ ] The export behaves as before when `IncludeJetBrainsAnnotations` is `false`, including the configuration that imports the JetBrains namespace from an externally supplied package. -- [ ] Exports whose bundled types were trimmed still compile through the generated-file build validation. -- [ ] The committed `Light.GuardClauses.SingleFile.cs` is regenerated, and its only change is the removal of the unreferenced code-analysis polyfill attributes; the JetBrains annotation types and all guard-clause code are byte-identical. -- [ ] Automated tests cover the trimming decisions for the relevant option combinations, including the configuration reported in issue #181. -- [ ] The source-inclusion documentation describes emission of bundled support types as reachability-driven, and documents that importing the JetBrains namespace without bundling its types requires the consuming project to supply them. +- [x] Bundled support types are emitted only when the generated file references them, directly or transitively, for every combination of the annotation-removal and inclusion options and for whitelisted exports whose retained assertions no longer use an annotation. +- [x] The JetBrains `NotNullAttribute` is emitted only as a dependency of `ContractAnnotationAttribute`, never because of the `[NotNull]` usages on assertion parameters. +- [x] When no JetBrains annotation type survives, the `JetBrains.Annotations` namespace, its license banner, and the `using JetBrains.Annotations;` directive are all absent from the generated file. +- [x] The `NotNullAttribute` alias directive is emitted exactly when the JetBrains `NotNullAttribute` is part of the export. +- [x] The export behaves as before when `IncludeJetBrainsAnnotations` is `false`, including the configuration that imports the JetBrains namespace from an externally supplied package. +- [x] Exports whose bundled types were trimmed still compile through the generated-file build validation. +- [x] The committed `Light.GuardClauses.SingleFile.cs` is regenerated, and its only change is the removal of the unreferenced code-analysis polyfill attributes; the JetBrains annotation types and all guard-clause code are byte-identical. +- [x] Automated tests cover the trimming decisions for the relevant option combinations, including the configuration reported in issue #181. +- [x] The source-inclusion documentation describes emission of bundled support types as reachability-driven, and documents that importing the JetBrains namespace without bundling its types requires the consuming project to supply them. ## Technical Details diff --git a/docs/source-code-inclusion.md b/docs/source-code-inclusion.md index 7ebf2f1..735487b 100644 --- a/docs/source-code-inclusion.md +++ b/docs/source-code-inclusion.md @@ -10,7 +10,7 @@ Copy [`Light.GuardClauses.SingleFile.cs`](../Light.GuardClauses.SingleFile.cs) i - changes public Light.GuardClauses types to `internal`, avoiding additions to the consuming assembly's public API; - enables nullable annotations; -- retains JetBrains contract annotations and the code-analysis, validated-null, and caller-argument-expression support needed by the portable source shape; +- retains the JetBrains contract annotations and only those code-analysis, validated-null, and caller-argument-expression support types that the portable source shape actually references; - includes the full selected-framework assertion surface; and - contains one flattened source shape with no conditional-compilation directives. @@ -49,7 +49,7 @@ Configuration keys are case-insensitive. Nested command-line keys use `:`, for e | `BaseNamespace` | `Light.GuardClauses` | Rewrites the base namespace | | `RemoveContractAnnotations` | `false` | Removes JetBrains contract annotations from members | | `RemoveNoEnumeration` | `false` | Removes JetBrains `NoEnumeration` annotation usages | -| `IncludeJetBrainsAnnotations` / `IncludeJetBrainsAnnotationsUsing` | `true` | Controls the bundled annotation and using directive | +| `IncludeJetBrainsAnnotations` / `IncludeJetBrainsAnnotationsUsing` | `true` | Controls whether the JetBrains annotation types are bundled and whether their namespace is imported | | `IncludeVersionComment` | `true` | Adds the generated version header | | `RemoveOverloadsWithExceptionFactory` | `false` | Globally removes assertion overloads that accept exception factories | | Nullable/validated-null options | varies; see settings | Include supporting attributes or remove their usages | @@ -67,6 +67,18 @@ Use the committed settings file for the exact names and defaults of all nullable The parser selects the appropriate conditional branches and the merger emits no `#if`, `#else`, `#endif`, or other conditional-compilation directives. The result is therefore for the selected target only, not a multi-target source file. +## Bundled support types + +Besides the guard-clause code itself, an export can bundle supporting attribute declarations: the JetBrains annotations, `ValidatedNotNullAttribute`, and the `System.Diagnostics.CodeAnalysis` and `System.Runtime.CompilerServices` polyfills that `netstandard2.0` does not provide. + +The inclusion options above decide which of these types the export is *allowed* to bundle. Which of them are actually emitted is then decided by reachability: after the configured annotation usages have been removed, the exporter binds the remaining attribute usages and keeps only the support types they reach, directly or transitively. So `IncludeJetBrainsAnnotations` and the polyfill inclusion options mean "provide the support types this export needs", not "provide all of them". Three consequences are worth knowing: + +- The JetBrains `NotNullAttribute` is only emitted as a dependency of `ContractAnnotationAttribute`. The `[NotNull]` usages on assertion parameters resolve to `System.Diagnostics.CodeAnalysis.NotNullAttribute`, so `RemoveContractAnnotations` removes both JetBrains types at once and also drops the `NotNullAttribute` alias directive that disambiguates them. +- When no JetBrains annotation type survives, the `JetBrains.Annotations` namespace, its license banner, and `using JetBrains.Annotations;` are all omitted. +- A whitelisted export drops the support types that its retained assertions do not annotate, even though the exporter processes the bundled declaration files as a whole. + +`IncludeJetBrainsAnnotationsUsing` is independent of this trimming while `IncludeJetBrainsAnnotations` is `false`: the generated file then imports `JetBrains.Annotations` without declaring it, and the consuming project has to supply the annotation types itself, for example through the [JetBrains.Annotations](https://www.nuget.org/packages/JetBrains.Annotations) NuGet package. Turn `IncludeJetBrainsAnnotationsUsing` off as well and use `RemoveContractAnnotations` plus `RemoveNoEnumeration` to obtain a file with no JetBrains dependency at all. + ## Assertion whitelisting and dependency retention When `AssertionWhitelist.IsEnabled` is `false`, all entry settings are ignored and the full surface for the selected framework is exported. diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs new file mode 100644 index 0000000..765ca3d --- /dev/null +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs @@ -0,0 +1,192 @@ +using System; +using System.IO; +using System.Linq; +using System.Reflection; +using FluentAssertions; +using Xunit; + +namespace Light.GuardClauses.SourceCodeTransformation.Tests; + +public static class AnnotationReachabilityTrimmerTests +{ + private const string JetBrainsAnnotationsNamespace = "JetBrains.Annotations"; + private const string CodeAnalysisNamespace = "System.Diagnostics.CodeAnalysis"; + private const string CompilerServicesNamespace = "System.Runtime.CompilerServices"; + private const string JetBrainsUsingDirective = "using JetBrains.Annotations;"; + + private const string NotNullAliasDirective = + "using NotNullAttribute = System.Diagnostics.CodeAnalysis.NotNullAttribute;"; + + [Fact] + public static void DefaultConfigurationRetainsEveryJetBrainsAnnotationType() + { + using var temporaryDirectory = new TemporaryDirectory(); + + var generatedFile = Export(temporaryDirectory, "DefaultJetBrains.cs"); + + generatedFile.GetDeclaredTypeNames(JetBrainsAnnotationsNamespace) + .Should().BeEquivalentTo( + "NotNullAttribute", + "ContractAnnotationAttribute", + "NoEnumerationAttribute" + ); + generatedFile.UsingDirectives.Should().Contain(JetBrainsUsingDirective).And.Contain(NotNullAliasDirective); + } + + [Fact] + public static void RemovingContractAnnotationsAlsoTrimsTheJetBrainsNotNullAttribute() + { + using var temporaryDirectory = new TemporaryDirectory(); + + // The [NotNull] usages on assertion parameters bind to the code-analysis attribute through the alias, + // thus the JetBrains NotNullAttribute is only reachable as a dependency of ContractAnnotationAttribute. + var generatedFile = Export( + temporaryDirectory, + "WithoutContractAnnotations.cs", + options => options with { RemoveContractAnnotations = true } + ); + + generatedFile.GetDeclaredTypeNames(JetBrainsAnnotationsNamespace) + .Should().BeEquivalentTo("NoEnumerationAttribute"); + generatedFile.UsingDirectives.Should().Contain(JetBrainsUsingDirective).And.NotContain(NotNullAliasDirective); + } + + [Fact] + public static void RemovingBothJetBrainsAnnotationsTrimsTheWholeNamespaceIncludingItsLicenseBanner() + { + using var temporaryDirectory = new TemporaryDirectory(); + + var generatedFile = Export( + temporaryDirectory, + "WithoutJetBrainsUsages.cs", + options => options with { RemoveContractAnnotations = true, RemoveNoEnumeration = true } + ); + + generatedFile.ContainsNamespace(JetBrainsAnnotationsNamespace).Should().BeFalse(); + generatedFile.SourceCode.Should().NotContain("JetBrains"); + generatedFile.UsingDirectives.Should() + .NotContain(JetBrainsUsingDirective).And.NotContain(NotNullAliasDirective); + } + + [Fact] + public static void ExportWithoutBundledJetBrainsAnnotationsKeepsTheExternallySuppliedImports() + { + using var temporaryDirectory = new TemporaryDirectory(); + + // IncludeJetBrainsAnnotationsUsing keeps its meaning here so that the consuming project can supply + // the annotations from an externally referenced package. + var generatedFile = Export( + temporaryDirectory, + "ExternalJetBrains.cs", + options => options with { IncludeJetBrainsAnnotations = false } + ); + + generatedFile.ContainsNamespace(JetBrainsAnnotationsNamespace).Should().BeFalse(); + generatedFile.UsingDirectives.Should().Contain(JetBrainsUsingDirective).And.Contain(NotNullAliasDirective); + generatedFile.SourceCode.Should().Contain("[ContractAnnotation(").And.Contain("[NoEnumeration]"); + } + + [Fact] + public static void NetStandardExportRetainsOnlyTheReferencedPolyfillAttributes() + { + using var temporaryDirectory = new TemporaryDirectory(); + + var generatedFile = Export(temporaryDirectory, "PolyfillReachability.cs"); + + generatedFile.GetDeclaredTypeNames(CodeAnalysisNamespace) + .Should().BeEquivalentTo( + "DoesNotReturnAttribute", + "NotNullAttribute", + "NotNullWhenAttribute" + ); + generatedFile.GetDeclaredTypeNames(CompilerServicesNamespace) + .Should().BeEquivalentTo("CallerArgumentExpressionAttribute"); + } + + [Fact] + public static void RemovingPolyfillUsagesTrimsTheirDeclarationsAndNamespaces() + { + using var temporaryDirectory = new TemporaryDirectory(); + + var generatedFile = Export( + temporaryDirectory, + "WithoutPolyfillUsages.cs", + options => options with + { + RemoveDoesNotReturn = true, + RemoveNotNullWhen = true, + RemoveCallerArgumentExpressions = true, + } + ); + + generatedFile.GetDeclaredTypeNames(CodeAnalysisNamespace).Should().BeEquivalentTo("NotNullAttribute"); + generatedFile.ContainsNamespace(CompilerServicesNamespace).Should().BeFalse(); + } + + [Fact] + public static void RemovingValidatedNotNullUsagesTrimsItsAttributeDeclaration() + { + using var temporaryDirectory = new TemporaryDirectory(); + + var retainedFile = Export(temporaryDirectory, "WithValidatedNotNull.cs"); + var trimmedFile = Export( + temporaryDirectory, + "WithoutValidatedNotNull.cs", + options => options with { RemoveValidatedNotNull = true } + ); + + retainedFile.GetDeclaredTypeNames("Light.GuardClauses").Should().Contain("ValidatedNotNullAttribute"); + trimmedFile.GetDeclaredTypeNames("Light.GuardClauses").Should().NotContain("ValidatedNotNullAttribute"); + } + + [Fact] + public static void WhitelistedExportTrimsAnnotationsThatItsRetainedAssertionsDoNotUse() + { + using var temporaryDirectory = new TemporaryDirectory(); + var whitelist = new AssertionWhitelist { IsEnabled = true }; + foreach (var property in typeof(AssertionWhitelist) + .GetProperties(BindingFlags.Instance | BindingFlags.Public) + .Where( + property => + property.PropertyType == typeof(AssertionWhitelist.AssertionEntry) + )) + { + property.SetValue( + whitelist, + new AssertionWhitelist.AssertionEntry { Include = property.Name == "InvalidOperation" } + ); + } + + // InvalidOperation and its transitive dependencies only use [ContractAnnotation] and [DoesNotReturn], + // so the bundled types behind the other annotations become unreachable although the whitelisted + // export still processes their source files as a whole. + var generatedFile = Export( + temporaryDirectory, + "WhitelistedExport.cs", + options => options with { AssertionWhitelist = whitelist } + ); + + generatedFile.SourceCode.Should().Contain("public static void InvalidOperation("); + generatedFile.GetDeclaredTypeNames(JetBrainsAnnotationsNamespace) + .Should().BeEquivalentTo("NotNullAttribute", "ContractAnnotationAttribute"); + generatedFile.GetDeclaredTypeNames("Light.GuardClauses").Should().NotContain("ValidatedNotNullAttribute"); + generatedFile.GetDeclaredTypeNames(CodeAnalysisNamespace).Should().BeEquivalentTo("DoesNotReturnAttribute"); + generatedFile.ContainsNamespace(CompilerServicesNamespace).Should().BeFalse(); + } + + private static GeneratedSourceFile Export( + TemporaryDirectory temporaryDirectory, + string fileName, + Func configureOptions = null + ) + { + var options = new SourceFileMergeOptions + { + SourceFolder = TestEnvironment.SourceDirectory.FullName, + TargetFile = Path.Combine(temporaryDirectory.DirectoryPath, fileName), + TargetFramework = SourceTargetFramework.NetStandard2_0, + IncludeVersionComment = false, + }; + return GeneratedSourceFile.Export(configureOptions == null ? options : configureOptions(options)); + } +} diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs index 9dc7b33..aaea795 100644 --- a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs @@ -160,4 +160,36 @@ public static void GeneratedFileWithoutBundledJetBrainsAnnotationsCompilesAfterR sourceCode.Should().Contain("[NotNull]"); sourceCode.Should().Contain("[NotNullWhen("); } + + [Fact] + public static void GeneratedFileWithPartiallyTrimmedBundledTypesCompiles() + { + using var temporaryDirectory = new TemporaryDirectory(); + var targetFile = Path.Combine(temporaryDirectory.DirectoryPath, "Generated.cs"); + + // This is the configuration reported in issue #181: the export bundles the JetBrains annotations + // solely to obtain NoEnumerationAttribute. + Program.Main( + [ + $"SourceFolder={TestEnvironment.SourceDirectory.FullName}", + $"TargetFile={targetFile}", + "TargetFramework=NetStandard2_0", + "IncludeVersionComment=false", + "IncludeJetBrainsAnnotations=true", + "IncludeJetBrainsAnnotationsUsing=true", + "RemoveContractAnnotations=true", + "RemoveValidatedNotNull=true", + "RemoveNoEnumeration=false", + "IncludeValidatedNotNullAttribute=false", + ] + ).Should().Be(0); + var sourceCode = File.ReadAllText(targetFile); + + sourceCode.Should().Contain("class NoEnumerationAttribute"); + sourceCode.Should().Contain("[NoEnumeration]"); + sourceCode.Should().NotContain("class ContractAnnotationAttribute"); + sourceCode.Should().NotContain("class ValidatedNotNullAttribute"); + sourceCode.Should().Contain("using JetBrains.Annotations;"); + sourceCode.Should().NotContain("using NotNullAttribute ="); + } } diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs new file mode 100644 index 0000000..3d0189e --- /dev/null +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs @@ -0,0 +1,49 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Light.GuardClauses.SourceCodeTransformation.Tests; + +/// +/// Runs the exporter and gives structured access to the generated file so that assertions about emitted +/// namespaces, types, and using directives do not have to rely on substring matching. +/// +internal sealed class GeneratedSourceFile +{ + private readonly CompilationUnitSyntax _root; + + private GeneratedSourceFile(string sourceCode, CompilationUnitSyntax root) + { + SourceCode = sourceCode; + _root = root; + } + + public string SourceCode { get; } + + public IReadOnlyList UsingDirectives => + _root.Usings.Select(usingDirective => usingDirective.ToString().Trim()).ToList(); + + public static GeneratedSourceFile Export(SourceFileMergeOptions options) + { + SourceFileMerger.CreateSingleSourceFile(options); + var sourceCode = File.ReadAllText(options.TargetFile); + var root = (CompilationUnitSyntax) CSharpSyntaxTree.ParseText(sourceCode).GetRoot(); + return new (sourceCode, root); + } + + public bool ContainsNamespace(string namespaceName) => FindNamespaces(namespaceName).Any(); + + public IReadOnlyList GetDeclaredTypeNames(string namespaceName) => + FindNamespaces(namespaceName) + .SelectMany(namespaceDeclaration => namespaceDeclaration.Members) + .OfType() + .Select(typeDeclaration => typeDeclaration.Identifier.ValueText) + .ToList(); + + private IEnumerable FindNamespaces(string namespaceName) => + _root.Members + .OfType() + .Where(namespaceDeclaration => namespaceDeclaration.Name.ToString() == namespaceName); +} diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs index 685627c..fb30513 100644 --- a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs @@ -68,7 +68,7 @@ public static void Net10ExportOmitsPolyfillAttributes() } [Fact] - public static void NetStandardExportIncludesPolyfillAttributes() + public static void NetStandardExportIncludesReferencedPolyfillAttributes() { using var temporaryDirectory = new TemporaryDirectory(); var targetFile = Path.Combine(temporaryDirectory.DirectoryPath, "NetStandardPolyfill.cs"); @@ -76,8 +76,10 @@ public static void NetStandardExportIncludesPolyfillAttributes() SourceFileMerger.CreateSingleSourceFile(CreateOptions(targetFile, SourceTargetFramework.NetStandard2_0)); var sourceCode = File.ReadAllText(targetFile); - sourceCode.Should().Contain("class AllowNullAttribute"); + sourceCode.Should().Contain("class DoesNotReturnAttribute"); sourceCode.Should().Contain("class CallerArgumentExpressionAttribute"); + // The generated file never references this polyfill, thus reachability trimming drops it. + sourceCode.Should().NotContain("class AllowNullAttribute"); } [Fact] diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs new file mode 100644 index 0000000..156cdd2 --- /dev/null +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs @@ -0,0 +1,365 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Light.GuardClauses.SourceCodeTransformation; + +/// +/// Removes the bundled support types that the generated file does not reference any longer. This step must +/// run after because only the merged tree shows which annotation +/// usages actually survive cleanup - the original source files still contain every removable usage. +/// +internal static class AnnotationReachabilityTrimmer +{ + private const string JetBrainsAnnotationsNamespace = "JetBrains.Annotations"; + private const string NotNullAttributeName = "NotNullAttribute"; + private const string ValidatedNotNullAttributeName = "ValidatedNotNullAttribute"; + + private static readonly HashSet BundledNamespaceNames = new (StringComparer.Ordinal) + { + JetBrainsAnnotationsNamespace, + "System.Diagnostics.CodeAnalysis", + "System.Runtime.CompilerServices", + }; + + public static CompilationUnitSyntax Trim(CompilationUnitSyntax compilationUnit, SourceFileMergeOptions options) + { + ArgumentNullException.ThrowIfNull(compilationUnit); + ArgumentNullException.ThrowIfNull(options); + + // At this point in the pipeline the root does not belong to a syntax tree that reflects it, thus the + // tree must be recreated with the export's parse options before anything can be bound. Creating the + // tree clones the root, so all further work happens on the tree's root. + var syntaxTree = CSharpSyntaxTree.Create( + compilationUnit, + SourceReachabilityAnalyzer.CreateParseOptions(options.TargetFramework) + ); + var root = (CompilationUnitSyntax) syntaxTree.GetRoot(); + + var catalog = BundledTypeCatalog.Create(root, options); + var reachableTypes = catalog.Types.Count == 0 ? + new () : + DetermineReachableTypes(syntaxTree, root, catalog); + + root = RemoveUnreachableTypes(root, catalog, reachableTypes); + return AdjustJetBrainsUsingDirectives(root, options, reachableTypes); + } + + private static HashSet DetermineReachableTypes( + SyntaxTree syntaxTree, + CompilationUnitSyntax root, + BundledTypeCatalog catalog + ) + { + var compilation = CSharpCompilation.Create( + "Light.GuardClauses.SourceExportAnnotationReachability", + [syntaxTree], + SourceReachabilityAnalyzer.CreateMetadataReferences(), + new ( + OutputKind.DynamicallyLinkedLibrary, + nullableContextOptions: NullableContextOptions.Enable, + allowUnsafe: true + ) + ); + var semanticModel = compilation.GetSemanticModel(syntaxTree, true); + + var typesBySymbol = new Dictionary(SymbolEqualityComparer.Default); + foreach (var bundledType in catalog.Types) + { + if (GetDeclaredSymbol(semanticModel, bundledType.Declaration) is { } typeSymbol) + { + typesBySymbol[typeSymbol] = bundledType; + } + } + + var reachableTypes = new HashSet(); + var typesToScan = new Queue(); + + // Reachability is established from attribute usages only: the bundled types are attributes, and the + // only cref references among them are self-references that disappear with their own declaration. + foreach (var attribute in root.DescendantNodes().OfType()) + { + if (catalog.IsDeclaredInsideBundledType(attribute)) + { + continue; + } + + MarkAttributeType(attribute); + } + + while (typesToScan.TryDequeue(out var scannedType)) + { + foreach (var attribute in scannedType.Declaration.DescendantNodes().OfType()) + { + MarkAttributeType(attribute); + } + } + + return reachableTypes; + + void MarkAttributeType(AttributeSyntax attribute) + { + var attributeType = ResolveAttributeType(semanticModel, attribute); + if (attributeType == null) + { + // A degraded binding must produce a slightly larger file, never one that fails to compile. + foreach (var matchingType in catalog.FindByUnqualifiedName(attribute.Name)) + { + Mark(matchingType); + } + + return; + } + + if (typesBySymbol.TryGetValue(attributeType, out var bundledType)) + { + Mark(bundledType); + } + } + + void Mark(BundledType bundledType) + { + if (reachableTypes.Add(bundledType)) + { + typesToScan.Enqueue(bundledType); + } + } + } + + private static INamedTypeSymbol? ResolveAttributeType(SemanticModel semanticModel, AttributeSyntax attribute) + { + var symbolInfo = semanticModel.GetSymbolInfo(attribute); + var attributeType = GetAttributeType(symbolInfo.Symbol); + if (attributeType == null) + { + foreach (var candidateSymbol in symbolInfo.CandidateSymbols) + { + attributeType = GetAttributeType(candidateSymbol); + if (attributeType != null) + { + break; + } + } + } + + attributeType ??= GetAttributeType(semanticModel.GetSymbolInfo(attribute.Name).Symbol); + return attributeType is null or IErrorTypeSymbol ? null : attributeType; + } + + private static INamedTypeSymbol? GetAttributeType(ISymbol? symbol) => + symbol switch + { + IMethodSymbol methodSymbol => methodSymbol.ContainingType, + INamedTypeSymbol namedTypeSymbol => namedTypeSymbol, + _ => null, + }; + + private static CompilationUnitSyntax RemoveUnreachableTypes( + CompilationUnitSyntax root, + BundledTypeCatalog catalog, + HashSet reachableTypes + ) + { + var nodesToRemove = new List(); + foreach (var namespaceDeclaration in catalog.BundledNamespaces) + { + var unreachableDeclarations = new List(); + var retainsMember = false; + foreach (var member in namespaceDeclaration.Members) + { + // Anything that is not a bundled type counts as retained: only the catalog decides what may + // be removed for lack of references. + if (catalog.TryGetBundledType(member, out var bundledType) && !reachableTypes.Contains(bundledType)) + { + unreachableDeclarations.Add(member); + } + else + { + retainsMember = true; + } + } + + if (retainsMember) + { + nodesToRemove.AddRange(unreachableDeclarations); + } + else + { + // Removing the namespace node also removes its license banner: the banner is leading trivia + // of the namespace token. + nodesToRemove.Add(namespaceDeclaration); + } + } + + foreach (var bundledType in catalog.TypesOutsideBundledNamespaces) + { + if (!reachableTypes.Contains(bundledType)) + { + nodesToRemove.Add(bundledType.Declaration); + } + } + + return nodesToRemove.Count == 0 ? + root : + root.RemoveNodes(nodesToRemove, SyntaxRemoveOptions.KeepNoTrivia)!; + } + + private static CompilationUnitSyntax AdjustJetBrainsUsingDirectives( + CompilationUnitSyntax root, + SourceFileMergeOptions options, + HashSet reachableTypes + ) + { + // When the JetBrains annotations are not bundled, IncludeJetBrainsAnnotationsUsing keeps its meaning + // so that consumers can supply the annotations from an externally referenced package. + if (!options.IncludeJetBrainsAnnotations) + { + return root; + } + + var retainsJetBrainsTypes = false; + var retainsJetBrainsNotNullAttribute = false; + foreach (var bundledType in reachableTypes) + { + if (bundledType.NamespaceName != JetBrainsAnnotationsNamespace) + { + continue; + } + + retainsJetBrainsTypes = true; + retainsJetBrainsNotNullAttribute |= bundledType.Name == NotNullAttributeName; + } + + var directivesToRemove = new List(); + foreach (var usingDirective in root.Usings) + { + // The alias only exists to disambiguate the two NotNullAttribute declarations. Removing the + // namespace while leaving its import in place would be a compile error, thus both decisions + // are derived from the trimmed set together. + if (usingDirective.Alias != null) + { + if (!retainsJetBrainsNotNullAttribute && + usingDirective.Alias.Name.Identifier.ValueText == NotNullAttributeName) + { + directivesToRemove.Add(usingDirective); + } + } + else if (!retainsJetBrainsTypes && + usingDirective.NamespaceOrType.ToString() == JetBrainsAnnotationsNamespace) + { + directivesToRemove.Add(usingDirective); + } + } + + return directivesToRemove.Count == 0 ? + root : + root.RemoveNodes(directivesToRemove, SyntaxRemoveOptions.KeepNoTrivia)!; + } + + private static ISymbol? GetDeclaredSymbol(SemanticModel semanticModel, MemberDeclarationSyntax declaration) => + declaration switch + { + BaseTypeDeclarationSyntax typeDeclaration => semanticModel.GetDeclaredSymbol(typeDeclaration), + DelegateDeclarationSyntax delegateDeclaration => semanticModel.GetDeclaredSymbol(delegateDeclaration), + _ => null, + }; + + private sealed record BundledType(MemberDeclarationSyntax Declaration, string NamespaceName, string Name); + + /// + /// Identifies the bundled support types in the merged tree. They cannot be derived from the mechanisms + /// that brought them there - two of them arrive as always-processed source files that whitelist mode + /// exempts from reachability analysis, the other two as literal blocks injected by the merger. + /// + private sealed class BundledTypeCatalog + { + private readonly Dictionary _typesByDeclaration = new (); + private readonly Dictionary> _typesByUnqualifiedName = new (StringComparer.Ordinal); + + public List Types { get; } = new (); + + public List BundledNamespaces { get; } = new (); + + public List TypesOutsideBundledNamespaces { get; } = new (); + + public static BundledTypeCatalog Create(CompilationUnitSyntax root, SourceFileMergeOptions options) + { + var catalog = new BundledTypeCatalog(); + foreach (var namespaceDeclaration in root.Members.OfType()) + { + var namespaceName = namespaceDeclaration.Name.ToString(); + if (BundledNamespaceNames.Contains(namespaceName)) + { + catalog.BundledNamespaces.Add(namespaceDeclaration); + foreach (var member in namespaceDeclaration.Members) + { + catalog.Add(member, namespaceName, false); + } + } + else if (namespaceName == options.BaseNamespace) + { + foreach (var member in namespaceDeclaration.Members) + { + if (member is BaseTypeDeclarationSyntax + { + Identifier.ValueText: ValidatedNotNullAttributeName, + }) + { + catalog.Add(member, namespaceName, true); + } + } + } + } + + return catalog; + } + + public bool TryGetBundledType(SyntaxNode declaration, out BundledType bundledType) => + _typesByDeclaration.TryGetValue(declaration, out bundledType!); + + public bool IsDeclaredInsideBundledType(SyntaxNode node) => + node.Ancestors().Any(_typesByDeclaration.ContainsKey); + + public IReadOnlyList FindByUnqualifiedName(NameSyntax attributeName) => + _typesByUnqualifiedName.TryGetValue( + CleanupStep.GetUnqualifiedAttributeName(attributeName), + out var bundledTypes + ) ? + bundledTypes : + []; + + private void Add(MemberDeclarationSyntax declaration, string namespaceName, bool isOutsideBundledNamespace) + { + var name = declaration switch + { + BaseTypeDeclarationSyntax typeDeclaration => typeDeclaration.Identifier.ValueText, + DelegateDeclarationSyntax delegateDeclaration => delegateDeclaration.Identifier.ValueText, + _ => null, + }; + if (name == null) + { + return; + } + + var bundledType = new BundledType(declaration, namespaceName, name); + Types.Add(bundledType); + _typesByDeclaration.Add(declaration, bundledType); + if (isOutsideBundledNamespace) + { + TypesOutsideBundledNamespaces.Add(bundledType); + } + + var unqualifiedName = CleanupStep.RemoveAttributeSuffix(name); + if (!_typesByUnqualifiedName.TryGetValue(unqualifiedName, out var bundledTypes)) + { + bundledTypes = new (); + _typesByUnqualifiedName.Add(unqualifiedName, bundledTypes); + } + + bundledTypes.Add(bundledType); + } + } +} diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/CleanupStep.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/CleanupStep.cs index 36a363e..b222702 100644 --- a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/CleanupStep.cs +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/CleanupStep.cs @@ -150,7 +150,12 @@ private static HashSet GetAnnotationNamesToRemove(SourceFileMergeOptions return annotationNames; } - private static string GetUnqualifiedAttributeName(NameSyntax name) + /// + /// Returns the rightmost component of the specified attribute name without its optional + /// Attribute suffix. This is the identity that matches an attribute usage to its declaration + /// when no semantic binding is available. + /// + internal static string GetUnqualifiedAttributeName(NameSyntax name) { var simpleName = name switch { @@ -158,7 +163,14 @@ private static string GetUnqualifiedAttributeName(NameSyntax name) AliasQualifiedNameSyntax aliasQualifiedName => aliasQualifiedName.Name, _ => (SimpleNameSyntax) name, }; - var identifier = simpleName.Identifier.ValueText; + return RemoveAttributeSuffix(simpleName.Identifier.ValueText); + } + + /// + /// Removes the optional Attribute suffix from the specified identifier. + /// + internal static string RemoveAttributeSuffix(string identifier) + { const string attributeSuffix = "Attribute"; return identifier.EndsWith(attributeSuffix, StringComparison.Ordinal) ? identifier[..^attributeSuffix.Length] : diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs index 83189bf..9d9fc40 100644 --- a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs @@ -658,6 +658,10 @@ node is ClassDeclarationSyntax } targetRoot = CleanupStep.RemoveAnnotations(targetRoot, options); + + Console.WriteLine("Trimming unreferenced bundled support types..."); + targetRoot = AnnotationReachabilityTrimmer.Trim(targetRoot, options); + targetRoot = targetRoot.NormalizeWhitespace(eol: "\n"); var targetFileContent = targetRoot.ToFullString(); diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs index 93500d7..e7b2dbc 100644 --- a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs @@ -11,11 +11,12 @@ namespace Light.GuardClauses.SourceCodeTransformation; internal static class SourceReachabilityAnalyzer { + private static IReadOnlyList? _metadataReferences; + public static CSharpParseOptions CreateParseOptions(SourceTargetFramework targetFramework) => new ( LanguageVersion.CSharp14, - preprocessorSymbols: targetFramework == SourceTargetFramework.Net10_0 - ? + preprocessorSymbols: targetFramework == SourceTargetFramework.Net10_0 ? [ "NET", "NETCOREAPP", @@ -23,10 +24,53 @@ public static CSharpParseOptions CreateParseOptions(SourceTargetFramework target "NET10_0_OR_GREATER", "NET9_0_OR_GREATER", "NET8_0_OR_GREATER", - ] - : ["NETSTANDARD", "NETSTANDARD2_0"] + ] : + ["NETSTANDARD", "NETSTANDARD2_0"] ); + /// + /// Creates the metadata references that every compilation of the exporter uses. The references are + /// resolved once because collecting them requires probing the whole trusted platform assembly list. + /// + public static IReadOnlyList CreateMetadataReferences() + { + if (_metadataReferences != null) + { + return _metadataReferences; + } + + var referencesByPath = new Dictionary(StringComparer.OrdinalIgnoreCase); + + if (AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is string trustedPlatformAssemblies) + { + foreach (var assemblyPath in trustedPlatformAssemblies.Split(Path.PathSeparator)) + { + AddReference(referencesByPath, assemblyPath); + } + } + + AddReference(referencesByPath, typeof(object).Assembly.Location); + AddReference(referencesByPath, typeof(Enumerable).Assembly.Location); + AddReference(referencesByPath, typeof(ImmutableArray).Assembly.Location); + + return _metadataReferences = referencesByPath.Values.ToArray(); + } + + private static void AddReference( + Dictionary referencesByPath, + string assemblyPath + ) + { + if (string.IsNullOrWhiteSpace(assemblyPath) || + !File.Exists(assemblyPath) || + referencesByPath.ContainsKey(assemblyPath)) + { + return; + } + + referencesByPath.Add(assemblyPath, MetadataReference.CreateFromFile(assemblyPath)); + } + public static SourceReachabilityAnalysis Analyze( SourceFileMergeOptions options, IEnumerable sourceFiles @@ -630,40 +674,6 @@ private static bool IsTypeDeclarationNode(SyntaxNode node) => private static bool HasExceptionFactoryParameter(MemberDeclarationSyntax member) => member is BaseMethodDeclarationSyntax method && method.ParameterList.Parameters.Any(parameter => parameter.Identifier.ValueText == "exceptionFactory"); - - private static IReadOnlyList CreateMetadataReferences() - { - var referencesByPath = new Dictionary(StringComparer.OrdinalIgnoreCase); - - if (AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is string trustedPlatformAssemblies) - { - foreach (var assemblyPath in trustedPlatformAssemblies.Split(Path.PathSeparator)) - { - AddReference(referencesByPath, assemblyPath); - } - } - - AddReference(referencesByPath, typeof(object).Assembly.Location); - AddReference(referencesByPath, typeof(Enumerable).Assembly.Location); - AddReference(referencesByPath, typeof(ImmutableArray).Assembly.Location); - - return referencesByPath.Values.ToArray(); - } - - private static void AddReference( - Dictionary referencesByPath, - string assemblyPath - ) - { - if (string.IsNullOrWhiteSpace(assemblyPath) || - !File.Exists(assemblyPath) || - referencesByPath.ContainsKey(assemblyPath)) - { - return; - } - - referencesByPath.Add(assemblyPath, MetadataReference.CreateFromFile(assemblyPath)); - } } private sealed record SourceFile(FileInfo File, SyntaxTree SyntaxTree, CompilationUnitSyntax Root) From cfd4f52f8f347bb761f5e84ef565694384ac251f Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sat, 25 Jul 2026 00:32:31 +0200 Subject: [PATCH 3/3] fix: treat using aliases as reachability roots on Source Export The annotation reachability trimmer established reachability from attribute usages only, but a using alias names its target type as well. Whenever no [NotNull] usage survived cleanup, the code-analysis NotNullAttribute polyfill was trimmed while the alias directive that names it remained, which does not compile - reproducible with a netstandard2.0 whitelist export that retains no annotated assertion. Bind the target of every alias directive and mark the bundled type it resolves to, using the same degraded-binding fallback as attribute usages, so a failed binding keeps too much rather than too little. Compile every export of the trimmer tests through the generated-file build validator: an over-eager trim removes a declaration that the generated file still names, which no assertion about emitted types can catch. The export that supplies the JetBrains annotations externally is the one exception because it cannot compile on its own. Group the test classes that run MSBuild on the shared source validation project into one collection so their builds no longer race for its intermediate output. Also resolve the exporter's metadata references through a Lazy, annotate TryGetBundledType with MaybeNullWhen, expose the bundled type catalog as read-only lists, and print the merger's progress in execution order. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Kenny Pflug --- .../AnnotationReachabilityTrimmerTests.cs | 24 +++++-- .../GeneratedFileBuildCollection.cs | 11 ++++ .../GeneratedFileBuildValidatorTests.cs | 1 + .../GeneratedSourceFile.cs | 14 ++++- .../SourceFileMergerFrameworkTests.cs | 1 + .../AnnotationReachabilityTrimmer.cs | 63 +++++++++++++++---- .../SourceFileMerger.cs | 4 +- .../SourceReachabilityAnalyzer.cs | 19 +++--- 8 files changed, 108 insertions(+), 29 deletions(-) create mode 100644 tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildCollection.cs diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs index 765ca3d..30ac7e9 100644 --- a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/AnnotationReachabilityTrimmerTests.cs @@ -7,6 +7,10 @@ namespace Light.GuardClauses.SourceCodeTransformation.Tests; +// Every export in this class is compiled by the generated-file build validator, thus it shares the +// collection with the other tests that build the source validation project: two concurrent MSBuild runs +// on the same project would race for its intermediate output. +[Collection(GeneratedFileBuildCollection.Name)] public static class AnnotationReachabilityTrimmerTests { private const string JetBrainsAnnotationsNamespace = "JetBrains.Annotations"; @@ -74,11 +78,13 @@ public static void ExportWithoutBundledJetBrainsAnnotationsKeepsTheExternallySup using var temporaryDirectory = new TemporaryDirectory(); // IncludeJetBrainsAnnotationsUsing keeps its meaning here so that the consuming project can supply - // the annotations from an externally referenced package. + // the annotations from an externally referenced package. This export therefore does not compile on + // its own, which is why it is the one export in this class that is not build-validated. var generatedFile = Export( temporaryDirectory, "ExternalJetBrains.cs", - options => options with { IncludeJetBrainsAnnotations = false } + options => options with { IncludeJetBrainsAnnotations = false }, + false ); generatedFile.ContainsNamespace(JetBrainsAnnotationsNamespace).Should().BeFalse(); @@ -170,14 +176,19 @@ public static void WhitelistedExportTrimsAnnotationsThatItsRetainedAssertionsDoN generatedFile.GetDeclaredTypeNames(JetBrainsAnnotationsNamespace) .Should().BeEquivalentTo("NotNullAttribute", "ContractAnnotationAttribute"); generatedFile.GetDeclaredTypeNames("Light.GuardClauses").Should().NotContain("ValidatedNotNullAttribute"); - generatedFile.GetDeclaredTypeNames(CodeAnalysisNamespace).Should().BeEquivalentTo("DoesNotReturnAttribute"); + // No retained assertion uses [NotNull], but the alias directive that the JetBrains NotNullAttribute + // keeps alive names the code-analysis polyfill, thus the polyfill must survive as well. + generatedFile.GetDeclaredTypeNames(CodeAnalysisNamespace) + .Should().BeEquivalentTo("DoesNotReturnAttribute", "NotNullAttribute"); + generatedFile.UsingDirectives.Should().Contain(NotNullAliasDirective); generatedFile.ContainsNamespace(CompilerServicesNamespace).Should().BeFalse(); } private static GeneratedSourceFile Export( TemporaryDirectory temporaryDirectory, string fileName, - Func configureOptions = null + Func configureOptions = null, + bool validateBuild = true ) { var options = new SourceFileMergeOptions @@ -187,6 +198,9 @@ private static GeneratedSourceFile Export( TargetFramework = SourceTargetFramework.NetStandard2_0, IncludeVersionComment = false, }; - return GeneratedSourceFile.Export(configureOptions == null ? options : configureOptions(options)); + return GeneratedSourceFile.Export( + configureOptions == null ? options : configureOptions(options), + validateBuild + ); } } diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildCollection.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildCollection.cs new file mode 100644 index 0000000..4c02a76 --- /dev/null +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildCollection.cs @@ -0,0 +1,11 @@ +namespace Light.GuardClauses.SourceCodeTransformation.Tests; + +/// +/// Groups all test classes that compile a generated file. The build validator runs MSBuild on the shared +/// source validation project, so these tests must not run in parallel with each other - concurrent builds +/// would race for the project's intermediate output. +/// +internal static class GeneratedFileBuildCollection +{ + public const string Name = "Generated File Build"; +} diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs index aaea795..0f39dce 100644 --- a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedFileBuildValidatorTests.cs @@ -5,6 +5,7 @@ namespace Light.GuardClauses.SourceCodeTransformation.Tests; +[Collection(GeneratedFileBuildCollection.Name)] public static class GeneratedFileBuildValidatorTests { [Fact] diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs index 3d0189e..8126591 100644 --- a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/GeneratedSourceFile.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using FluentAssertions; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -25,9 +26,20 @@ private GeneratedSourceFile(string sourceCode, CompilationUnitSyntax root) public IReadOnlyList UsingDirectives => _root.Usings.Select(usingDirective => usingDirective.ToString().Trim()).ToList(); - public static GeneratedSourceFile Export(SourceFileMergeOptions options) + /// + /// Runs the exporter and compiles the result unless is false. + /// Trimming decisions can only be trusted when a compiler confirms them: an over-eager trim removes a + /// declaration that the generated file still names, which no assertion about emitted types would catch. + /// + public static GeneratedSourceFile Export(SourceFileMergeOptions options, bool validateBuild = true) { SourceFileMerger.CreateSingleSourceFile(options); + if (validateBuild) + { + GeneratedFileBuildValidator.Validate(options.TargetFramework, options.TargetFile) + .Should().Be(0, "the generated file must compile"); + } + var sourceCode = File.ReadAllText(options.TargetFile); var root = (CompilationUnitSyntax) CSharpSyntaxTree.ParseText(sourceCode).GetRoot(); return new (sourceCode, root); diff --git a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs index fb30513..0b4afc1 100644 --- a/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs +++ b/tests/Light.GuardClauses.SourceCodeTransformation.Tests/SourceFileMergerFrameworkTests.cs @@ -5,6 +5,7 @@ namespace Light.GuardClauses.SourceCodeTransformation.Tests; +[Collection(GeneratedFileBuildCollection.Name)] public static class SourceFileMergerFrameworkTests { [Fact] diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs index 156cdd2..7c3cf68 100644 --- a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/AnnotationReachabilityTrimmer.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -57,7 +58,7 @@ BundledTypeCatalog catalog var compilation = CSharpCompilation.Create( "Light.GuardClauses.SourceExportAnnotationReachability", [syntaxTree], - SourceReachabilityAnalyzer.CreateMetadataReferences(), + SourceReachabilityAnalyzer.GetMetadataReferences(), new ( OutputKind.DynamicallyLinkedLibrary, nullableContextOptions: NullableContextOptions.Enable, @@ -78,8 +79,10 @@ BundledTypeCatalog catalog var reachableTypes = new HashSet(); var typesToScan = new Queue(); - // Reachability is established from attribute usages only: the bundled types are attributes, and the - // only cref references among them are self-references that disappear with their own declaration. + // Reachability is established from attribute usages and using aliases. Any future bundled type must + // be reachable through one of these two - a type that is referenced in another position (a base type, + // a member signature, a typeof expression) would be trimmed although it is still used. The only cref + // references among the bundled types are self-references that disappear with their own declaration. foreach (var attribute in root.DescendantNodes().OfType()) { if (catalog.IsDeclaredInsideBundledType(attribute)) @@ -90,6 +93,16 @@ BundledTypeCatalog catalog MarkAttributeType(attribute); } + // A using alias names its target type, thus an alias that outlives its target does not compile even + // when nothing else refers to the alias. + foreach (var usingDirective in root.Usings) + { + if (usingDirective.Alias != null) + { + MarkAliasedType(usingDirective); + } + } + while (typesToScan.TryDequeue(out var scannedType)) { foreach (var attribute in scannedType.Declaration.DescendantNodes().OfType()) @@ -120,6 +133,28 @@ void MarkAttributeType(AttributeSyntax attribute) } } + void MarkAliasedType(UsingDirectiveSyntax usingDirective) + { + var aliasedType = GetAttributeType(semanticModel.GetSymbolInfo(usingDirective.NamespaceOrType).Symbol); + if (aliasedType is not null and not IErrorTypeSymbol) + { + if (typesBySymbol.TryGetValue(aliasedType, out var bundledType)) + { + Mark(bundledType); + } + + return; + } + + if (usingDirective.NamespaceOrType is NameSyntax aliasedName) + { + foreach (var matchingType in catalog.FindByUnqualifiedName(aliasedName)) + { + Mark(matchingType); + } + } + } + void Mark(BundledType bundledType) { if (reachableTypes.Add(bundledType)) @@ -276,14 +311,17 @@ private sealed record BundledType(MemberDeclarationSyntax Declaration, string Na /// private sealed class BundledTypeCatalog { + private readonly List _bundledNamespaces = new (); + private readonly List _types = new (); private readonly Dictionary _typesByDeclaration = new (); private readonly Dictionary> _typesByUnqualifiedName = new (StringComparer.Ordinal); + private readonly List _typesOutsideBundledNamespaces = new (); - public List Types { get; } = new (); + public IReadOnlyList Types => _types; - public List BundledNamespaces { get; } = new (); + public IReadOnlyList BundledNamespaces => _bundledNamespaces; - public List TypesOutsideBundledNamespaces { get; } = new (); + public IReadOnlyList TypesOutsideBundledNamespaces => _typesOutsideBundledNamespaces; public static BundledTypeCatalog Create(CompilationUnitSyntax root, SourceFileMergeOptions options) { @@ -293,7 +331,7 @@ public static BundledTypeCatalog Create(CompilationUnitSyntax root, SourceFileMe var namespaceName = namespaceDeclaration.Name.ToString(); if (BundledNamespaceNames.Contains(namespaceName)) { - catalog.BundledNamespaces.Add(namespaceDeclaration); + catalog._bundledNamespaces.Add(namespaceDeclaration); foreach (var member in namespaceDeclaration.Members) { catalog.Add(member, namespaceName, false); @@ -317,8 +355,11 @@ public static BundledTypeCatalog Create(CompilationUnitSyntax root, SourceFileMe return catalog; } - public bool TryGetBundledType(SyntaxNode declaration, out BundledType bundledType) => - _typesByDeclaration.TryGetValue(declaration, out bundledType!); + public bool TryGetBundledType( + SyntaxNode declaration, + [MaybeNullWhen(false)] out BundledType bundledType + ) => + _typesByDeclaration.TryGetValue(declaration, out bundledType); public bool IsDeclaredInsideBundledType(SyntaxNode node) => node.Ancestors().Any(_typesByDeclaration.ContainsKey); @@ -345,11 +386,11 @@ private void Add(MemberDeclarationSyntax declaration, string namespaceName, bool } var bundledType = new BundledType(declaration, namespaceName, name); - Types.Add(bundledType); + _types.Add(bundledType); _typesByDeclaration.Add(declaration, bundledType); if (isOutsideBundledNamespace) { - TypesOutsideBundledNamespaces.Add(bundledType); + _typesOutsideBundledNamespaces.Add(bundledType); } var unqualifiedName = CleanupStep.RemoveAttributeSuffix(name); diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs index 9d9fc40..1490641 100644 --- a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceFileMerger.cs @@ -657,16 +657,16 @@ node is ClassDeclarationSyntax ); } + Console.WriteLine("Removing configured annotation usages..."); targetRoot = CleanupStep.RemoveAnnotations(targetRoot, options); Console.WriteLine("Trimming unreferenced bundled support types..."); targetRoot = AnnotationReachabilityTrimmer.Trim(targetRoot, options); + Console.WriteLine("File is cleaned up..."); targetRoot = targetRoot.NormalizeWhitespace(eol: "\n"); var targetFileContent = targetRoot.ToFullString(); - - Console.WriteLine("File is cleaned up..."); targetFileContent = CleanupStep.CleanupFormatting(targetFileContent).ToString(); targetFileContent = targetFileContent.ReplaceLineEndings("\n"); if (!targetFileContent.EndsWith('\n')) diff --git a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs index e7b2dbc..a09c086 100644 --- a/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs +++ b/tools/source-export/Light.GuardClauses.SourceCodeTransformation/SourceReachabilityAnalyzer.cs @@ -3,6 +3,7 @@ using System.Collections.Immutable; using System.IO; using System.Linq; +using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -11,7 +12,8 @@ namespace Light.GuardClauses.SourceCodeTransformation; internal static class SourceReachabilityAnalyzer { - private static IReadOnlyList? _metadataReferences; + private static readonly Lazy> LazyMetadataReferences = + new (ResolveMetadataReferences, LazyThreadSafetyMode.ExecutionAndPublication); public static CSharpParseOptions CreateParseOptions(SourceTargetFramework targetFramework) => new ( @@ -29,16 +31,13 @@ public static CSharpParseOptions CreateParseOptions(SourceTargetFramework target ); /// - /// Creates the metadata references that every compilation of the exporter uses. The references are + /// Gets the metadata references that every compilation of the exporter uses. The references are /// resolved once because collecting them requires probing the whole trusted platform assembly list. /// - public static IReadOnlyList CreateMetadataReferences() - { - if (_metadataReferences != null) - { - return _metadataReferences; - } + public static IReadOnlyList GetMetadataReferences() => LazyMetadataReferences.Value; + private static IReadOnlyList ResolveMetadataReferences() + { var referencesByPath = new Dictionary(StringComparer.OrdinalIgnoreCase); if (AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is string trustedPlatformAssemblies) @@ -53,7 +52,7 @@ public static IReadOnlyList CreateMetadataReferences() AddReference(referencesByPath, typeof(Enumerable).Assembly.Location); AddReference(referencesByPath, typeof(ImmutableArray).Assembly.Location); - return _metadataReferences = referencesByPath.Values.ToArray(); + return referencesByPath.Values.ToArray(); } private static void AddReference( @@ -466,7 +465,7 @@ public static SourceCatalog Create(IEnumerable files, CSharpParseOptio var compilation = CSharpCompilation.Create( "Light.GuardClauses.SourceExportReachability", sourceFiles.Select(sourceFile => sourceFile.SyntaxTree), - CreateMetadataReferences(), + GetMetadataReferences(), new ( OutputKind.DynamicallyLinkedLibrary, nullableContextOptions: NullableContextOptions.Enable,