From 8509bf3d8718a820071605fa4fcce9a948df2fa1 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Thu, 24 Sep 2026 19:33:27 +0100 Subject: [PATCH] fix: sg leaking types --- docs/wiki/Analyzers.md | 4 +- docs/wiki/Packaging.md | 47 ++- docs/wiki/Testing-TUnit.md | 47 +++ docs/wiki/Type-Library.md | 15 +- docs/wiki/index.md | 6 + mkdocs.yml | 21 + package.json | 2 +- purview-build.json | 3 + .../AnalyzerReleases.Unshipped.md | 2 +- ...cs => UnqualifiedFrameworkCrefAnalyzer.cs} | 35 +- ...lineCodeForFrameworkCrefCodeFixProvider.cs | 176 +++++++++ .../QualifyFrameworkCrefCodeFixProvider.cs | 77 ---- .../Helpers/FrameworkCrefRewriter.cs | 219 +++++++++++ .../Helpers/SourceEmitter.TypeLibrary.cs | 5 + .../Helpers/TypeLibraryModelLibrary.cs | 127 +++++- .../FrameworkTypeInternalizer.cs | 360 ++++++++++++++++++ .../MergeToolRunner.cs | 17 + .../SourceGeneratorFramework/Sdk/README.md | 2 +- ... UnqualifiedFrameworkCrefAnalyzerTests.cs} | 50 ++- ...odeForFrameworkCrefCodeFixProviderTests.cs | 105 +++++ ...ualifyFrameworkCrefCodeFixProviderTests.cs | 93 ----- .../TypeLibraryGeneratorTests.cs | 38 ++ .../FrameworkTypeInternalizerTests.cs | 243 ++++++++++++ .../MergeToolRunnerTests.cs | 113 +----- ...eratorFramework.MergeTool.UnitTests.csproj | 5 + .../TestWorkspace.cs | 122 ++++++ 26 files changed, 1613 insertions(+), 321 deletions(-) create mode 100644 docs/wiki/index.md create mode 100644 mkdocs.yml rename src/src/SourceGeneratorFramework.Analyzers/{AmbiguousFrameworkCrefAnalyzer.cs => UnqualifiedFrameworkCrefAnalyzer.cs} (82%) create mode 100644 src/src/SourceGeneratorFramework.CodeFixers/PreferInlineCodeForFrameworkCrefCodeFixProvider.cs delete mode 100644 src/src/SourceGeneratorFramework.CodeFixers/QualifyFrameworkCrefCodeFixProvider.cs create mode 100644 src/src/SourceGeneratorFramework.Generators/Helpers/FrameworkCrefRewriter.cs create mode 100644 src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs rename src/tests/SourceGeneratorFramework.Analyzers.UnitTests/{AmbiguousFrameworkCrefAnalyzerTests.cs => UnqualifiedFrameworkCrefAnalyzerTests.cs} (73%) create mode 100644 src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/PreferInlineCodeForFrameworkCrefCodeFixProviderTests.cs delete mode 100644 src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/QualifyFrameworkCrefCodeFixProviderTests.cs create mode 100644 src/tests/SourceGeneratorFramework.MergeTool.UnitTests/FrameworkTypeInternalizerTests.cs create mode 100644 src/tests/SourceGeneratorFramework.MergeTool.UnitTests/TestWorkspace.cs diff --git a/docs/wiki/Analyzers.md b/docs/wiki/Analyzers.md index 740c78c..4723a6d 100644 --- a/docs/wiki/Analyzers.md +++ b/docs/wiki/Analyzers.md @@ -51,7 +51,7 @@ The analyzers enforce two families of rules: | `PSGFR37` | One extension class per receiver type; split classes that extend multiple types. | | `PSGFR38` | Extension classes should carry `[EditorBrowsable(EditorBrowsableState.Never)]`. | | `PSGFR39` | A non-packable Roslyn component that explicitly opts out of the default self-contained analyzer output (`PurviewMergeSourceGeneratorFrameworkForAnalyzerFiles=false`) while embedding the framework, otherwise the package embeds the loose framework DLL under `analyzers/`. | -| `PSGFR40` | In Roslyn components (`IsRoslynComponent=true`), qualify XML doc `cref` references to SGF public types with `global::Purview.SourceGeneratorFramework...`. | +| `PSGFR40` | In Roslyn components (`IsRoslynComponent=true`), reference SGF types as inline code (`Type`) instead of a `cref`: copied documentation must not depend on cref resolution. | ## Type-library and attribute-model diagnostics @@ -81,7 +81,7 @@ analyzer rules above, including: - `PreferStructuredCodeWriterIfBlockCodeFixProvider` — rewrites raw `if`/`else if`/`else` block text to the structured `IfBlock`/`ElseIf`/`Else` APIs (`PSGFR23`). - `CodeWriterToStringCodeFixProvider` — replaces embedded `CodeWriter` string interpolation (`PSGFR29`). -- `QualifyFrameworkCrefCodeFixProvider` — rewrites SGF XML doc `cref` targets to fully qualified `global::Purview.SourceGeneratorFramework...` names (`PSGFR40`). +- `PreferInlineCodeForFrameworkCrefCodeFixProvider` — rewrites SGF XML doc `cref` targets to inline code (`Type`) (`PSGFR40`). - `AttributeDataModelSymbolPropertyCodeFixProvider` — fixes attribute-data-model symbol properties. - `ReorganizeExtensionClassCodeFixProvider` — renames (`PSGFR35`), splits multi-receiver classes (`PSGFR37`), moves the class under `Extensions/{ReceiverNamespace}/`, and updates referencing files diff --git a/docs/wiki/Packaging.md b/docs/wiki/Packaging.md index a3ff57b..fc96158 100644 --- a/docs/wiki/Packaging.md +++ b/docs/wiki/Packaging.md @@ -115,7 +115,40 @@ substituting the merged path into `TargetPathWithTargetPlatformMoniker` immediat runs, leaving `GetTargetPath` — which resolves assembly references — pointing at the unmerged bin. This is what keeps the in-repo test harness working while shipped assemblies stay self-contained. -### Self-contained analyzer validation (PSGFR39) +### Framework type internalization in merged components + +ILRepack's `Internalize` is best-effort: a framework type that reaches the merged component's public +API surface stays public, and the types the framework's own generators emit into the component (the +`Purview.SourceGeneratorFramework.Generators` attribute set, generated type libraries, marker +attributes) are not part of the merged framework assembly at all, so ILRepack never sees them. Either +gap leaks framework types out of what must be a self-contained analyzer, and any project that loads +that analyzer alongside the real framework assembly fails with `CS0433` ambiguity for every leaked +type. + +The merge tool therefore runs a deterministic internalization pass over the merged output +(`FrameworkTypeInternalizer`) after `ILRepack` finishes: + +- every type in a framework-owned namespace (`Purview.SourceGeneratorFramework` and its children, + including the generated `Generators` attribute set) becomes non-public; +- `Microsoft.CodeAnalysis.EmbeddedAttribute` (the framework-emitted marker) becomes non-public; +- **Roslyn component entry points are never internalized** — a generator, analyzer, code fix + provider, or refactoring provider that is reachable from the framework namespace stays public, + because Roslyn only instantiates public components (PSGFR27). This is what keeps the framework's + own bundled analyzers working after the pass; +- the pass reports the merge result: leftover public framework types fail the merge (exit code `5`), + and public component members whose signature exposes a framework type are logged as warnings so the + component author can make them (or their declaring type) non-public. + +The component's own generated types (the type library, attribute data models) keep their accessibility +in the component assembly: TLB0015 requires a hand-written partial to be declared +`public static partial` so it can merge with the generated library, and in-repo consumers such as code +fixers and sibling assemblies compile against it. Self-containment is therefore enforced at the merge +boundary rather than by rewriting generated accessibility. See [Type-Library.md](Type-Library.md). + +> A merged component is an analyzer artifact and must never be referenced as a compile-time +> dependency. Tests that need to run a *packaged* generator load it out of band — see +> [Testing-TUnit.md](Testing-TUnit.md). + The bundled `SelfContainedGeneratorAnalyzer` (PSGFR39) runs on every project that references the framework and errors when a **non-packable** Roslyn component explicitly opts out of the default @@ -303,9 +336,15 @@ The exact Roslyn baseline is a product-support decision. The following checks are the acceptance criteria for the self-contained packaging: -1. **No assembly reference** — every shipped Roslyn component DLL - (`analyzers/dotnet/cs/*.dll`) has no assembly reference to `Purview.SourceGeneratorFramework`. - Inspect the metadata directly; do not rely on "the sample compiled". +1. **No assembly reference and no public framework types** — every shipped Roslyn component DLL + (`analyzers/dotnet/cs/*.dll`) has no assembly reference to `Purview.SourceGeneratorFramework` and + exposes **no public type** in a framework-owned namespace (`Purview.SourceGeneratorFramework` and + its children, including the generated `Generators` attribute set, plus the framework-emitted + `Microsoft.CodeAnalysis.EmbeddedAttribute`). The only exception is the component's own Roslyn + component entry points, which must stay public because Roslyn only instantiates public components. + Inspect the metadata directly; do not rely on "the sample compiled". The merge tool fails with exit + code `5` when a merge leaves public framework types behind, and logs a warning naming any public + member that still exposes a framework type. 2. **No loose framework DLL in packages** — no `.nupkg` contains `Purview.SourceGeneratorFramework.dll` under `analyzers/`, and `*.pdb` files are forbidden in the `.nupkg` (symbols ship only through the `.snupkg`). diff --git a/docs/wiki/Testing-TUnit.md b/docs/wiki/Testing-TUnit.md index a2c2fbc..b45a9b1 100644 --- a/docs/wiki/Testing-TUnit.md +++ b/docs/wiki/Testing-TUnit.md @@ -94,6 +94,53 @@ that supports its API usage; this framework is built against Roslyn 5.0, which s generator as an analyzer must be Roslyn 5.0 or later (`.NET 10` SDK / Visual Studio 2026). Do not force a newer `System.Collections.Immutable` version through central package management. +## Running a packaged (merged) generator in tests + +A component that ships as a self-contained analyzer must **not** be added as a compile-time +``. Its assembly carries the framework implementation merged into itself, and — because +ILRepack cannot internalize a framework type that reaches the component's public API surface — some +packages expose framework types publicly. Referencing such an assembly from a test project that also +loads the real `Purview.SourceGeneratorFramework.dll` (which the Testing packages do) makes every +framework type ambiguous (`CS0433`). From framework `1.0.0-prerelease.51` the merge tool guarantees a +merged component exposes no framework types apart from its own Roslyn entry points, but a merged +component remains an analyzer artifact and should still be consumed out of band. + +To register a packaged generator as an additional generator/analyzer: + +1. copy the analyzer DLL from the package beside the test binaries, without referencing it: + +```xml + + + + + +``` + +2. resolve the types out of band and pass them through the options: + +```csharp +var assembly = Assembly.LoadFrom(Path.Combine(AppContext.BaseDirectory, "My.Generator.dll")); + +options.AdditionalGeneratorTypes = + [.. options.AdditionalGeneratorTypes, assembly.GetType("My.Namespace.MyGenerator", throwOnError: true)!]; +options.AnalyzerTypes = [assembly.GetType("My.Namespace.MyAnalyzer", throwOnError: true)!]; +``` + +`SourceGeneratorTestRunner` instantiates the supplied types with `Activator.CreateInstance`, so this is +equivalent to `typeof(...)` without the compile-time reference. Two consequences: the loaded +generator's framework copy owns its own logging registry and CodeWriter scope validation (do not +assert on its `LogEntries`, and leave `ValidateCodeWriterScopes` off for that run), and the loaded +assembly must be built against a Roslyn version compatible with the test host. + +For a component in the same repository, prefer a project reference to the component project: it +resolves the **unmerged** bin output plus the loose framework DLL, so framework types keep a single +identity and `typeof(...)`, `InternalsVisibleTo`, and every assertion API keep working. + ## Which base class and method | Roslyn type | Base class | Method | diff --git a/docs/wiki/Type-Library.md b/docs/wiki/Type-Library.md index d576dfd..3d0d611 100644 --- a/docs/wiki/Type-Library.md +++ b/docs/wiki/Type-Library.md @@ -12,6 +12,19 @@ generated type library) whose nested `public static partial` classes mirror the members. Every class — the root and each nested namespace class — exposes a `public const string Namespace` and the leaf classes expose the members as `public static readonly` fields: +The generated type library is always `public` in the component's own assembly so a hand-written +partial can merge with it (TLB0015 enforces the matching `public static partial` declaration) and so +in-repo consumers such as code fixers and sibling assemblies can compile against it. When the +component is packaged, the merge tool internalizes every framework-owned type in the shipped analyzer +(including the `TypeIdentity`/`TypeReference`/`PurviewTypeLibrary` members this library exposes), so +nothing leaks out of the package; see [Packaging.md](Packaging.md). + +Author documentation is copied into the generated library. A `cref` that targets a framework type is +rendered as inline code (`TypeReference`) while it is copied, because the generated file's +namespace and using set differ from the author's source and an unresolvable cref would produce +`CS1574`. `PSGFR40` reports the same pattern in the editor and its code fix applies the same +rewrite. + ```csharp namespace MyGenerator; @@ -32,7 +45,7 @@ public static partial class TypeLibrary ``` No `extension(...)` blocks are emitted. The generated types are `public static partial` so you can expand -them with your own methods in a separate partial file — but the extension partial must be declared +them with your own methods in a separate partial file — the extension partial must be declared `public static partial` in the **same** namespace as the generated type. The generated type is emitted in the namespace given by the `Namespace` argument, or the **global namespace** when it is omitted, so a partial declared inside your project namespace will not merge with it (it silently shadows the generated diff --git a/docs/wiki/index.md b/docs/wiki/index.md new file mode 100644 index 0000000..142040b --- /dev/null +++ b/docs/wiki/index.md @@ -0,0 +1,6 @@ +# Source Generator Framework + +Reusable foundations for building, testing, packaging, and optimizing Roslyn source generators. + +[Documentation overview](Home.md){ .md-button .md-button--primary } +[Get started](Getting-Started.md){ .md-button } diff --git a/mkdocs.yml b/mkdocs.yml new file mode 100644 index 0000000..521e987 --- /dev/null +++ b/mkdocs.yml @@ -0,0 +1,21 @@ +site_name: Source Generator Framework +site_description: Developer documentation for the Purview Source Generator Framework +repo_url: https://github.com/purview-dev/sourcegenerator-framework +edit_uri: edit/main/docs/wiki/ +docs_dir: docs/wiki + +theme: + name: material + palette: + - media: "(prefers-color-scheme: light)" + scheme: default + primary: indigo + accent: cyan + - media: "(prefers-color-scheme: dark)" + scheme: slate + primary: indigo + accent: cyan + features: [navigation.instant, navigation.sections, navigation.top, search.highlight, search.suggest, content.code.copy] + +plugins: [search, techdocs-core] +markdown_extensions: [admonition, attr_list, md_in_html, pymdownx.details, pymdownx.superfences] diff --git a/package.json b/package.json index 168f5f4..ed999bc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "purview-sourcegenerator-framework", - "version": "1.0.0-prerelease.50", + "version": "1.0.0-prerelease.51", "license": "MIT", "author": { "name": "Kieron Lanning", diff --git a/purview-build.json b/purview-build.json index 8d3e035..3d71580 100644 --- a/purview-build.json +++ b/purview-build.json @@ -46,6 +46,9 @@ "purview-logo-light.png", "README.md" ] + }, + "ForbiddenContent": { + "*": ["analyzers/**/Purview.SourceGeneratorFramework.dll"] } }, "Release": { diff --git a/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md b/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md index 39dcd15..cafe263 100644 --- a/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/src/SourceGeneratorFramework.Analyzers/AnalyzerReleases.Unshipped.md @@ -15,7 +15,7 @@ PSGFR36 | Purview.SourceGeneratorFramework | Warning | Extension class is not pl PSGFR37 | Purview.SourceGeneratorFramework | Warning | Extension class extends multiple receiver types PSGFR38 | Purview.SourceGeneratorFramework | Warning | Extension class is missing EditorBrowsable PSGFR39 | Purview.SourceGeneratorFramework | Error | Roslyn component must produce a self-contained analyzer -PSGFR40 | Purview.SourceGeneratorFramework | Warning | Unqualified SGF cref is ambiguous +PSGFR40 | Purview.SourceGeneratorFramework | Warning | SGF cref should be inline code TLB0014 | TypeLibrary | Warning | Type library partial extension is declared in a different namespace TLB0015 | TypeLibrary | Info | Type library partial extension must be declared 'public static partial' TLB0016 | TypeLibrary | Error | Enum value member type must be TypeIdentity or EnumValueDefinition diff --git a/src/src/SourceGeneratorFramework.Analyzers/AmbiguousFrameworkCrefAnalyzer.cs b/src/src/SourceGeneratorFramework.Analyzers/UnqualifiedFrameworkCrefAnalyzer.cs similarity index 82% rename from src/src/SourceGeneratorFramework.Analyzers/AmbiguousFrameworkCrefAnalyzer.cs rename to src/src/SourceGeneratorFramework.Analyzers/UnqualifiedFrameworkCrefAnalyzer.cs index 19f47aa..0cd56ba 100644 --- a/src/src/SourceGeneratorFramework.Analyzers/AmbiguousFrameworkCrefAnalyzer.cs +++ b/src/src/SourceGeneratorFramework.Analyzers/UnqualifiedFrameworkCrefAnalyzer.cs @@ -7,25 +7,32 @@ namespace Purview.SourceGeneratorFramework.Analyzers; /// -/// Flags unqualified XML documentation cref references to SGF public types in Roslyn components. -/// Qualifying these cref targets avoids the duplicate-framework ambiguity that can arise when the -/// same SGF type is visible through multiple assembly identities. +/// Flags unqualified XML documentation cref references to SGF public types in Roslyn components, and +/// points at inline code (<c>) instead of a cref. +/// +/// A cref has to resolve in every compilation that contains the documentation. Component documentation +/// is copied into generated code (TypeLibraryGenerator re-emits the spec, member, and enum-value +/// docs) whose namespace and using set differ from the author's source, so a cref to an SGF type can +/// end up unresolvable (CS1574) or resolve through a second assembly identity — the duplicate-framework +/// ambiguity this rule was originally added for. Inline code carries the type as text, which never +/// depends on a symbol, a namespace, or an assembly identity. +/// /// [DiagnosticAnalyzer(LanguageNames.CSharp)] -public sealed class AmbiguousFrameworkCrefAnalyzer : DiagnosticAnalyzer +public sealed class UnqualifiedFrameworkCrefAnalyzer : DiagnosticAnalyzer { public const string DiagnosticId = "PSGFR40"; - internal const string QualifiedTypePropertyName = "QualifiedTypeName"; + const string FrameworkAssemblyName = "Purview.SourceGeneratorFramework"; static readonly DiagnosticDescriptor Rule = new( DiagnosticId, - "Qualify SGF cref with global::", - "XML documentation cref '{0}' refers to SGF type '{1}'; qualify the cref with 'global::'", + "Reference SGF types as inline code", + "XML documentation cref '{0}' refers to SGF type '{1}'; use {1} so the documentation stays resolvable when it is copied into generated code", "Purview.SourceGeneratorFramework", DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: "Detects unqualified XML documentation cref references to Purview.SourceGeneratorFramework public types in Roslyn components so they can be rewritten to a fully qualified global:: name." + description: "Detects XML documentation cref references to Purview.SourceGeneratorFramework public types in Roslyn components so they can be rewritten to inline code, which stays valid wherever the documentation is emitted." ); public override ImmutableArray SupportedDiagnostics => [Rule]; @@ -90,16 +97,8 @@ ImmutableDictionary publicFrameworkTypes if (!ReferencesFrameworkType(semanticModel, cref.Cref, frameworkType, context.CancellationToken)) continue; - var qualifiedTypeName = frameworkType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - var diagnostic = Diagnostic.Create( - Rule, - cref.GetLocation(), - ImmutableDictionary.Empty.Add(QualifiedTypePropertyName, qualifiedTypeName), - cref.Cref.ToString(), - qualifiedTypeName.Substring("global::".Length) - ); - - context.ReportDiagnostic(diagnostic); + var crefText = cref.Cref.ToString(); + context.ReportDiagnostic(Diagnostic.Create(Rule, cref.GetLocation(), crefText, crefText)); } } } diff --git a/src/src/SourceGeneratorFramework.CodeFixers/PreferInlineCodeForFrameworkCrefCodeFixProvider.cs b/src/src/SourceGeneratorFramework.CodeFixers/PreferInlineCodeForFrameworkCrefCodeFixProvider.cs new file mode 100644 index 0000000..468c396 --- /dev/null +++ b/src/src/SourceGeneratorFramework.CodeFixers/PreferInlineCodeForFrameworkCrefCodeFixProvider.cs @@ -0,0 +1,176 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using Purview.SourceGeneratorFramework.Analyzers; + +namespace Purview.SourceGeneratorFramework.CodeFixers; + +/// +/// Replaces an SGF XML documentation cref (<see cref="TypeReference"/>) with inline code +/// (<c>TypeReference</c>). Inline code carries the type as text, so the documentation +/// stays valid when the framework's generators copy it into generated files whose namespace and using +/// set differ from the author's source. +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferInlineCodeForFrameworkCrefCodeFixProvider))] +public sealed class PreferInlineCodeForFrameworkCrefCodeFixProvider : CodeFixProvider +{ + internal const string EquivalenceKey = "PreferInlineCodeForSgfType"; + + public override ImmutableArray FixableDiagnosticIds => [UnqualifiedFrameworkCrefAnalyzer.DiagnosticId]; + + // A single-pass fix-all: the cref element is replaced in one rewrite, so no fix is reapplied against + // a document whose spans have already shifted. + public override FixAllProvider GetFixAllProvider() => InlineCodeFixAllProvider.Instance; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + return; + + foreach (var diagnostic in context.Diagnostics) + { + var node = root.FindNode( + diagnostic.Location.SourceSpan, + getInnermostNodeForTie: true, + findInsideTrivia: true + ); + var crefAttribute = + node as XmlCrefAttributeSyntax + ?? node.AncestorsAndSelf().OfType().FirstOrDefault(); + if (crefAttribute is null) + continue; + + var inlineText = crefAttribute.Cref.ToString(); + var crefSpan = crefAttribute.Span; + context.RegisterCodeFix( + CodeAction.Create( + $"Use {inlineText}", + cancellationToken => UseInlineCodeAsync(context.Document, crefSpan, inlineText, cancellationToken), + EquivalenceKey + ), + diagnostic + ); + } + } + + static async Task UseInlineCodeAsync( + Document document, + TextSpan crefSpan, + string inlineText, + CancellationToken cancellationToken + ) + { + // Resolve the cref against the current document: the batch fixer applies fixes one after + // another (latest span first), so a root captured while registering would be stale. + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + return document; + + var node = root.FindNode(crefSpan, getInnermostNodeForTie: true, findInsideTrivia: true); + var crefAttribute = + node as XmlCrefAttributeSyntax ?? node.AncestorsAndSelf().OfType().FirstOrDefault(); + if (crefAttribute is null) + return document; + + // The cref attribute lives on the / element; inline code replaces the element. + var owner = + crefAttribute + .AncestorsAndSelf() + .FirstOrDefault(static ancestor => ancestor is XmlElementSyntax or XmlEmptyElementSyntax) + as XmlNodeSyntax; + if (owner is null) + return document; + + return document.WithSyntaxRoot(root.ReplaceNode(owner, BuildInlineCodeElement(owner, inlineText))); + } + + static XmlElementSyntax BuildInlineCodeElement(XmlNodeSyntax owner, string inlineText) + { + var name = SyntaxFactory.XmlName(SyntaxFactory.Identifier("c")); + + // `shown` keeps the author's text; an empty element shows the type name. + var content = + owner is XmlElementSyntax element && element.Content.Any(static node => !IsWhitespace(node)) + ? element.Content + : SyntaxFactory.SingletonList(SyntaxFactory.XmlText(inlineText)); + + return SyntaxFactory.XmlElement(name, content).WithTriviaFrom(owner); + } + + static bool IsWhitespace(XmlNodeSyntax node) => + node is XmlTextSyntax text && text.TextTokens.All(static token => string.IsNullOrWhiteSpace(token.ValueText)); + + /// + /// Rewrites every reported SGF cref in a document in a single pass. + /// + sealed class InlineCodeFixAllProvider : FixAllProvider + { + public static readonly InlineCodeFixAllProvider Instance = new(); + + public override Task GetFixAsync(FixAllContext fixAllContext) + { + var document = fixAllContext.Document ?? fixAllContext.Project.Documents.FirstOrDefault(); + if (document is null) + return Task.FromResult(null); + + return Task.FromResult( + CodeAction.Create( + "Use inline code for SGF cref references", + cancellationToken => FixAllAsync(document, fixAllContext, cancellationToken), + EquivalenceKey + ) + ); + } + + static async Task FixAllAsync( + Document document, + FixAllContext fixAllContext, + CancellationToken cancellationToken + ) + { + var diagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false); + if (diagnostics.IsEmpty) + return document; + + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + return document; + + List owners = []; + foreach (var diagnostic in diagnostics) + { + var node = root.FindNode( + diagnostic.Location.SourceSpan, + getInnermostNodeForTie: true, + findInsideTrivia: true + ); + var owner = + node.AncestorsAndSelf() + .FirstOrDefault(static ancestor => ancestor is XmlElementSyntax or XmlEmptyElementSyntax) + as XmlNodeSyntax; + + if (owner is not null) + owners.Add(owner); + } + + if (owners.Count == 0) + return document; + + return document.WithSyntaxRoot( + root.ReplaceNodes( + owners, + static (original, _) => BuildInlineCodeElement(original, GetCrefText(original)) + ) + ); + } + + static string GetCrefText(XmlNodeSyntax owner) => + owner.DescendantNodesAndSelf().OfType().FirstOrDefault()?.Cref.ToString() + ?? string.Empty; + } +} diff --git a/src/src/SourceGeneratorFramework.CodeFixers/QualifyFrameworkCrefCodeFixProvider.cs b/src/src/SourceGeneratorFramework.CodeFixers/QualifyFrameworkCrefCodeFixProvider.cs deleted file mode 100644 index 0fb4900..0000000 --- a/src/src/SourceGeneratorFramework.CodeFixers/QualifyFrameworkCrefCodeFixProvider.cs +++ /dev/null @@ -1,77 +0,0 @@ -using System.Collections.Immutable; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Purview.SourceGeneratorFramework.Analyzers; - -namespace Purview.SourceGeneratorFramework.CodeFixers; - -/// -/// Qualifies SGF XML documentation cref targets with their global:: name. -/// -[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(QualifyFrameworkCrefCodeFixProvider))] -public sealed class QualifyFrameworkCrefCodeFixProvider : CodeFixProvider -{ - internal const string EquivalenceKey = "QualifyFrameworkCref"; - - public override ImmutableArray FixableDiagnosticIds => [AmbiguousFrameworkCrefAnalyzer.DiagnosticId]; - - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - if (root is null) - return; - - foreach (var diagnostic in context.Diagnostics) - { - if ( - !diagnostic.Properties.TryGetValue( - AmbiguousFrameworkCrefAnalyzer.QualifiedTypePropertyName, - out var qualifiedTypeName - ) || string.IsNullOrWhiteSpace(qualifiedTypeName) - ) - continue; - - var resolvedQualifiedTypeName = qualifiedTypeName!; - - var node = root.FindNode( - diagnostic.Location.SourceSpan, - getInnermostNodeForTie: true, - findInsideTrivia: true - ); - var crefAttribute = - node as XmlCrefAttributeSyntax - ?? node.AncestorsAndSelf().OfType().FirstOrDefault(); - if (crefAttribute is null) - continue; - - context.RegisterCodeFix( - CodeAction.Create( - "Qualify SGF cref with global::", - _ => QualifyCrefAsync(context.Document, root, crefAttribute, resolvedQualifiedTypeName), - EquivalenceKey - ), - diagnostic - ); - } - } - - static Task QualifyCrefAsync( - Document document, - SyntaxNode root, - XmlCrefAttributeSyntax crefAttribute, - string qualifiedTypeName - ) - { - var replacement = crefAttribute - .WithCref(SyntaxFactory.TypeCref(SyntaxFactory.ParseTypeName(qualifiedTypeName))) - .WithTriviaFrom(crefAttribute); - - var newRoot = root.ReplaceNode(crefAttribute, replacement); - return Task.FromResult(document.WithSyntaxRoot(newRoot)); - } -} diff --git a/src/src/SourceGeneratorFramework.Generators/Helpers/FrameworkCrefRewriter.cs b/src/src/SourceGeneratorFramework.Generators/Helpers/FrameworkCrefRewriter.cs new file mode 100644 index 0000000..c9fc556 --- /dev/null +++ b/src/src/SourceGeneratorFramework.Generators/Helpers/FrameworkCrefRewriter.cs @@ -0,0 +1,219 @@ +using System.Text; +using System.Xml; +using System.Xml.Linq; + +namespace Purview.SourceGeneratorFramework.Generators.Helpers; + +/// +/// Rewrites references to framework-owned types in documentation that is copied into generated code. +/// +/// A cref has to resolve to a symbol in every compilation that contains the documentation, and +/// copied documentation lands in generated files whose namespace and using set differ from the +/// author's source (a generated type library, for example, imports only +/// global::Purview.SourceGeneratorFramework). An unresolvable cref produces CS1574 in the +/// generated file, and a cref that resolves through another assembly identity produces exactly the +/// ambiguity the PSGFR40 analyzer warns about. +/// +/// +/// Rendering the type as inline code (<c>) removes that dependency entirely: the text is +/// documentation, never a symbol reference. Matching is name-based because documentation XML carries no +/// resolved symbols, so a cref whose qualifier is neither absent nor the framework namespace keeps its +/// original form. +/// +/// +static class FrameworkCrefRewriter +{ + const string FrameworkNamespace = "Purview.SourceGeneratorFramework"; + const string GlobalAlias = "global::"; + + /// + /// Rewrites every framework-type cref found under into an inline + /// code element, in place. + /// + /// A documentation element (for example the captured summary). + /// Public type names declared by the framework assembly. + public static void Rewrite(XElement element, EquatableArray frameworkTypeNames) + { + if (frameworkTypeNames.IsEmpty) + return; + + var crefs = element.DescendantsAndSelf().Where(IsCrefElement).ToList(); + foreach (var cref in crefs) + { + var crefText = (string?)cref.Attribute("cref"); + if (crefText is null || !TryGetInlineText(crefText, frameworkTypeNames, out var inlineText)) + continue; + + // Preserve author-supplied content (`shown`); otherwise show the type. + cref.ReplaceWith(new XElement("c", cref.Nodes().Any() ? cref.Nodes().ToArray() : [new XText(inlineText)])); + } + } + + /// + /// Rewrites framework-type crefs in a captured documentation fragment, returning the original text + /// when it is empty, contains no framework cref, or is not well-formed XML. + /// + public static string? RewriteDocumentation(string? documentation, EquatableArray frameworkTypeNames) + { + if (string.IsNullOrWhiteSpace(documentation) || frameworkTypeNames.IsEmpty) + return documentation; + + try + { + var wrapper = XElement.Parse( + "" + documentation + "", + LoadOptions.PreserveWhitespace + ); + Rewrite(wrapper, frameworkTypeNames); + + var result = string.Join( + "\n", + wrapper.Elements().Select(static element => element.ToString(SaveOptions.DisableFormatting)) + ) + .Trim(); + + return result.Length == 0 ? documentation : result; + } + catch (XmlException) + { + return documentation; + } + } + + static bool IsCrefElement(XElement element) => + (element.Name.LocalName == "see" || element.Name.LocalName == "seealso") + && element.Attribute("cref") is not null; + + /// + /// Returns the text to render as inline code when names a framework type. + /// + static bool TryGetInlineText(string crefText, EquatableArray frameworkTypeNames, out string inlineText) + { + inlineText = string.Empty; + + var text = crefText.Trim(); + if (text.StartsWith(GlobalAlias, StringComparison.Ordinal)) + text = text.Substring(GlobalAlias.Length); + + // Roslyn expands crefs when documentation is captured (`T:Namespace.Type`, + // `M:Namespace.Type.Member(...)`), so the declaration-id prefix is stripped before matching. + text = StripIdPrefix(text); + + var segments = SplitSegments(text); + if (segments.Length == 0) + return false; + + for (var index = 0; index < segments.Length; index++) + { + var segment = StripCallArguments(segments[index]); + if (segment.Length == 0) + continue; + + var identifierLength = segment.IndexOf('{'); + var identifier = identifierLength < 0 ? segment : segment.Substring(0, identifierLength); + if ( + identifier.Length == 0 + || !frameworkTypeNames.AsImmutableArray().Contains(identifier, StringComparer.Ordinal) + ) + continue; + + // Accept a bare type name, a member cref whose receiver is a framework type + // (`CodeWriter.Write`), or a cref written through the framework namespace. Any other + // qualifier means the cref targets something else that merely shares the name. + if (index > 0 && !HasFrameworkQualifier(segments, index)) + return false; + + inlineText = BuildInlineText(segments, index); + return true; + } + + return false; + } + + /// + /// Renders the matched type plus any member path as written, dropping call arguments so a cref such + /// as CodeWriter.Write(System.String) reads as CodeWriter.Write. + /// + static string BuildInlineText(string[] segments, int matchedIndex) + { + StringBuilder builder = new(segments[matchedIndex]); + + for (var index = matchedIndex + 1; index < segments.Length; index++) + { + var raw = segments[index]; + var argumentIndex = raw.IndexOf('('); + var text = argumentIndex < 0 ? raw : raw.Substring(0, argumentIndex); + if (text.Length == 0) + break; + + builder.Append('.').Append(text); + if (argumentIndex >= 0) + break; + } + + return builder.ToString(); + } + + /// + /// Strips the documentation-comment declaration id prefix (T:, M:, P:, ...) that + /// Roslyn adds when it expands a cref. + /// + static string StripIdPrefix(string text) + { + if (text.Length < 2 || text[1] != ':') + return text; + + return text[0] switch + { + 'N' or 'T' or 'F' or 'P' or 'M' or 'E' or 'O' or 'C' or '!' => text.Substring(2).TrimStart(), + _ => text, + }; + } + + static string StripCallArguments(string segment) + { + var parenthesis = segment.IndexOf('('); + return parenthesis < 0 ? segment : segment.Substring(0, parenthesis); + } + + /// + /// Splits a cref on method/member separators only, keeping generic argument lists intact so a name + /// such as ImmutableArray{System.String} is not mistaken for a framework type. + /// + static string[] SplitSegments(string text) + { + List segments = []; + var start = 0; + var depth = 0; + + for (var index = 0; index < text.Length; index++) + { + switch (text[index]) + { + case '{' or '(' or '[': + depth++; + break; + case '}' or ')' or ']': + if (depth > 0) + depth--; + break; + case '.' when depth == 0: + segments.Add(text.Substring(start, index - start)); + start = index + 1; + break; + default: + break; + } + } + + segments.Add(text.Substring(start)); + return [.. segments]; + } + + static bool HasFrameworkQualifier(string[] segments, int matchedIndex) + { + var prefix = string.Join(".", segments, 0, matchedIndex); + return prefix.Equals(FrameworkNamespace, StringComparison.Ordinal) + || prefix.StartsWith(FrameworkNamespace + ".", StringComparison.Ordinal); + } +} diff --git a/src/src/SourceGeneratorFramework.Generators/Helpers/SourceEmitter.TypeLibrary.cs b/src/src/SourceGeneratorFramework.Generators/Helpers/SourceEmitter.TypeLibrary.cs index 8352e8c..13d4ee1 100644 --- a/src/src/SourceGeneratorFramework.Generators/Helpers/SourceEmitter.TypeLibrary.cs +++ b/src/src/SourceGeneratorFramework.Generators/Helpers/SourceEmitter.TypeLibrary.cs @@ -385,6 +385,11 @@ public static SourceText EmitTypeLibrary(TypeLibraryModel model) ?? $"Represents the {model.ClassName} type library, containing type identity/ reference information." ); + // The generated type library is deliberately public: the framework's TLB0015 validation + // requires a hand-written partial to be declared `public static partial` so the two can merge, + // and in-repo consumers (code fixers, sibling assemblies) compile against it. Self-containment + // is enforced at the merge boundary instead — see FrameworkTypeInternalizer, which + // internalizes every framework-owned type in the shipped analyzer. TypeDeclarationOptions options = new(model.ClassName, TypeDeclarationAccessibility.Public) { IsStatic = true, diff --git a/src/src/SourceGeneratorFramework.Generators/Helpers/TypeLibraryModelLibrary.cs b/src/src/SourceGeneratorFramework.Generators/Helpers/TypeLibraryModelLibrary.cs index 518a7e9..c5b52ec 100644 --- a/src/src/SourceGeneratorFramework.Generators/Helpers/TypeLibraryModelLibrary.cs +++ b/src/src/SourceGeneratorFramework.Generators/Helpers/TypeLibraryModelLibrary.cs @@ -14,9 +14,10 @@ IncrementalGeneratorInitializationContext context { // The framework PurviewTypeLibrary shape is fixed for a given compilation, so it is walked once per // compilation and cached as a value-equatable model instead of being re-walked for every - // [GenerateTypeLibrary] spec in the compilation. + // [GenerateTypeLibrary] spec in the compilation. The framework's public type names travel with it + // because copied documentation renders framework-type cref references as inline code. var frameworkTree = context - .CompilationProvider.Select(static (compilation, _) => BuildFrameworkTree(compilation)) + .CompilationProvider.Select(static (compilation, _) => BuildFrameworkModel(compilation)) .WithTrackingName("GetFrameworkTypeLibraryTree"); return IncrementalPipeline @@ -26,13 +27,22 @@ IncrementalGeneratorInitializationContext context static (ctx, _) => (INamedTypeSymbol)ctx.TargetSymbol, predicate: static (ctx, _) => ctx is ClassDeclarationSyntax ) - .CombineWith(frameworkTree, static (specSymbol, tree, ct) => BuildTarget(specSymbol, tree, ct)) + .CombineWith(frameworkTree, static (specSymbol, framework, ct) => BuildTarget(specSymbol, framework, ct)) .WithTrackingName("GetTypeLibraryTargets"); } + /// + /// The framework information a type-library target needs: the fixed PurviewTypeLibrary shape + /// and the framework assembly's public type names. + /// + sealed record FrameworkTypeLibraryModel( + EquatableArray Tree, + EquatableArray PublicTypeNames + ); + static GeneratorResult BuildTarget( INamedTypeSymbol specSymbol, - EquatableArray frameworkTree, + FrameworkTypeLibraryModel framework, CancellationToken cancellationToken ) { @@ -53,7 +63,7 @@ CancellationToken cancellationToken // The generated type library inherits the full framework PurviewTypeLibrary shape, so every // framework member is present before the user's [TypeRef] members are merged in. The shape was // already walked (once per compilation) by the GetFrameworkTypeLibraryTree stage. - AddFrameworkTree(frameworkTree, root, nodeLookup); + AddFrameworkTree(framework.Tree, root, nodeLookup); foreach (var field in specSymbol.GetMembers().OfType()) ProcessTypeRefField(field, root, nodeLookup, diagnostics, cancellationToken); @@ -71,6 +81,12 @@ CancellationToken cancellationToken if (diagnostics.Any(d => d.IsBlocking)) return GeneratorResult.Create([.. diagnostics]); + // Author documentation is copied into generated files whose namespace and using set differ from + // the source, so framework-type cref references are rendered as inline code before they are + // emitted. Non-framework crefs keep their original form. + RewriteFrameworkCrefs(root, framework.PublicTypeNames); + specDocumentation = FrameworkCrefRewriter.RewriteDocumentation(specDocumentation, framework.PublicTypeNames); + TypeLibraryModel model = new( Specifier: specSymbol.ContainingNamespace.IsGlobalNamespace ? specSymbol.Name @@ -284,6 +300,107 @@ static bool GetGenerateFullNameConstant(AttributeData typeRef) return false; } + /// + /// Builds the framework model consumed by every type-library target: the fixed + /// PurviewTypeLibrary shape plus the public type names the framework owns. + /// + static FrameworkTypeLibraryModel BuildFrameworkModel(Compilation compilation) => + new(BuildFrameworkTree(compilation), BuildFrameworkOwnedPublicTypeNames(compilation)); + + /// + /// Collects the public type names of the framework assembly and of the framework's + /// Generators namespace, whose attribute types the framework emits into the component itself. + /// + static EquatableArray BuildFrameworkOwnedPublicTypeNames(Compilation compilation) + { + List names = []; + + foreach (var reference in compilation.References) + { + if (compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol assembly) + continue; + + if (string.Equals(assembly.Identity.Name, "Purview.SourceGeneratorFramework", StringComparison.Ordinal)) + CollectPublicTypeNames(assembly.GlobalNamespace, names); + } + + var generatedNamespace = compilation + .GlobalNamespace.GetNamespaceMembers() + .FirstOrDefault(static @namespace => @namespace.Name == "Purview") + ?.GetNamespaceMembers() + .FirstOrDefault(static @namespace => @namespace.Name == "SourceGeneratorFramework") + ?.GetNamespaceMembers() + .FirstOrDefault(static @namespace => @namespace.Name == "Generators"); + + if (generatedNamespace is not null) + CollectPublicTypeNames(generatedNamespace, names); + + return new EquatableArray([ + .. names.Distinct(StringComparer.Ordinal).OrderBy(static name => name, StringComparer.Ordinal), + ]); + } + + /// + /// Collects public type names from a namespace. Nested types are skipped: PurviewTypeLibrary + /// mirrors the BCL as nested classes, none of which are framework types. + /// + static void CollectPublicTypeNames(INamespaceSymbol @namespace, List names) + { + foreach (var member in @namespace.GetMembers()) + { + if (member is INamespaceSymbol childNamespace) + { + CollectPublicTypeNames(childNamespace, names); + continue; + } + + if (member is INamedTypeSymbol { DeclaredAccessibility: Accessibility.Public } namedType) + names.Add(namedType.Name); + } + } + + /// + /// Renders framework-type cref references in copied documentation as inline code for every member + /// and enum value in the type-library tree. + /// + static void RewriteFrameworkCrefs(List nodes, EquatableArray frameworkTypeNames) + { + if (frameworkTypeNames.IsEmpty) + return; + + foreach (var node in nodes) + { + for (var index = 0; index < node.Members.Count; index++) + { + var member = node.Members[index]; + node.Members[index] = member with + { + Documentation = FrameworkCrefRewriter.RewriteDocumentation( + member.Documentation, + frameworkTypeNames + ), + }; + } + + foreach (var group in node.EnumGroups) + { + for (var index = 0; index < group.Values.Count; index++) + { + var value = group.Values[index]; + group.Values[index] = value with + { + Documentation = FrameworkCrefRewriter.RewriteDocumentation( + value.Documentation, + frameworkTypeNames + ), + }; + } + } + + RewriteFrameworkCrefs(node.Children, frameworkTypeNames); + } + } + /// /// Walks the framework PurviewTypeLibrary nested namespace classes and their members once, /// producing a value-equatable model that is cached by the GetFrameworkTypeLibraryTree pipeline diff --git a/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs b/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs new file mode 100644 index 0000000..1608f52 --- /dev/null +++ b/src/src/SourceGeneratorFramework.MergeTool/FrameworkTypeInternalizer.cs @@ -0,0 +1,360 @@ +using System.Collections.Immutable; +using Mono.Cecil; + +/// +/// Forces framework-owned types in a merged Roslyn component to non-public visibility. +/// +/// ILRepack's Internalize is best-effort: a framework type that reaches the merged component's +/// public API surface stays public (for example a component type library exposing +/// TypeIdentity/TypeReference members), and the types the framework's own generators +/// emit into the component (the Generators attribute set, generated type libraries, marker +/// attributes) are not part of the merged framework assembly at all, so they are never internalized. +/// Either gap leaks Purview.SourceGeneratorFramework types out of what must be a +/// self-contained analyzer, and any project that loads the component alongside the real framework +/// assembly then reports CS0433 ambiguity for every leaked type. +/// +/// +/// This pass therefore rewrites every framework-owned type to non-public after the merge, and reports +/// what it could not internalize so the build fails instead of shipping a broken package. Roslyn +/// component entry points (generators, analyzers, code fix providers, refactoring providers) are +/// never internalized, including the framework's own bundled components whose entry points live under +/// the framework namespace: Roslyn can only instantiate public component types. +/// +/// +static class FrameworkTypeInternalizer +{ + /// + /// Namespaces the framework owns in a merged component. A type is owned when its namespace equals + /// the prefix or sits beneath it. + /// + public static readonly ImmutableArray DefaultOwnedNamespaces = ["Purview.SourceGeneratorFramework"]; + + /// + /// Marker types the framework's generators emit into components and that must never be public. + /// + public static readonly ImmutableArray DefaultOwnedTypeFullNames = + [ + "Microsoft.CodeAnalysis.EmbeddedAttribute", + ]; + + static readonly ImmutableArray s_roslynComponentAttributeFullNames = + [ + "Microsoft.CodeAnalysis.GeneratorAttribute", + "Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzerAttribute", + "Microsoft.CodeAnalysis.CodeFixes.ExportCodeFixProviderAttribute", + "Microsoft.CodeAnalysis.CodeRefactorings.ExportCodeRefactoringProviderAttribute", + ]; + + static readonly ImmutableArray s_roslynComponentInterfaceFullNames = + [ + "Microsoft.CodeAnalysis.IIncrementalGenerator", + "Microsoft.CodeAnalysis.ISourceGenerator", + "Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer", + "Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider", + "Microsoft.CodeAnalysis.CodeRefactorings.CodeRefactoringProvider", + ]; + + /// + /// Internalizes every framework-owned type in the merged component and returns a report of the + /// changes plus anything that could not be internalized. + /// + /// The merged component to rewrite in place. + /// Assembly resolution paths for the merged component. + /// Optional sink for non-blocking findings, such as component members that expose framework types. + /// Namespace prefixes to internalize; defaults to the framework's own namespaces. + /// Additional type full names to internalize. + public static FrameworkInternalizationReport Apply( + string assemblyPath, + IEnumerable searchDirectories, + Action? warn = null, + ImmutableArray? ownedNamespaces = null, + ImmutableArray? ownedTypeFullNames = null + ) + { + var namespaces = ownedNamespaces ?? DefaultOwnedNamespaces; + var typeFullNames = ownedTypeFullNames ?? DefaultOwnedTypeFullNames; + var hasSymbols = File.Exists(Path.ChangeExtension(assemblyPath, ".pdb")); + + using var resolver = MergeToolRunner.CreateResolver(searchDirectories); + using var assembly = AssemblyDefinition.ReadAssembly( + assemblyPath, + new ReaderParameters + { + AssemblyResolver = resolver, + ReadSymbols = hasSymbols, + InMemory = true, + } + ); + + var allTypes = assembly.MainModule.Types.SelectMany(MergeToolRunner.Flatten).ToList(); + + var internalizedTypeCount = 0; + foreach (var type in allTypes) + { + if (!IsOwned(type, namespaces, typeFullNames) || !IsPubliclyVisible(type)) + continue; + + // Roslyn ignores non-public components, so an internalized entry point would silently + // stop running. Component types are always left alone. + if (IsRoslynComponent(type)) + continue; + + MakeNonPublic(type); + internalizedTypeCount++; + } + + List remainingPublicTypes = []; + foreach (var type in allTypes) + { + if (IsOwned(type, namespaces, typeFullNames) && IsPubliclyVisible(type) && !IsRoslynComponent(type)) + remainingPublicTypes.Add(type.FullName); + } + + List exposingMembers = []; + foreach (var type in allTypes) + { + if (IsOwned(type, namespaces, typeFullNames) || !IsPubliclyVisible(type)) + continue; + + CollectExposingMembers(type, namespaces, typeFullNames, exposingMembers); + } + + if (internalizedTypeCount > 0) + assembly.Write(assemblyPath, new WriterParameters { WriteSymbols = hasSymbols }); + + if (warn is not null) + { + // Group by declaring type: a component whose type library exposes framework types has many + // such members, and one actionable line per type is more useful than one per member. + foreach (var group in exposingMembers.GroupBy(ExposingMemberOwner, StringComparer.Ordinal)) + { + var samples = group.Take(3).Select(ExposingMemberName); + warn( + $"'{group.Key}' is public and exposes Purview.SourceGeneratorFramework types ({group.Count()} member(s), e.g. {string.Join(", ", samples)}). The exposed types were internalized in the merged component, so make the declaring type or its members non-public to keep the component's public surface self-contained." + ); + } + } + + return new(internalizedTypeCount, [.. remainingPublicTypes], [.. exposingMembers]); + } + + static string ExposingMemberOwner(string member) + { + var separator = member.LastIndexOf('.'); + return separator <= 0 ? member : member[..separator]; + } + + static string ExposingMemberName(string member) + { + var separator = member.LastIndexOf('.'); + return separator <= 0 ? member : member[(separator + 1)..]; + } + + static void CollectExposingMembers( + TypeDefinition type, + ImmutableArray ownedNamespaces, + ImmutableArray ownedTypeFullNames, + List exposingMembers + ) + { + if (ReferencesOwnedType(type.BaseType, ownedNamespaces, ownedTypeFullNames)) + exposingMembers.Add(type.FullName); + + foreach (var @interface in type.Interfaces) + { + if (ReferencesOwnedType(@interface.InterfaceType, ownedNamespaces, ownedTypeFullNames)) + exposingMembers.Add($"{type.FullName} : {@interface.InterfaceType.FullName}"); + } + + foreach (var field in type.Fields) + { + if (IsPubliclyVisible(field) && ReferencesOwnedType(field.FieldType, ownedNamespaces, ownedTypeFullNames)) + exposingMembers.Add($"{type.FullName}.{field.Name}"); + } + + foreach (var property in type.Properties) + { + if ( + IsPubliclyVisible(property) + && ( + ReferencesOwnedType(property.PropertyType, ownedNamespaces, ownedTypeFullNames) + || property.Parameters.Any(parameter => + ReferencesOwnedType(parameter.ParameterType, ownedNamespaces, ownedTypeFullNames) + ) + ) + ) + { + exposingMembers.Add($"{type.FullName}.{property.Name}"); + } + } + + foreach (var @event in type.Events) + { + if (IsPubliclyVisible(@event) && ReferencesOwnedType(@event.EventType, ownedNamespaces, ownedTypeFullNames)) + exposingMembers.Add($"{type.FullName}.{@event.Name}"); + } + + foreach (var method in type.Methods) + { + if ( + !IsPubliclyVisible(method) + || ( + !ReferencesOwnedType(method.ReturnType, ownedNamespaces, ownedTypeFullNames) + && !method.Parameters.Any(parameter => + ReferencesOwnedType(parameter.ParameterType, ownedNamespaces, ownedTypeFullNames) + ) + ) + ) + { + continue; + } + + exposingMembers.Add($"{type.FullName}.{method.Name}"); + } + } + + static bool ReferencesOwnedType( + TypeReference? type, + ImmutableArray ownedNamespaces, + ImmutableArray ownedTypeFullNames + ) + { + while (type is not null) + { + switch (type) + { + case RequiredModifierType requiredModifier: + return ReferencesOwnedType(requiredModifier.ModifierType, ownedNamespaces, ownedTypeFullNames) + || ReferencesOwnedType(requiredModifier.ElementType, ownedNamespaces, ownedTypeFullNames); + case OptionalModifierType optionalModifier: + return ReferencesOwnedType(optionalModifier.ModifierType, ownedNamespaces, ownedTypeFullNames) + || ReferencesOwnedType(optionalModifier.ElementType, ownedNamespaces, ownedTypeFullNames); + case GenericInstanceType genericInstance: + return ReferencesOwnedType(genericInstance.ElementType, ownedNamespaces, ownedTypeFullNames) + || genericInstance.GenericArguments.Any(argument => + ReferencesOwnedType(argument, ownedNamespaces, ownedTypeFullNames) + ); + case FunctionPointerType functionPointer: + return ReferencesOwnedType(functionPointer.ReturnType, ownedNamespaces, ownedTypeFullNames) + || functionPointer.Parameters.Any(parameter => + ReferencesOwnedType(parameter.ParameterType, ownedNamespaces, ownedTypeFullNames) + ); + case GenericParameter genericParameter: + return genericParameter.Constraints.Any(constraint => + ReferencesOwnedType(constraint.ConstraintType, ownedNamespaces, ownedTypeFullNames) + ); + case TypeSpecification specification: + type = specification.ElementType; + continue; + default: + break; + } + + if (ownedTypeFullNames.Contains(type.FullName, StringComparer.Ordinal)) + return true; + + return IsOwnedNamespace(type.Namespace, ownedNamespaces); + } + + return false; + } + + static bool IsOwned( + TypeDefinition type, + ImmutableArray ownedNamespaces, + ImmutableArray ownedTypeFullNames + ) => + ownedTypeFullNames.Contains(type.FullName, StringComparer.Ordinal) + || IsOwnedNamespace(type.Namespace, ownedNamespaces); + + static bool IsOwnedNamespace(string? @namespace, ImmutableArray ownedNamespaces) => + @namespace is not null + && ownedNamespaces.Any(prefix => + @namespace.Equals(prefix, StringComparison.Ordinal) + || @namespace.StartsWith(prefix + ".", StringComparison.Ordinal) + ); + + /// + /// Detects Roslyn component entry points so they are never internalized. Roslyn discovers + /// components through their attributes and only instantiates public types. + /// + static bool IsRoslynComponent(TypeDefinition type) + { + foreach (var attribute in type.CustomAttributes) + { + if (s_roslynComponentAttributeFullNames.Contains(attribute.AttributeType.FullName, StringComparer.Ordinal)) + return true; + } + + for (var current = type; current is not null; current = Resolve(current.BaseType)) + { + if (IsRoslynComponentType(current.BaseType)) + return true; + + foreach (var @interface in current.Interfaces) + { + if (IsRoslynComponentType(@interface.InterfaceType)) + return true; + } + } + + return false; + } + + static bool IsRoslynComponentType(TypeReference? type) + { + if (type is null) + return false; + + return s_roslynComponentInterfaceFullNames.Contains(type.FullName, StringComparer.Ordinal) + || s_roslynComponentInterfaceFullNames.Contains( + Resolve(type)?.FullName ?? string.Empty, + StringComparer.Ordinal + ); + } + + static TypeDefinition? Resolve(TypeReference? type) + { + try + { + return type?.Resolve(); + } + catch (AssemblyResolutionException) + { + // A base type that cannot be resolved (for example a Roslyn interface resolved from a + // different host) simply cannot be reported; name matching above already covered it. + return null; + } + } + + static bool IsPubliclyVisible(TypeDefinition type) => type.IsNested ? type.IsNestedPublic : type.IsPublic; + + static bool IsPubliclyVisible(FieldDefinition field) => + field.IsPublic || field.IsFamily || field.IsFamilyOrAssembly; + + static bool IsPubliclyVisible(PropertyDefinition property) => + IsPubliclyVisible(property.GetMethod) || IsPubliclyVisible(property.SetMethod); + + static bool IsPubliclyVisible(EventDefinition @event) => + IsPubliclyVisible(@event.AddMethod) || IsPubliclyVisible(@event.RemoveMethod); + + static bool IsPubliclyVisible(MethodDefinition? method) => + method is not null && (method.IsPublic || method.IsFamily || method.IsFamilyOrAssembly); + + static void MakeNonPublic(TypeDefinition type) => + type.Attributes = type.IsNested + ? (type.Attributes & ~TypeAttributes.VisibilityMask) | TypeAttributes.NestedAssembly + : (type.Attributes & ~TypeAttributes.VisibilityMask) | TypeAttributes.NotPublic; +} + +/// +/// Describes a framework-type internalization pass over a merged component. +/// +/// The number of types rewritten to non-public visibility. +/// Framework-owned types that are still public; a non-empty value must fail the merge. +/// Public members of non-framework types whose signature references a framework-owned type. +sealed record FrameworkInternalizationReport( + int InternalizedTypeCount, + ImmutableArray PublicFrameworkTypesRemaining, + ImmutableArray PublicMembersExposingFrameworkTypes +); diff --git a/src/src/SourceGeneratorFramework.MergeTool/MergeToolRunner.cs b/src/src/SourceGeneratorFramework.MergeTool/MergeToolRunner.cs index 3c0af06..d145ce9 100644 --- a/src/src/SourceGeneratorFramework.MergeTool/MergeToolRunner.cs +++ b/src/src/SourceGeneratorFramework.MergeTool/MergeToolRunner.cs @@ -86,6 +86,23 @@ public static int Run(string[] args, TextWriter error, ILogger? logger = null) RestoreCanonicalIsExternalInit(outputPath, searchDirectories); + // ILRepack's internalize is best-effort and cannot reach framework types the component's + // own generators emit, so force self-containment deterministically and fail the build + // rather than shipping an analyzer that leaks framework types. + var internalization = FrameworkTypeInternalizer.Apply( + outputPath, + searchDirectories, + logger is not null ? logger.Warn : message => error.WriteLine(message) + ); + + if (internalization.PublicFrameworkTypesRemaining.Length > 0) + { + error.WriteLine( + $"The merged component '{outputPath}' still exposes public Purview.SourceGeneratorFramework types: {string.Join(", ", internalization.PublicFrameworkTypesRemaining)}." + ); + return 5; + } + return 0; } finally diff --git a/src/src/SourceGeneratorFramework/Sdk/README.md b/src/src/SourceGeneratorFramework/Sdk/README.md index 3689549..a16a2d2 100644 --- a/src/src/SourceGeneratorFramework/Sdk/README.md +++ b/src/src/SourceGeneratorFramework/Sdk/README.md @@ -989,7 +989,7 @@ The `Purview.SourceGeneratorFramework` package includes the `Purview.SourceGener | `PSGFR36` | Extension classes must be placed in the extended type's namespace under an `Extensions` folder. | | `PSGFR37` | One extension class per receiver type; split classes that extend multiple types. | | `PSGFR38` | Extension classes should carry `[EditorBrowsable(EditorBrowsableState.Never)]`. | -| `PSGFR40` | In Roslyn components (`IsRoslynComponent=true`), qualify XML doc `cref` references to SGF public types with `global::Purview.SourceGeneratorFramework...`. | +| `PSGFR40` | In Roslyn components (`IsRoslynComponent=true`), reference SGF types as inline code (`Type`) instead of a `cref`: copied documentation must not depend on cref resolution. | ## Documentation diff --git a/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/AmbiguousFrameworkCrefAnalyzerTests.cs b/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/UnqualifiedFrameworkCrefAnalyzerTests.cs similarity index 73% rename from src/tests/SourceGeneratorFramework.Analyzers.UnitTests/AmbiguousFrameworkCrefAnalyzerTests.cs rename to src/tests/SourceGeneratorFramework.Analyzers.UnitTests/UnqualifiedFrameworkCrefAnalyzerTests.cs index d29e36e..7a5f994 100644 --- a/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/AmbiguousFrameworkCrefAnalyzerTests.cs +++ b/src/tests/SourceGeneratorFramework.Analyzers.UnitTests/UnqualifiedFrameworkCrefAnalyzerTests.cs @@ -1,11 +1,12 @@ using System.Collections.Immutable; +using System.Globalization; using Purview.SourceGeneratorFramework.Testing; using Purview.SourceGeneratorFramework.Testing.TUnit; namespace Purview.SourceGeneratorFramework.Analyzers; -public sealed class AmbiguousFrameworkCrefAnalyzerTests - : TUnitDiagnosticAnalyzerTestBase +public sealed class UnqualifiedFrameworkCrefAnalyzerTests + : TUnitDiagnosticAnalyzerTestBase { static readonly AnalyzerTestOptions RoslynComponentOptions = new() { @@ -26,7 +27,7 @@ public sealed class AmbiguousFrameworkCrefAnalyzerTests }; [Test] - public async Task BareSgfCrefs_ReportDiagnosticsForMultipleTypes(CancellationToken cancellationToken) + public async Task BareSgfCrefs_ReportInlineCodeGuidanceForMultipleTypes(CancellationToken cancellationToken) { const string source = """ using Purview.SourceGeneratorFramework; @@ -49,17 +50,54 @@ public static class TypeRefs var result = await AnalyzeAsync(source, RoslynComponentOptions, cancellationToken); await Assert.That(result).HasDiagnostics(7); - await Assert.That(result).HasDiagnostic(AmbiguousFrameworkCrefAnalyzer.DiagnosticId); + await Assert.That(result).HasDiagnostic(UnqualifiedFrameworkCrefAnalyzer.DiagnosticId); } [Test] - public async Task QualifiedSgfCrefs_DoNotReportDiagnostic(CancellationToken cancellationToken) + public async Task BareSgfCref_MessageRecommendsInlineCode(CancellationToken cancellationToken) { const string source = """ using Purview.SourceGeneratorFramework; /// - /// Shared building blocks. + /// Shared building blocks. + /// + public static class TypeRefs + { + } + """; + + var result = await AnalyzeAsync(source, RoslynComponentOptions, cancellationToken); + + var diagnostic = result.Diagnostics.Single(); + await Assert.That(diagnostic.GetMessage(CultureInfo.InvariantCulture)).Contains("TypeReference"); + } + + [Test] + public async Task FrameworkQualifiedSgfCref_DoesNotReportDiagnostic(CancellationToken cancellationToken) + { + const string source = """ + /// + /// Shared building blocks. + /// + public static class TypeRefs + { + } + """; + + var result = await AnalyzeAsync(source, RoslynComponentOptions, cancellationToken); + + await Assert.That(result).HasNoDiagnostics(); + } + + [Test] + public async Task MemberCref_DoesNotReportDiagnostic(CancellationToken cancellationToken) + { + const string source = """ + using Purview.SourceGeneratorFramework; + + /// + /// Writes with . /// public static class TypeRefs { diff --git a/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/PreferInlineCodeForFrameworkCrefCodeFixProviderTests.cs b/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/PreferInlineCodeForFrameworkCrefCodeFixProviderTests.cs new file mode 100644 index 0000000..37e0112 --- /dev/null +++ b/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/PreferInlineCodeForFrameworkCrefCodeFixProviderTests.cs @@ -0,0 +1,105 @@ +using System.Collections.Immutable; +using Purview.SourceGeneratorFramework.Analyzers; +using Purview.SourceGeneratorFramework.Testing; +using Purview.SourceGeneratorFramework.Testing.TUnit; + +namespace Purview.SourceGeneratorFramework.CodeFixers; + +public sealed class PreferInlineCodeForFrameworkCrefCodeFixProviderTests + : TUnitCodeFixTestBase +{ + static CodeFixTestOptions Options => + new() + { + EquivalenceKey = PreferInlineCodeForFrameworkCrefCodeFixProvider.EquivalenceKey, + AnalyzerConfigOptions = new Dictionary + { + ["build_property.IsRoslynComponent"] = "true", + }.ToImmutableDictionary(), + AdditionalAssemblyTypes = [typeof(CodeWriter), typeof(TypeReference)], + }; + + [Test] + public async Task BareTypeReferenceCref_BecomesInlineCode(CancellationToken cancellationToken) + { + const string source = """ + using Purview.SourceGeneratorFramework; + + /// + /// Shared building blocks. + /// + public static class TypeRefs + { + } + """; + + var result = await ApplyCodeFixAsync(source, Options, cancellationToken); + + await Assert.That(result).HasDiagnostic(UnqualifiedFrameworkCrefAnalyzer.DiagnosticId); + await Assert.That(result.FixedSource).Contains("TypeReference"); + } + + [Test] + public async Task BareCodeWriterCref_BecomesInlineCode(CancellationToken cancellationToken) + { + const string source = """ + using Purview.SourceGeneratorFramework; + + /// + /// Writes with . + /// + public static class TypeRefs + { + } + """; + + var result = await ApplyCodeFixAsync(source, Options, cancellationToken); + + await Assert.That(result).HasDiagnostic(UnqualifiedFrameworkCrefAnalyzer.DiagnosticId); + await Assert.That(result.FixedSource).Contains("CodeWriter"); + } + + [Test] + public async Task CrefWithContent_KeepsAuthorText(CancellationToken cancellationToken) + { + const string source = """ + using Purview.SourceGeneratorFramework; + + /// + /// Shared the reference value building blocks. + /// + public static class TypeRefs + { + } + """; + + var result = await ApplyCodeFixAsync(source, Options, cancellationToken); + + await Assert.That(result.FixedSource).Contains("the reference value"); + } + + [Test] + public async Task FixAll_ConvertsMultipleCrefs(CancellationToken cancellationToken) + { + const string source = """ + using Purview.SourceGeneratorFramework; + + /// + /// Shared building blocks. + /// Writes with . + /// + public static class TypeRefs + { + } + """; + + var result = await ApplyFixAllAsync(source, Options, cancellationToken); + + await Assert + .That(result.Diagnostics.Select(static d => d.Id).ToArray()) + .Contains(UnqualifiedFrameworkCrefAnalyzer.DiagnosticId); + await Assert.That(result.Diagnostics).Count().IsEqualTo(2); + await Assert.That(result.FixedSources["Test1.cs"]).Contains("TypeReference"); + await Assert.That(result.FixedSources["Test1.cs"]).Contains("CodeWriter"); + } +} diff --git a/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/QualifyFrameworkCrefCodeFixProviderTests.cs b/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/QualifyFrameworkCrefCodeFixProviderTests.cs deleted file mode 100644 index d3fd9a8..0000000 --- a/src/tests/SourceGeneratorFramework.CodeFixers.UnitTests/QualifyFrameworkCrefCodeFixProviderTests.cs +++ /dev/null @@ -1,93 +0,0 @@ -using System.Collections.Immutable; -using Purview.SourceGeneratorFramework.Analyzers; -using Purview.SourceGeneratorFramework.Testing; -using Purview.SourceGeneratorFramework.Testing.TUnit; - -namespace Purview.SourceGeneratorFramework.CodeFixers; - -public sealed class QualifyFrameworkCrefCodeFixProviderTests - : TUnitCodeFixTestBase -{ - static CodeFixTestOptions Options => - new() - { - EquivalenceKey = QualifyFrameworkCrefCodeFixProvider.EquivalenceKey, - AnalyzerConfigOptions = new Dictionary - { - ["build_property.IsRoslynComponent"] = "true", - }.ToImmutableDictionary(), - AdditionalAssemblyTypes = [typeof(CodeWriter), typeof(TypeReference)], - }; - - [Test] - public async Task BareTypeReferenceCref_IsQualified(CancellationToken cancellationToken) - { - const string source = """ - using Purview.SourceGeneratorFramework; - - /// - /// Shared building blocks. - /// - public static class TypeRefs - { - } - """; - - var result = await ApplyCodeFixAsync(source, Options, cancellationToken); - - await Assert.That(result).HasDiagnostic(AmbiguousFrameworkCrefAnalyzer.DiagnosticId); - await Assert - .That(result.FixedSource) - .Contains(""); - } - - [Test] - public async Task BareCodeWriterCref_IsQualified(CancellationToken cancellationToken) - { - const string source = """ - using Purview.SourceGeneratorFramework; - - /// - /// Writes with . - /// - public static class TypeRefs - { - } - """; - - var result = await ApplyCodeFixAsync(source, Options, cancellationToken); - - await Assert.That(result).HasDiagnostic(AmbiguousFrameworkCrefAnalyzer.DiagnosticId); - await Assert - .That(result.FixedSource) - .Contains(""); - } - - [Test] - public async Task FixAll_QualifiesMultipleCrefs(CancellationToken cancellationToken) - { - const string source = """ - using Purview.SourceGeneratorFramework; - - /// - /// Shared building blocks. - /// Writes with . - /// - public static class TypeRefs - { - } - """; - - var result = await ApplyFixAllAsync(source, Options, cancellationToken); - - await Assert - .That(result.Diagnostics.Select(static d => d.Id).ToArray()) - .Contains(AmbiguousFrameworkCrefAnalyzer.DiagnosticId); - await Assert - .That(result.FixedSources["Test1.cs"]) - .Contains(""); - await Assert - .That(result.FixedSources["Test1.cs"]) - .Contains(""); - } -} diff --git a/src/tests/SourceGeneratorFramework.Generators.UnitTests/TypeLibraryGeneratorTests.cs b/src/tests/SourceGeneratorFramework.Generators.UnitTests/TypeLibraryGeneratorTests.cs index 8d08a71..0827073 100644 --- a/src/tests/SourceGeneratorFramework.Generators.UnitTests/TypeLibraryGeneratorTests.cs +++ b/src/tests/SourceGeneratorFramework.Generators.UnitTests/TypeLibraryGeneratorTests.cs @@ -1526,6 +1526,44 @@ await Assert ); } + [Test] + public async Task Generate_CopiedDocumentation_RendersFrameworkTypesAsInlineCode( + CancellationToken cancellationToken + ) + { + const string source = """ + using Purview.SourceGeneratorFramework; + using Purview.SourceGeneratorFramework.Generators; + + namespace Test; + + /// + /// Spec that documents and . + /// + [GenerateTypeLibrary(ClassName = "SampleTypeLibrary", Namespace = "Test")] + static partial class TypeLibraryModel + { + /// + /// Identity for the writer. + /// + [TypeRef("Purview.Telemetry")] + static readonly TypeIdentity ActivitySourceGenerationAttribute = default; + } + """; + + var result = await GenerateAsync(source, cancellationToken: cancellationToken); + + var generated = await GetGeneratedStringAsync(result, "Test.TypeLibraryModel.g.cs", cancellationToken); + + await Assert.That(generated).IsNotNull(); + // Framework types become inline code so the copied documentation cannot fail to resolve. + await Assert.That(generated).Contains("TypeReference"); + await Assert.That(generated).Contains("the writer"); + // Non-framework crefs keep their original form (Roslyn expands them when docs are captured). + await Assert.That(generated).Contains("cref=\"T:System.String\""); + await Assert.That(generated).DoesNotContain("System.String"); + } + static async Task GetGeneratedStringAsync( DriverRunResult result, string fileName, diff --git a/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/FrameworkTypeInternalizerTests.cs b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/FrameworkTypeInternalizerTests.cs new file mode 100644 index 0000000..0994e54 --- /dev/null +++ b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/FrameworkTypeInternalizerTests.cs @@ -0,0 +1,243 @@ +using System.Collections.Immutable; +using Mono.Cecil; + +namespace Purview.SourceGeneratorFramework.MergeTool; + +public sealed class FrameworkTypeInternalizerTests +{ + /// + /// Mirrors the real framework shape: framework types live in the framework namespace, and the + /// framework ships the Roslyn component interface used to identify component entry points. + /// + const string FixtureFrameworkSource = """ + namespace System.Runtime.CompilerServices + { + public static class IsExternalInit; + } + + namespace Microsoft.CodeAnalysis + { + public interface IIncrementalGenerator; + } + + namespace Purview.SourceGeneratorFramework + { + public sealed record TypeReference + { + public string Name { get; } + + public TypeReference(string name) => Name = name; + } + + public sealed class CodeWriter + { + public void Write(string value) { } + } + + public readonly record struct TypeIdentity(string Name, string Namespace); + } + """; + + /// + /// Mirrors a component that leaks framework types publicly: a generated type library declared in + /// the framework namespace (so it is never part of the merged framework assembly and ILRepack + /// cannot internalize it), a component entry point in the same namespace that must stay public, + /// and a public component member exposing a framework type. + /// + const string LeakingComponentSource = """ + namespace Purview.SourceGeneratorFramework + { + using Microsoft.CodeAnalysis; + + public static class TypeLibrary + { + public static readonly TypeIdentity TypeReference = + new("TypeReference", "Purview.SourceGeneratorFramework"); + } + + public sealed class ComponentEntryPoint : IIncrementalGenerator + { + } + } + + namespace Fixture.Component + { + public sealed class PublicSurface + { + public Purview.SourceGeneratorFramework.TypeReference Reference { get; } = new("Exposed"); + } + } + """; + + /// + /// A component that references the real framework assembly: it publishes a framework-typed + /// member and declares a Roslyn component entry point inside the framework namespace. + /// + const string RealFrameworkComponentSource = """ + namespace Microsoft.CodeAnalysis + { + public interface IIncrementalGenerator; + } + + namespace Purview.SourceGeneratorFramework + { + public static class TypeLibrary + { + public static global::Purview.SourceGeneratorFramework.TypeIdentity Identity => default; + } + + public sealed class ComponentEntryPoint : global::Microsoft.CodeAnalysis.IIncrementalGenerator + { + } + } + """; + + [Test] + public async Task Apply_GivenOwnedPublicTypes_InternalizesThemAndKeepsComponentsPublic( + CancellationToken cancellationToken + ) + { + // Arrange + cancellationToken.ThrowIfCancellationRequested(); + using TestWorkspace workspace = new(); + var frameworkPath = workspace.Compile("Fixture.Framework", FixtureFrameworkSource); + var componentPath = workspace.Compile("Fixture.Component", LeakingComponentSource, frameworkPath); + List warnings = []; + + // Act + var report = FrameworkTypeInternalizer.Apply( + componentPath, + [workspace.GetPath("Fixture.Component")], + warnings.Add, + ownedNamespaces: ["Purview.SourceGeneratorFramework"] + ); + + // Assert + await Assert.That(report.PublicFrameworkTypesRemaining).IsEmpty(); + await Assert.That(report.InternalizedTypeCount).IsGreaterThan(0); + + using var component = AssemblyDefinition.ReadAssembly(componentPath); + var typeLibrary = component.MainModule.GetType("Purview.SourceGeneratorFramework.TypeLibrary"); + await Assert.That(typeLibrary).IsNotNull(); + await Assert.That(typeLibrary!.IsNotPublic).IsTrue(); + + var entryPoint = component.MainModule.GetType("Purview.SourceGeneratorFramework.ComponentEntryPoint"); + await Assert.That(entryPoint).IsNotNull(); + await Assert.That(entryPoint!.IsPublic).IsTrue(); + + await Assert + .That(warnings) + .Contains(static warning => + warning.Contains("Fixture.Component.PublicSurface", StringComparison.Ordinal) + && warning.Contains("Reference", StringComparison.Ordinal) + ); + } + + [Test] + public async Task Run_GivenComponentLeakingFrameworkTypes_ProducesSelfContainedMergedAssembly( + CancellationToken cancellationToken + ) + { + // Arrange + cancellationToken.ThrowIfCancellationRequested(); + using TestWorkspace workspace = new(); + var frameworkPath = workspace.Compile("Fixture.Framework", FixtureFrameworkSource); + var componentPath = workspace.Compile("Fixture.Component", LeakingComponentSource, frameworkPath); + var outputPath = workspace.GetPath("merged", "Fixture.Component.dll"); + TestLogger logger = new(); + + // Act + var exitCode = MergeToolRunner.Run( + [componentPath, frameworkPath, outputPath, TestWorkspace.NetStandardReferenceDirectory], + TextWriter.Null, + logger + ); + + // Assert + await Assert.That(exitCode).IsEqualTo(0); + await Assert.That(File.Exists(outputPath)).IsTrue(); + + using var merged = AssemblyDefinition.ReadAssembly(outputPath); + var publicFrameworkTypes = merged + .MainModule.Types.SelectMany(MergeToolRunner.Flatten) + .Where(static type => + type.Namespace is not null + && ( + type.Namespace.Equals("Purview.SourceGeneratorFramework", StringComparison.Ordinal) + || type.Namespace.StartsWith("Purview.SourceGeneratorFramework.", StringComparison.Ordinal) + ) + && (type.IsNested ? type.IsNestedPublic : type.IsPublic) + ) + .Select(static type => type.FullName) + .ToImmutableArray(); + + // The only public framework-namespace type left must be the Roslyn component entry point: + // Roslyn cannot instantiate non-public components, so entry points are never internalized. + await Assert.That(publicFrameworkTypes).HasSingleItem(); + await Assert.That(publicFrameworkTypes[0]).IsEqualTo("Purview.SourceGeneratorFramework.ComponentEntryPoint"); + await Assert + .That(logger.Warnings) + .Contains(static warning => + warning.Contains("Fixture.Component.PublicSurface", StringComparison.Ordinal) + && warning.Contains("Reference", StringComparison.Ordinal) + ); + + var entryPoint = merged.MainModule.GetType("Purview.SourceGeneratorFramework.ComponentEntryPoint"); + await Assert.That(entryPoint).IsNotNull(); + await Assert.That(entryPoint!.IsPublic).IsTrue(); + } + + /// + /// Merges the real framework assembly rather than a fixture stand-in, so the self-contained + /// contract is asserted against the shipped metadata (and against the types the framework's own + /// generators emit into the component). + /// + [Test] + public async Task Run_GivenRealFrameworkAssembly_ProducesSelfContainedMergedAssembly( + CancellationToken cancellationToken + ) + { + // Arrange + cancellationToken.ThrowIfCancellationRequested(); + using TestWorkspace workspace = new(); + var frameworkPath = typeof(Purview.SourceGeneratorFramework.TypeIdentity).Assembly.Location; + var componentPath = workspace.Compile("Fixture.RealComponent", RealFrameworkComponentSource, frameworkPath); + var outputPath = workspace.GetPath("merged", "Fixture.RealComponent.dll"); + TestLogger logger = new(); + + // Act + var exitCode = MergeToolRunner.Run( + [ + componentPath, + frameworkPath, + outputPath, + TestWorkspace.NetStandardReferenceDirectory, + AppContext.BaseDirectory, + ], + TextWriter.Null, + logger + ); + + // Assert + await Assert.That(exitCode).IsEqualTo(0); + + using var merged = AssemblyDefinition.ReadAssembly(outputPath); + var publicFrameworkTypes = merged + .MainModule.Types.SelectMany(MergeToolRunner.Flatten) + .Where(static type => IsFrameworkOwnedNamespace(type.Namespace)) + .Where(static type => type.IsNested ? type.IsNestedPublic : type.IsPublic) + .Select(static type => type.FullName) + .ToImmutableArray(); + + // Only the component's own Roslyn entry point stays public in the merged artifact. + await Assert.That(publicFrameworkTypes).HasSingleItem(); + await Assert.That(publicFrameworkTypes[0]).IsEqualTo("Purview.SourceGeneratorFramework.ComponentEntryPoint"); + } + + static bool IsFrameworkOwnedNamespace(string? @namespace) => + @namespace is not null + && ( + @namespace.Equals("Purview.SourceGeneratorFramework", StringComparison.Ordinal) + || @namespace.StartsWith("Purview.SourceGeneratorFramework.", StringComparison.Ordinal) + ); +} diff --git a/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/MergeToolRunnerTests.cs b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/MergeToolRunnerTests.cs index 07c2325..86dab44 100644 --- a/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/MergeToolRunnerTests.cs +++ b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/MergeToolRunnerTests.cs @@ -1,6 +1,5 @@ using System.Reflection; using System.Runtime.Loader; -using ILRepacking; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Mono.Cecil; @@ -162,7 +161,7 @@ await Assert var mergedTypeReference = merged.MainModule.GetType("Fixture.Framework.TypeReference"); await Assert.That(mergedTypeReference).IsNotNull(); - await Assert.That(mergedTypeReference!.IsNotPublic).IsTrue(); + await Assert.That(mergedTypeReference.IsNotPublic).IsTrue(); var consumer = merged.MainModule.GetType("Fixture.Component.Consumer"); var create = consumer.Methods.Single(static method => method.Name == "Create"); @@ -208,114 +207,4 @@ static async Task AssertMergedAssemblyExecutesAsync(string outputPath) loadContext.Unload(); } } - - sealed class TestLogger : ILogger - { - public bool ShouldLogVerbose { get; set; } - - public List Warnings { get; } = []; - - public void Error(string msg) { } - - public void Info(string msg) { } - - public void Verbose(string msg) { } - - public void Warn(string msg) => Warnings.Add(msg); - } - - sealed class TestWorkspace : IDisposable - { - readonly string _directory = Path.Combine( - Path.GetTempPath(), - $"Purview.SourceGeneratorFramework.MergeTool.Tests.{Guid.NewGuid():N}" - ); - - public static string NetStandardReferenceDirectory { get; } = GetNetStandardReferenceDirectory(); - - public TestWorkspace() - { - Directory.CreateDirectory(_directory); - } - - public string Compile(string assemblyName, string source, params string[] additionalReferences) - { - var outputPath = GetPath(assemblyName, $"{assemblyName}.dll"); - Directory.CreateDirectory(Path.GetDirectoryName(outputPath)!); - - var references = Directory - .EnumerateFiles(NetStandardReferenceDirectory, "*.dll") - .Select(static path => (MetadataReference)MetadataReference.CreateFromFile(path)) - .ToList(); - references.AddRange(additionalReferences.Select(static path => MetadataReference.CreateFromFile(path))); - - var compilation = CreateCompilation(assemblyName, source, references); - Emit(compilation, outputPath); - return outputPath; - } - - static CSharpCompilation CreateCompilation( - string assemblyName, - string source, - IEnumerable references - ) => - CSharpCompilation.Create( - assemblyName, - [ - CSharpSyntaxTree.ParseText( - source, - CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.Latest) - ), - ], - references, - new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) - ); - - static void Emit(CSharpCompilation compilation, string outputPath) - { - using var output = File.Create(outputPath); - var result = compilation.Emit(output); - if (!result.Success) - { - throw new InvalidOperationException(string.Join(Environment.NewLine, result.Diagnostics)); - } - } - - public string GetPath(params string[] parts) => parts.Aggregate(_directory, Path.Combine); - - public void Dispose() - { - if (Directory.Exists(_directory)) - { - Directory.Delete(_directory, recursive: true); - } - } - - static string GetNetStandardReferenceDirectory() - { - var candidateRoots = new[] - { - Environment.GetEnvironmentVariable("NUGET_PACKAGES"), - Environment.GetEnvironmentVariable("RestorePackagesPath"), - Environment.GetEnvironmentVariable("NuGetPackageRoot"), - Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".nuget", "packages"), - } - .Where(static path => !string.IsNullOrWhiteSpace(path)) - .Select(static path => path!) - .Select(static path => path.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)) - .Distinct(StringComparer.OrdinalIgnoreCase); - - foreach (var packageRoot in candidateRoots) - { - var path = Path.Combine(packageRoot, "netstandard.library", "2.0.3", "build", "netstandard2.0", "ref"); - - if (Directory.Exists(path)) - return path; - } - - throw new DirectoryNotFoundException( - $"The .NET Standard 2.0 reference directory was not found under any known package roots: {string.Join(", ", candidateRoots)}" - ); - } - } } diff --git a/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/SourceGeneratorFramework.MergeTool.UnitTests.csproj b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/SourceGeneratorFramework.MergeTool.UnitTests.csproj index 92eea72..b142a28 100644 --- a/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/SourceGeneratorFramework.MergeTool.UnitTests.csproj +++ b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/SourceGeneratorFramework.MergeTool.UnitTests.csproj @@ -7,4 +7,9 @@ + + + + + diff --git a/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/TestWorkspace.cs b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/TestWorkspace.cs new file mode 100644 index 0000000..b37b6d8 --- /dev/null +++ b/src/tests/SourceGeneratorFramework.MergeTool.UnitTests/TestWorkspace.cs @@ -0,0 +1,122 @@ +using ILRepacking; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace Purview.SourceGeneratorFramework.MergeTool; + +/// +/// Captures merge tool log output for assertions. +/// +sealed class TestLogger : ILogger +{ + public bool ShouldLogVerbose { get; set; } + + public List Warnings { get; } = []; + + public void Error(string msg) { } + + public void Info(string msg) { } + + public void Verbose(string msg) { } + + public void Warn(string msg) => Warnings.Add(msg); +} + +/// +/// Compiles fixture assemblies into a temporary directory so merge tests can run the tool against +/// real component/framework inputs. +/// +sealed class TestWorkspace : IDisposable +{ + readonly string _directory = Path.Combine( + Path.GetTempPath(), + $"Purview.SourceGeneratorFramework.MergeTool.Tests.{Guid.NewGuid():N}" + ); + + public static string NetStandardReferenceDirectory { get; } = GetNetStandardReferenceDirectory(); + + public TestWorkspace() + { + Directory.CreateDirectory(_directory); + } + + public string Compile(string assemblyName, string source, params string[] additionalReferences) + { + var outputPath = GetPath(assemblyName, $"{assemblyName}.dll"); + Directory.CreateDirectory(Path.GetDirectoryName(outputPath)!); + + var references = Directory + .EnumerateFiles(NetStandardReferenceDirectory, "*.dll") + .Select(static path => (MetadataReference)MetadataReference.CreateFromFile(path)) + .ToList(); + references.AddRange(additionalReferences.Select(static path => MetadataReference.CreateFromFile(path))); + + var compilation = CreateCompilation(assemblyName, source, references); + Emit(compilation, outputPath); + return outputPath; + } + + static CSharpCompilation CreateCompilation( + string assemblyName, + string source, + IEnumerable references + ) => + CSharpCompilation.Create( + assemblyName, + [ + CSharpSyntaxTree.ParseText( + source, + CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.Latest) + ), + ], + references, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + ); + + static void Emit(CSharpCompilation compilation, string outputPath) + { + using var output = File.Create(outputPath); + var result = compilation.Emit(output); + if (!result.Success) + { + throw new InvalidOperationException(string.Join(Environment.NewLine, result.Diagnostics)); + } + } + + public string GetPath(params string[] parts) => parts.Aggregate(_directory, Path.Combine); + + public void Dispose() + { + if (Directory.Exists(_directory)) + { + Directory.Delete(_directory, recursive: true); + } + } + + static string GetNetStandardReferenceDirectory() + { + var candidateRoots = new[] + { + Environment.GetEnvironmentVariable("NUGET_PACKAGES"), + Environment.GetEnvironmentVariable("RestorePackagesPath"), + Environment.GetEnvironmentVariable("NuGetPackageRoot"), + Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".nuget", "packages"), + } + .Where(static path => !string.IsNullOrWhiteSpace(path)) + .Select(static path => path!) + .Select(static path => path.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)) + .Distinct(StringComparer.OrdinalIgnoreCase); + + foreach (var packageRoot in candidateRoots) + { + var path = Path.Combine(packageRoot, "netstandard.library", "2.0.3", "build", "netstandard2.0", "ref"); + + if (Directory.Exists(path)) + return path; + } + + throw new DirectoryNotFoundException( + $"The .NET Standard 2.0 reference directory was not found under any known package roots: {string.Join(", ", candidateRoots)}" + ); + } +}