From df07813c713da9d534b8df0999202820f2094941 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Thu, 12 Nov 2026 12:21:29 +0000 Subject: [PATCH] feat: added supressor for static members on IUnionMembers --- .config/dotnet-tools.json | 2 +- .editorconfig | 2 +- AGENTS.md | 12 +- CHANGELOG.md | 5 + README.md | 13 +- docs/wiki/Diagnostics.md | 10 +- docs/wiki/Getting-Started.md | 4 +- docs/wiki/Interop.md | 11 + docs/wiki/Union-Design.md | 8 +- package.json | 2 +- skills/purview-unions-diagnostics/SKILL.md | 17 +- skills/purview-unions-modelling/SKILL.md | 8 +- skills/purview-unions-quickstart/SKILL.md | 2 +- .../UnionMembersNestingSuppressor.cs | 110 +++++++-- src/src/SourceGenerator/README.md | 4 +- src/src/Unions/Sdk/README.md | 4 +- .../UnionMembersNestingSuppressorTests.cs | 211 +++++++++++++++++- 17 files changed, 367 insertions(+), 58 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index bc9bc27..1978504 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -17,7 +17,7 @@ "rollForward": false }, "purview.build": { - "version": "0.9.0", + "version": "0.9.1", "commands": [ "purview-build" ], diff --git a/.editorconfig b/.editorconfig index a9b39cb..229c475 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,4 +1,4 @@ -# Purview.BuildSdk 2.0.0 - managed file, do not edit +# Purview.BuildSdk 2.0.1 - managed file, do not edit # # This file is written by Purview.BuildSdk and is replaced whenever the SDK version changes. The version # on the first line is how the SDK recognises a file it owns: a .editorconfig without that marker is diff --git a/AGENTS.md b/AGENTS.md index b5901b4..d170695 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,17 +84,19 @@ The compiler's contract is not ours to redefine. Every generated union must sati ## Diagnostics rules - Prefix `UNI`. Every reported rule is in `AnalyzerReleases.Unshipped.md` (RS2008). Suppression - descriptors are not release-tracked. `UNI0000` and `UNI9001` are suppression descriptors, not rules: they - suppress `CA1034` and `CA1515` on a union's public nested `IUnionMembers` provider. + descriptors are not release-tracked. `UNI0000`, `UNI9001` and `UNI9002` are suppression descriptors, not + rules: `UNI0000` suppresses `CA1034` and `UNI9001` suppresses `CA1515` on a union's public nested + `IUnionMembers` provider, and `UNI9002` suppresses `CA1000` on a generic union's provider's static `Create` + members. - The blocking policy lives beside the descriptors (`DiagnosticLibrary.IsBlocking`); there is one policy for both hosts. - A rule is reported once. The analyzer reports per-target findings; the generator applies the same findings to `GeneratorResult.ShouldProcess` and reports only `UNI9000`. - Do not suppress warnings with `NoWarn`, `#pragma warning disable` or `SuppressMessage` in product code. The only suppressions kept repo-wide are `Purview.BuildSdk`'s `PDS0006` for `CA1815` on a union - declaration, this package's `UNI0000` `DiagnosticSuppressor` for `CA1034` and `UNI9001` for `CA1515` on a - union's `IUnionMembers` provider, and the deliberate `RSEXPERIMENTAL006` at the `ITypeSymbol.IsUnion` call - in a test. + declaration, this package's `UNI0000` `DiagnosticSuppressor` for `CA1034`, `UNI9001` for `CA1515` and + `UNI9002` for `CA1000` on a union's `IUnionMembers` provider, and the deliberate `RSEXPERIMENTAL006` at the + `ITypeSymbol.IsUnion` call in a test. ## Packaging rules diff --git a/CHANGELOG.md b/CHANGELOG.md index 55e14ce..b80deb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A `UNI0000` `DiagnosticSuppressor` that suppresses `CA1034` and a `UNI9001` suppressor that suppresses `CA1515` on the public nested `IUnionMembers` provider the C# union contract requires. Both ship with the package's analyzer, so a surface-C union needs no suppression attribute. +- A `UNI9002` `DiagnosticSuppressor` that suppresses `CA1000` (do not declare static members on generic + types) on a generic union's `IUnionMembers` provider static `Create` members. The members are the + contract's creation members and the provider is necessarily generic when the union is, so `CA1000` cannot + be satisfied without removing the union semantics. This lets the `purview-dev/results` + `ProvidedOption.IUnionMembers` provider drop its `SuppressMessage` workaround. - A `UNI0005` code fix that makes the case type and every containing type public. ### Fixed diff --git a/README.md b/README.md index a9babfb..2b08e05 100644 --- a/README.md +++ b/README.md @@ -92,9 +92,11 @@ provider owns its one-line `Create` bodies, because a source generator cannot ad declared. The provider must be **public**, so a strict analyzer set reports `CA1034` (nested types should not be -visible) and `CA1515` (public types should be internal). The package's `UNI0000` and `UNI9001` suppression -descriptors handle both for an `IUnionMembers` provider nested in a union; no attribute is needed. The nested -shape is mandated by the language, not a design choice. +visible) and `CA1515` (public types should be internal). A generic union's provider is itself generic, so its +static `Create` members are reported as `CA1000` (do not declare static members on generic types). The +package's `UNI0000`, `UNI9001` and `UNI9002` suppression descriptors handle all three for an `IUnionMembers` +provider nested in a union; no attribute is needed. The nested shape is mandated by the language, not a +design choice. ### D — hand-written `Union` @@ -152,8 +154,9 @@ takes `int?`, and `TryGetValue(out int)` returns false for null — a null value The generated union is `CA1815`-clean: `Purview.BuildSdk`'s `PDS0006` suppresses the warning for a union declaration, because a union is matched by its active case rather than compared by value. Surface C is -`CA1034`- and `CA1515`-clean: the package's `UNI0000` and `UNI9001` suppression descriptors cover the -language-mandated public nested `IUnionMembers` provider. No `#pragma` is needed. +`CA1034`-, `CA1515`- and `CA1000`-clean: the package's `UNI0000`, `UNI9001` and `UNI9002` suppression +descriptors cover the language-mandated public nested `IUnionMembers` provider and its static `Create` +members. No `#pragma` is needed. ## Diagnostics diff --git a/docs/wiki/Diagnostics.md b/docs/wiki/Diagnostics.md index ce3e002..e04bc33 100644 --- a/docs/wiki/Diagnostics.md +++ b/docs/wiki/Diagnostics.md @@ -26,10 +26,12 @@ whether it can emit and reports only `UNI9000`. A rule is reported once. Every error blocks its union: the generated constructors, storage and access members are one coherent shape, so there is no partial output. `UNI0015` is the only warning and does not block. -`UNI0000` and `UNI9001` are **suppression descriptors**, not reported rules. `UNI0000` suppresses `CA1034` -(nested types should not be visible) and `UNI9001` suppresses `CA1515` (public types should be internal) on a -union's public nested `IUnionMembers` provider, which the C# union contract requires. Add either id to -`NoWarn` to turn that suppression off. +`UNI0000`, `UNI9001` and `UNI9002` are **suppression descriptors**, not reported rules. `UNI0000` suppresses +`CA1034` (nested types should not be visible) and `UNI9001` suppresses `CA1515` (public types should be +internal) on a union's public nested `IUnionMembers` provider, which the C# union contract requires. +`UNI9002` suppresses `CA1000` (do not declare static members on generic types) on that provider's static +`Create` members, which are the contract's creation members and are necessarily generic when the union is. +Add any of these ids to `NoWarn` to turn that suppression off; a consumer should not need to. ## Details diff --git a/docs/wiki/Getting-Started.md b/docs/wiki/Getting-Started.md index c26a6dc..5ba745f 100644 --- a/docs/wiki/Getting-Started.md +++ b/docs/wiki/Getting-Started.md @@ -67,8 +67,8 @@ Option some = new Some(42); ``` A nested `IUnionMembers` provider is the alternative when another generator must see the case list; the -provider is public, so the package's `UNI0000` and `UNI9001` suppressors handle the resulting `CA1034` and -`CA1515`. See [Union Design](Union-Design.md). +provider is public, so the package's `UNI0000`, `UNI9001` and `UNI9002` suppressors handle the resulting +`CA1034`, `CA1515` and `CA1000`. See [Union Design](Union-Design.md). ## No generator diff --git a/docs/wiki/Interop.md b/docs/wiki/Interop.md index 6358fa6..40ffbbf 100644 --- a/docs/wiki/Interop.md +++ b/docs/wiki/Interop.md @@ -29,10 +29,21 @@ follow-up in `purview-dev/results`. Do not hand-write the constructors to satisfy Results: the union generator would then treat the union as hand-written and the declared and existing lists would disagree (`UNI0012`/`UNI0014`). +A hand-written `[Union]` union whose cases live on a nested `IUnionMembers` provider — the shape the Results +tests use — needs no `SuppressMessage` attribute for `CA1000`. The package's `UNI9002` suppression descriptor +answers the rule on the provider's static `Create` members, which are generic whenever the union is. See +[Diagnostics](Diagnostics.md). + ### Follow-up > `purview-dev/results`: add the two-generator integration test once `Purview.Unions` is on a CI-safe package > feed. +> +> `purview-dev/results`: once a `Purview.Unions` version carrying `UNI9002` is on the feed, bump +> `PurviewUnionsVersion` in `Directory.Packages.props` and delete the `CA1000` `SuppressMessage` from +> `ProvidedOption.IUnionMembers` in `src/tests/SourceGenerator.UnitTests/GeneratedTestUnions.cs`. Verify by +> raising `CA1000` to `warning` in a temporary project-local `.editorconfig` under that test project: the build +> must report no `CA1000` for the provider's static `Create` members. ## Purview.Telemetry diff --git a/docs/wiki/Union-Design.md b/docs/wiki/Union-Design.md index facdbdc..55ff7c5 100644 --- a/docs/wiki/Union-Design.md +++ b/docs/wiki/Union-Design.md @@ -100,10 +100,10 @@ Design points: - **No equality, `ToString`, `Deconstruct`, `Match` or `Switch`** for generated unions in v1. A union is matched by its active case; `CA1815` is expected and `PDS0006` answers it. A `record` union gets the compiler's record equality over the tag and fields. -- **Surface C is `CA1034`- and `CA1515`-clean.** The provider must be a public nested interface, so the - package's `UNI0000` and `UNI9001` suppression descriptors suppress the nesting and visibility warnings for - an `IUnionMembers` provider inside a union; a differently named nested type, or one outside a union, keeps - them. +- **Surface C is `CA1034`-, `CA1515`- and `CA1000`-clean.** The provider must be a public nested interface, + so the package's `UNI0000`, `UNI9001` and `UNI9002` suppression descriptors suppress the nesting, visibility + and static-member-on-generic-type warnings for an `IUnionMembers` provider inside a union (for `CA1000`, on + its static `Create` members); a differently named nested type, or one outside a union, keeps them. - **Accessibility is never widened.** The partial declaration repeats the union's accessibility, and every case type must be at least as accessible as the union because a public creation member cannot expose a less accessible parameter type. An internal union can therefore use internal case types; a public union needs diff --git a/package.json b/package.json index ed028bd..eb334d9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "unions", - "version": "1.0.0-prerelease.2", + "version": "1.0.0-prerelease.3", "license": "MIT", "homepage": "https://purview.dev/projects/unions/", "bugs": { diff --git a/skills/purview-unions-diagnostics/SKILL.md b/skills/purview-unions-diagnostics/SKILL.md index 936b867..395b0ef 100644 --- a/skills/purview-unions-diagnostics/SKILL.md +++ b/skills/purview-unions-diagnostics/SKILL.md @@ -34,10 +34,12 @@ never reported twice. Every error blocks its union: the generated constructors, storage and access members are one coherent shape, so there is no partial output to emit. `UNI0015` is the only warning and does not block. -`UNI0000` and `UNI9001` are **suppression descriptors**, not reported rules. `UNI0000` suppresses `CA1034` -(nested types should not be visible) and `UNI9001` suppresses `CA1515` (public types should be internal) on a -union's public nested `IUnionMembers` provider, which the C# union contract requires. Add either id to -`NoWarn` to turn that suppression off. +`UNI0000`, `UNI9001` and `UNI9002` are **suppression descriptors**, not reported rules. `UNI0000` suppresses +`CA1034` (nested types should not be visible) and `UNI9001` suppresses `CA1515` (public types should be +internal) on a union's public nested `IUnionMembers` provider, which the C# union contract requires. +`UNI9002` suppresses `CA1000` (do not declare static members on generic types) on the provider's static +`Create` members: they are the contract's creation members, and a generic union's provider is necessarily +generic. Add any of these ids to `NoWarn` to turn that suppression off; you should not need to. ## `UNI0001` — not partial @@ -104,7 +106,12 @@ The generator catches its own failures so one malformed union cannot discard eve There is deliberately no `NoWarn`/`#pragma` in this stack. The suppressions are `DiagnosticSuppressors`, not reported rules: `Purview.BuildSdk`'s `PDS0006` for `CA1815` on a union declaration, and this package's -`UNI0000` for `CA1034` and `UNI9001` for `CA1515` on a union's nested `IUnionMembers` provider. +`UNI0000` for `CA1034`, `UNI9001` for `CA1515` and `UNI9002` for `CA1000` on a union's nested +`IUnionMembers` provider. + +`UNI9002` covers a generic union's provider static `Create` members, which `CA1000` reports on. A +hand-written `[Union]` generic union's provider needs no `SuppressMessage` attribute for them; the +suppression descriptor already answers the finding. ## Checklist diff --git a/skills/purview-unions-modelling/SKILL.md b/skills/purview-unions-modelling/SKILL.md index a0df7ee..18d6c3e 100644 --- a/skills/purview-unions-modelling/SKILL.md +++ b/skills/purview-unions-modelling/SKILL.md @@ -76,9 +76,11 @@ one-line `Create` bodies and delegates to the generated `CreateCaseN` methods. U want the case list visible to other generators. Because `IUnionMembers` must be **public**, a strict analyzer set reports `CA1034` and `CA1515` (nested types -should not be visible; public types should be internal). The package's `UNI0000`/`UNI9001` suppression -descriptors handle both for an `IUnionMembers` provider nested in a union; no attribute is needed. The nested -shape is mandated by the language, not a design choice. +should not be visible; public types should be internal). A generic union's provider is itself generic, so its +static `Create` members are reported as `CA1000` (do not declare static members on generic types). The +package's `UNI0000`/`UNI9001`/`UNI9002` suppression descriptors handle all three for an `IUnionMembers` +provider nested in a union; no attribute is needed. The nested shape is mandated by the language, not a +design choice. ### D — hand-written `Purview.Unions.Union` diff --git a/skills/purview-unions-quickstart/SKILL.md b/skills/purview-unions-quickstart/SKILL.md index 0e19167..5148910 100644 --- a/skills/purview-unions-quickstart/SKILL.md +++ b/skills/purview-unions-quickstart/SKILL.md @@ -111,7 +111,7 @@ for inspecting the declared source in isolation; it is not a way to make a faili | --- | --- | --- | | A | `[GenerateUnion(typeof(int), typeof(string))]` | Every case can be named in an attribute argument | | B | partial static factory signatures | A case names a type parameter | -| C | a nested `IUnionMembers` provider | A generic union, or another generator must see the cases (the provider is public, but `UNI0000`/`UNI9001` suppress the resulting `CA1034`/`CA1515`) | +| C | a nested `IUnionMembers` provider | A generic union, or another generator must see the cases (the provider is public, but `UNI0000`/`UNI9001`/`UNI9002` suppress the resulting `CA1034`/`CA1515`/`CA1000`) | | D | `Union` | Exactly two cases, or no generator | ## Checklist diff --git a/src/src/SourceGenerator/Analyzers/UnionMembersNestingSuppressor.cs b/src/src/SourceGenerator/Analyzers/UnionMembersNestingSuppressor.cs index 9f0c429..703aeea 100644 --- a/src/src/SourceGenerator/Analyzers/UnionMembersNestingSuppressor.cs +++ b/src/src/SourceGenerator/Analyzers/UnionMembersNestingSuppressor.cs @@ -7,8 +7,9 @@ namespace Purview.Unions.SourceGenerator.Analyzers; /// -/// Suppresses CA1034 (nested types should not be visible) and CA1515 (public types should be -/// internal) on the IUnionMembers provider the C# union contract requires a union to nest. +/// Suppresses CA1034 (nested types should not be visible), CA1515 (public types should be +/// internal) and CA1000 (do not declare static members on generic types) on the IUnionMembers +/// provider the C# union contract requires a union to nest. /// /// /// @@ -19,14 +20,24 @@ namespace Purview.Unions.SourceGenerator.Analyzers; /// making the interface internal removes the union semantics. /// /// -/// The suppression is deliberately narrow. It applies only when the reported type is an interface named -/// IUnionMembers whose immediate containing type is a union (a type carrying -/// [GenerateUnion] or [Union]). A type of that name nested in an ordinary type, and any other -/// nested type in a union, keep the warnings. +/// The provider's static Create methods are the contract's creation members. A generic union's +/// provider is necessarily generic (it inherits the union's type parameters), so those members are reported +/// as CA1000. The advice cannot be followed either: the members must be static and named Create +/// for the language to find them, so removing them removes the union semantics. +/// +/// +/// The suppressions are deliberately narrow. The nesting and visibility ones apply only when the reported +/// type is an interface named IUnionMembers whose immediate containing type is a union (a type carrying +/// [GenerateUnion] or [Union]). CA1000 is reported at the member, not the interface, so +/// that suppression resolves the member symbol instead: only a public static Create returning its +/// containing union, on a public IUnionMembers nested in that union, is covered. A type of that name +/// nested in an ordinary type, any other nested type in a union, a static helper on the provider, and a +/// static member of the union itself keep the warnings. /// /// /// These are suppression descriptors, not reported rules, so they are not release-tracked. UNI0000 -/// turns the CA1034 suppression off and UNI9001 turns the CA1515 suppression off. +/// turns the CA1034 suppression off, UNI9001 turns the CA1515 suppression off and +/// UNI9002 turns the CA1000 suppression off. /// /// [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -50,26 +61,37 @@ public sealed class UnionMembersNestingSuppressor : DiagnosticSuppressor "IUnionMembers is required by the C# union contract to be a public nested interface, so CA1515 cannot be satisfied without removing the union semantics." ); + /// + /// The suppression of CA1000 on a generic union's IUnionMembers provider's static + /// Create members. + /// + public static readonly SuppressionDescriptor SuppressGenericUnionMembersStatic = new( + "UNI9002", + "CA1000", + "IUnionMembers' static Create members are the C# union contract's creation members; a generic union's provider is necessarily generic, so CA1000 cannot be satisfied without removing the union semantics." + ); + /// public override ImmutableArray SupportedSuppressions => - [SuppressNestedUnionMembers, SuppressNestedUnionMembersVisibility]; + [SuppressNestedUnionMembers, SuppressNestedUnionMembersVisibility, SuppressGenericUnionMembersStatic]; /// public override void ReportSuppressions(SuppressionAnalysisContext context) { foreach (var diagnostic in context.ReportedDiagnostics) { - var descriptor = diagnostic.Id switch + var suppression = diagnostic.Id switch { - "CA1034" => SuppressNestedUnionMembers, - "CA1515" => SuppressNestedUnionMembersVisibility, + "CA1034" when IsUnionMembersProvider(context, diagnostic) => SuppressNestedUnionMembers, + "CA1515" when IsUnionMembersProvider(context, diagnostic) => SuppressNestedUnionMembersVisibility, + "CA1000" when IsUnionMembersCreate(context, diagnostic) => SuppressGenericUnionMembersStatic, _ => null, }; - if (descriptor is null || !IsUnionMembersProvider(context, diagnostic)) + if (suppression is null) continue; - context.ReportSuppression(Suppression.Create(descriptor, diagnostic)); + context.ReportSuppression(Suppression.Create(suppression, diagnostic)); } } @@ -106,6 +128,68 @@ static bool IsUnionMembersProvider(SuppressionAnalysisContext context, Diagnosti return false; } + /// + /// Determines whether the reported diagnostic sits on a static Create member of a union's + /// IUnionMembers provider. + /// + /// + /// CA1000 is reported at the static member rather than at the interface, so the member symbol is + /// resolved instead of the enclosing type. The innermost declared method wins. + /// + static bool IsUnionMembersCreate(SuppressionAnalysisContext context, Diagnostic diagnostic) + { + var location = diagnostic.Location; + if (!location.IsInSource || location.SourceTree is not { } tree) + return false; + + var semanticModel = context.GetSemanticModel(tree); + var root = tree.GetRoot(context.CancellationToken); + var node = root.FindNode(location.SourceSpan, getInnermostNodeForTie: true); + + for (var current = node; current is not null; current = current.Parent) + { + if (semanticModel.GetDeclaredSymbol(current, context.CancellationToken) is not IMethodSymbol method) + continue; + + return IsUnionMembersCreate(method); + } + + return false; + } + + /// + /// Determines whether a method symbol is a creation member of a union's IUnionMembers provider. + /// + /// + /// The conditions mirror the provider contract the generator recognises: a public static method named + /// Create with one by-value parameter, returning the union, declared on a public nested + /// IUnionMembers interface inside that union. + /// + static bool IsUnionMembersCreate(IMethodSymbol method) + { + if (!method.IsStatic || method.DeclaredAccessibility != Accessibility.Public) + return false; + + if (!string.Equals(method.Name, PropertyLibrary.UnionMembersCreateMethodName, StringComparison.Ordinal)) + return false; + + if (method.Parameters.Length != 1 || method.Parameters[0].RefKind != RefKind.None) + return false; + + if ( + method.ContainingType is not { TypeKind: TypeKind.Interface } provider + || provider.DeclaredAccessibility != Accessibility.Public + || !string.Equals(provider.Name, PropertyLibrary.UnionMembersInterfaceName, StringComparison.Ordinal) + ) + return false; + + if (provider.ContainingType is not { } union || !IsUnion(union)) + return false; + + // The provider's Create returns the union it creates; that is the contract the generator reads. + return SymbolEqualityComparer.Default.Equals(method.ReturnType, union); + } + /// /// Determines whether a type is a union. /// diff --git a/src/src/SourceGenerator/README.md b/src/src/SourceGenerator/README.md index daadd81..b806328 100644 --- a/src/src/SourceGenerator/README.md +++ b/src/src/SourceGenerator/README.md @@ -47,9 +47,9 @@ keeps the two in sync. `UNI0001`–`UNI0016` and `UNI9000`; see `docs/wiki/Diagnostics.md`. `AnalyzerReleases.Unshipped.md` is the release-tracked catalogue (RS2008). -`UNI0000` and `UNI9001` are suppression descriptors, not rules: the assembly also ships +`UNI0000`, `UNI9001` and `UNI9002` are suppression descriptors, not rules: the assembly also ships `UnionMembersNestingSuppressor`, which suppresses `CA1034` and `CA1515` on a union's public nested -`IUnionMembers` provider. +`IUnionMembers` provider and `CA1000` on a generic provider's static `Create` members. ## Tests diff --git a/src/src/Unions/Sdk/README.md b/src/src/Unions/Sdk/README.md index 4a70484..a886481 100644 --- a/src/src/Unions/Sdk/README.md +++ b/src/src/Unions/Sdk/README.md @@ -77,8 +77,8 @@ takes `int?`, and `TryGetValue(out int)` returns false for null. `UNI0001`–`UNI0016` and `UNI9000`. See the [Diagnostics](https://github.com/purview-dev/unions/blob/main/docs/wiki/Diagnostics.md) page. Code fixes add a missing `partial` (`UNI0001`), make an inaccessible case type public (`UNI0005`) and, on the fallback path, the -`[Union]` attribute (`UNI0007`). `UNI0000` and `UNI9001` are suppression descriptors, not rules: they suppress -`CA1034` and `CA1515` on a union's nested `IUnionMembers` provider. +`[Union]` attribute (`UNI0007`). `UNI0000`, `UNI9001` and `UNI9002` are suppression descriptors, not rules: +they suppress `CA1034`, `CA1515` and `CA1000` on a union's nested `IUnionMembers` provider. ## Switches diff --git a/src/tests/SourceGenerator.UnitTests/UnionMembersNestingSuppressorTests.cs b/src/tests/SourceGenerator.UnitTests/UnionMembersNestingSuppressorTests.cs index ecc7aa2..d31a489 100644 --- a/src/tests/SourceGenerator.UnitTests/UnionMembersNestingSuppressorTests.cs +++ b/src/tests/SourceGenerator.UnitTests/UnionMembersNestingSuppressorTests.cs @@ -4,10 +4,10 @@ using Microsoft.CodeAnalysis.Diagnostics; using Purview.Unions.SourceGenerator.Analyzers; -// The stand-in analyzer below reports the real CA1034 id and lives in a test assembly. These are -// analyzer-authoring rules for a shipped component; the stand-in is never packed, and the test assembly -// already references Workspaces for the code-fix tests. -#pragma warning disable RS1029 // CA1034 and CA1515 are the real rules under test. +// The stand-in analyzers below report the real CA1034, CA1515 and CA1000 ids and live in a test assembly. +// These are analyzer-authoring rules for a shipped component; the stand-ins are never packed, and the test +// assembly already references Workspaces for the code-fix tests. +#pragma warning disable RS1029 // CA1034, CA1515 and CA1000 are the real rules under test. #pragma warning disable RS1036 // This is a test assembly, not an analyzer project. #pragma warning disable RS1038 // The test assembly references Workspaces for the code-fix tests. #pragma warning disable RS1041 // The test assembly targets net10.0 and net11.0. @@ -19,10 +19,11 @@ namespace Purview.Unions.SourceGenerator; /// Tests for . /// /// -/// CA1034 is reported by a stand-in analyzer rather than by the real CodeQuality rule: a suppressor only -/// sees a diagnostic id and a location, so what is under test is which declarations the suppression lands -/// on, not the real rule's implementation. The stand-in reports on every nested type, which is what the -/// real rule does. +/// CA1034, CA1515 and CA1000 are reported by stand-in analyzers rather than by the real CodeQuality rules: a +/// suppressor only sees a diagnostic id and a location, so what is under test is which declarations the +/// suppression lands on, not the real rules' implementation. The CA1034 and CA1515 stand-ins report on every +/// nested type, and the CA1000 stand-in reports on every static member of a generic type, which is what the +/// real rules do. /// public sealed class UnionMembersNestingSuppressorTests { @@ -139,6 +140,155 @@ public interface IUnionMembers { } await Assert.That(diagnostic.IsSuppressed).IsFalse(); } + [Test] + public async Task CA1000_GivenGenericGenerateUnionMembersProvider_IsSuppressed(CancellationToken cancellationToken) + { + const string source = """ + using Purview.Unions; + + namespace Test; + + [GenerateUnion] + public readonly partial struct Outcome + { + public interface IUnionMembers + { + static Outcome Create(T value) => default; + } + } + """; + + var diagnostics = await AnalyzeAsync(source, cancellationToken); + + var diagnostic = diagnostics.Single(static entry => entry.Id == "CA1000"); + await Assert.That(diagnostic.IsSuppressed).IsTrue(); + } + + [Test] + public async Task CA1000_GivenHandWrittenUnionMembersProvider_IsSuppressed(CancellationToken cancellationToken) + { + // The downstream results test declares its generic union by hand with [Union] rather than + // [GenerateUnion], so the shape is reproduced here: a self-contained UnionAttribute stands in for + // the polyfill the generator emits on net10.0 and the BCL supplies on net11.0. + const string source = """ + namespace System.Runtime.CompilerServices + { + [global::System.AttributeUsage(global::System.AttributeTargets.Struct)] + internal sealed class UnionAttribute : global::System.Attribute { } + } + + namespace Test + { + public readonly record struct ProvidedSome(T Value); + + public readonly record struct ProvidedNone; + + [System.Runtime.CompilerServices.Union] + public readonly struct ProvidedOption : ProvidedOption.IUnionMembers + { + public interface IUnionMembers + { + static ProvidedOption Create(ProvidedSome value) => default; + + static ProvidedOption Create(ProvidedNone value) => default; + } + } + } + """; + + var diagnostics = await AnalyzeAsync(source, cancellationToken); + var ca1000 = diagnostics.Where(static entry => entry.Id == "CA1000").ToArray(); + + await Assert.That(ca1000.Length).IsEqualTo(2); + await Assert.That(ca1000.All(static entry => entry.IsSuppressed)).IsTrue(); + } + + [Test] + public async Task CA1000_GivenIUnionMembersOutsideAUnion_IsNotSuppressed(CancellationToken cancellationToken) + { + const string source = """ + namespace Test; + + public sealed class Container + { + public interface IUnionMembers + { + static Container Create(int value) => new(); + } + } + """; + + var diagnostics = await AnalyzeAsync(source, cancellationToken); + + var diagnostic = diagnostics.Single(static entry => entry.Id == "CA1000"); + await Assert.That(diagnostic.IsSuppressed).IsFalse(); + } + + [Test] + public async Task CA1000_GivenNonCreateStaticMemberInAUnion_IsNotSuppressed(CancellationToken cancellationToken) + { + const string source = """ + using Purview.Unions; + + namespace Test; + + [GenerateUnion] + public readonly partial struct Outcome + { + public interface IUnionMembers + { + static Outcome Describe(T value) => default; + } + } + """; + + var diagnostics = await AnalyzeAsync(source, cancellationToken); + + var diagnostic = diagnostics.Single(static entry => entry.Id == "CA1000"); + await Assert.That(diagnostic.IsSuppressed).IsFalse(); + } + + [Test] + public async Task CA1000_GivenStaticMemberInANonUnionGenericType_IsNotSuppressed( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Test; + + public sealed class Holder + { + public static Holder Of(int value) => new(); + } + """; + + var diagnostics = await AnalyzeAsync(source, cancellationToken); + + var diagnostic = diagnostics.Single(static entry => entry.Id == "CA1000"); + await Assert.That(diagnostic.IsSuppressed).IsFalse(); + } + + [Test] + public async Task CA1000_GivenStaticMemberOnTheUnionItself_IsNotSuppressed(CancellationToken cancellationToken) + { + const string source = """ + using Purview.Unions; + + namespace Test; + + [GenerateUnion] + public readonly partial struct Outcome + { + public static Outcome Empty() => default; + } + """; + + var diagnostics = await AnalyzeAsync(source, cancellationToken); + + var diagnostic = diagnostics.Single(static entry => entry.Id == "CA1000"); + await Assert.That(diagnostic.IsSuppressed).IsFalse(); + } + [System.Diagnostics.CodeAnalysis.SuppressMessage( "Maintainability", "CA1506:Avoid excessive class coupling", @@ -164,6 +314,7 @@ static async Task AnalyzeAsync(string source, CancellationToken ca var analyzers = ImmutableArray.Create( new NestedTypeCa1034Analyzer(), new NestedTypeCa1515Analyzer(), + new StaticMemberOnGenericTypeCa1000Analyzer(), new UnionMembersNestingSuppressor() ); @@ -182,7 +333,7 @@ static async Task AnalyzeAsync(string source, CancellationToken ca .WithAnalyzers(analyzers, analysisOptions) .GetAllDiagnosticsAsync(cancellationToken); - return [.. diagnostics.Where(static diagnostic => diagnostic.Id is "CA1034" or "CA1515")]; + return [.. diagnostics.Where(static diagnostic => diagnostic.Id is "CA1034" or "CA1515" or "CA1000")]; } static ImmutableArray References() @@ -267,4 +418,46 @@ public override void Initialize(AnalysisContext context) ); } } + + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class StaticMemberOnGenericTypeCa1000Analyzer : DiagnosticAnalyzer + { + static readonly DiagnosticDescriptor Rule = new( + "CA1000", + "Do not declare static members on generic types", + "Do not declare static members on generic types", + "Design", + DiagnosticSeverity.Warning, + isEnabledByDefault: true + ); + + public override ImmutableArray SupportedDiagnostics => [Rule]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterSymbolAction( + static symbolContext => + { + var symbol = symbolContext.Symbol; + + if ( + !symbol.IsStatic + || symbol + is IMethodSymbol { MethodKind: MethodKind.Constructor or MethodKind.StaticConstructor } + || symbol.ContainingType is not { IsGenericType: true } + ) + return; + + symbolContext.ReportDiagnostic(Diagnostic.Create(Rule, symbol.Locations.FirstOrDefault())); + }, + SymbolKind.Method, + SymbolKind.Property, + SymbolKind.Field, + SymbolKind.Event + ); + } + } }