Drop init accessors from emitted support types in MSTest.AotReflection.SourceGeneration#9015
Draft
Evangelink wants to merge 8 commits into
Draft
Conversation
Adds a focused unit-test project for the AotReflection source generator PoC introduced in microsoft#8574. The PoC had no test coverage until now. Coverage highlights (13 tests): - Support types emission (TestClassReflectionInfo, TestMethodReflectionInfo, TestPropertyReflectionInfo, TestConstructorReflectionInfo). - Registry emission shape and namespace (MSTest.SourceGenerated). - Empty registry when no [TestClass] is present. - Skipping of static / abstract / open-generic test classes. - Constructor invoker, parameter types / names, async return shape. - Class-level attribute capture; property getter & setter delegate text. - Compile-clean snapshot (catches CS errors the generator may introduce). - Incrementality: support-types step is cached when input is unchanged. Also: - Adds MSTest.AotReflection.SourceGeneration to TestFx.slnx and MSTest.slnf (missing since microsoft#8574). - Adds [InternalsVisibleTo] for the new test project (generator class is internal sealed). Part of microsoft#1837. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…8639)
The PoC's TestClassModelBuilder built its fully-qualified type names with SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier, then fed the resulting FQN into both casts (where '?' is harmless and merely cosmetic, since the emitted setter already uses 'value!') and 'typeof(...)' expressions (where '?' on a reference type is invalid C# and produces CS8639).
Removing the flag fixes 'typeof(string?)' / 'typeof(MyRef?)' while preserving 'typeof(int?)' (nullable value types are rendered via UseSpecialTypes, which is unaffected).
Adds a focused regression test that runs the Roslyn compiler over the generated source and asserts both the textual shape ('typeof(global::Sample.TestContext)', 'typeof(string)', 'typeof(int?)') and the absence of any compile errors.
Discovered while building tests for microsoft#8574; depends on microsoft#9004 for the test infrastructure.
Part of microsoft#1837.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Today TestClassModelBuilder only enumerates members directly declared on the `[TestClass]` type. As soon as a fixture extends a base class, MSTest members (`[TestInitialize]`, `[TestCleanup]`, `[TestMethod]`, the `[TestContext]` property, ...) declared on the base disappear from the generated registry. This change makes the builder walk the inheritance chain (stopping at `System.Object`): * Methods and properties are folded from base types into the model. * Iteration is derived-first; an override or `new`-shadowed member with the same signature/name wins over the base declaration. The signature key includes ref-kinds so genuine overloads survive. * Attributes are collected across the `OverriddenMethod` / `OverriddenProperty` chain (deduped by attribute class FQN) so an `override` that does not re-apply `[TestMethod]` still sees the base attribute - matching the runtime `GetCustomAttributes(inherit: true)` semantics. * Accessibility is broadened to include `Protected` / `ProtectedOrInternal` / `ProtectedAndInternal` so abstract bases can expose their hooks to the emitted code (which lives in the consumer's assembly). * Constructors are NEVER inherited (only taken from the leaf type). Adds 9 new tests covering: inherited methods, multi-level inheritance, overridden virtual (attribute inheritance + de-dup), `new`-hidden methods, overload preservation, inherited properties, no inherited constructors, abstract-base fold-down, and not walking past `System.Object`. Part of microsoft#1837. Depends on microsoft#9004 (test project), microsoft#9005 (typeof nullable). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds an AssemblyAttributes property to the emitted MSTestReflectionMetadata registry containing all attributes declared with [assembly: ...] in the same compilation. The attribute payload is built via the existing AttributeApplicationModel pipeline (reused from class/method attribute emission), so adapters can iterate without calling Assembly.GetCustomAttributes at runtime. Part of microsoft#1837. Stacked on microsoft#9004, microsoft#9005, microsoft#9006. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Part of microsoft#1837. Adds compile-time materialization of `[DataRow]` attribute applications on `[TestMethod]` members. The generator now emits a `DataRows` property on each `TestMethodReflectionInfo` containing a flat `IReadOnlyList<object?[]>` mirroring the runtime shape of `DataRowAttribute.Data`, so a consumer can iterate parameterised cases without re-reading the attributes via reflection. Highlights: - New `DataRowModel(EquatableArray<TypedConstantModel> Arguments)` capturing one row of arguments per attribute application. - `BuildDataRows` detects `Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute` applications and flattens the variadic `params object?[] moreData` tail back into the row so the emitted array matches `DataRowAttribute.Data` rather than preserving a nested array. - Inheritance-aware: reuses the inherited attribute walk introduced in microsoft#9006, so `[DataRow]` applied on a base method (when the override is virtual) is still picked up. - Emitter always emits `DataRows` (empty array for non-data-driven tests) for shape parity with the other `TestMethodReflectionInfo` properties. Deferred to a follow-up: `[DynamicData]` materialization. Resolving the data source method/property/field at compile time is materially more complex (handles `Method`/`Property`/`Field`/`AutoDetect` source kinds plus `object[]` / `IEnumerable<object[]>` return shapes) and warrants its own PR. Tests: - `Generator_EmitsEmptyDataRows_WhenMethodHasNoDataRow` - `Generator_CapturesSingleDataRow_WithScalarArgs` - `Generator_CapturesMultipleDataRows_InDeclarationOrder` - `Generator_FlattensParamsArrayInDataRow` - `Generator_HandlesNullValueInDataRow` Total: 32/32 passing. Depends on microsoft#9004, microsoft#9005, microsoft#9006, microsoft#9007. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Part of microsoft#1837. Changes the emitted `TestMethodReflectionInfo.Invoke` delegate from `Func<object?, object?[]?, object?>` to `Func<object?, object?[]?, Task>` so the caller can `await` a single Task regardless of the underlying test method's signature. Per return shape: - `void` / non-Task sync: `{ call; return Task.CompletedTask; }` — no allocation, no extra wrapping. - `Task` / `Task<T>`: `{ Task? __t = call; return __t ?? Task.CompletedTask; }` — forward the Task; tolerate a misbehaving test returning `null` rather than NRE. - `ValueTask` / `ValueTask<T>`: `{ var __vt = call; return __vt.IsCompletedSuccessfully ? Task.CompletedTask : __vt.AsTask(); }` — fast path for synchronous completion skips `AsTask()` allocation. - Non-void non-Task sync (e.g. `int Test()`): `{ _ = call; return Task.CompletedTask; }` — value discarded, side effects retained. Support type updated to declare `Invoke` as `Func<object?, object?[]?, Task>` with default `static (_, _) => Task.CompletedTask`. Both the support-types file and the registry file now import `System.Threading.Tasks`. Also drops a duplicate blank line left over from PR-A4 (SA1507). Tests: - `Generator_SupportType_DeclaresInvokeAsTaskReturning` - `Generator_InvokerForVoidMethod_ReturnsCompletedTask` - `Generator_InvokerForTaskMethod_ForwardsTask` - `Generator_InvokerForTaskOfTMethod_ForwardsTask` - `Generator_InvokerForValueTaskMethod_UnwrapsViaAsTask` - `Generator_InvokerForValueTaskOfTMethod_UnwrapsViaAsTask` - `Generator_InvokerForNonVoidSyncMethod_DiscardsResultAndReturnsCompletedTask` - `Generator_EmittedRegistry_ImportsSystemThreadingTasks` Total: 40/40 passing (1 existing test updated for new shape). Depends on microsoft#9004, microsoft#9005, microsoft#9006, microsoft#9007, microsoft#9011. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lass shapes
Adds analyzer-style diagnostics emitted by MSTest.AotReflection.SourceGeneration when it encounters a [TestClass] shape it cannot lower into the generated registry:
- AOTSG0001 - static [TestClass] cannot be instantiated.
- AOTSG0002 - open-generic [TestClass] (directly or via outer generic).
- AOTSG0003 - inaccessible [TestClass] (file-local, private/protected nested, or nested in an inaccessible outer).
- AOTSG0004 - generic [TestMethod] cannot be invoked without runtime instantiation.
- AOTSG0005 - [TestMethod]/constructor parameter uses
ef/in/out.
When any of these fire, the offending type or member is skipped from the emitted registry instead of producing invalid C#. Adds 10 new tests covering positive and negative cases, plus AnalyzerReleases.{Shipped,Unshipped}.md to satisfy RS2008.
Part of microsoft#1837.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The support types (TestClassReflectionInfo, TestMethodReflectionInfo, TestPropertyReflectionInfo, TestConstructorReflectionInfo) emitted by MSTest.AotReflection.SourceGeneration are new public-shaped API in the consumer assembly (they are emitted as `internal sealed` but exposed through the static `MSTestReflectionMetadata` registry). The repo guideline forbids `init` accessors on newly introduced public API for MSTest and Microsoft.Testing.Platform.
Replaces `{ get; init; }` with `{ get; set; }` for every emitted auto-property so the existing object-initializer call sites in the emitter continue to work without other changes. Adds a focused unit test that fails if any `{ get; init; }` reappears in the generated support-types source.
Part of microsoft#1837.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates MSTest.AotReflection.SourceGeneration’s emitted “support types” to avoid init accessors (to align with the repo’s “no new init accessors in public-shaped API” guideline), and adds/updates unit-test coverage for the generator output.
Changes:
- Switch emitted support-type properties from
{ get; init; }to{ get; set; }and add a regression test ensuringinitaccessors don’t reappear. - Add/extend generator unit tests and wire the new analyzer + unit test projects into solution views.
- (Stacked changes present in this diff) expand generator output/model to include assembly attributes, data rows, and additional diagnostics plumbing.
Show a summary per file
| File | Description |
|---|---|
| TestFx.slnx | Adds the AOT reflection generator and its unit tests to the main solution view. |
| MSTest.slnf | Adds the AOT reflection generator project to the MSTest solution filter. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs | Adds the MSTest Runner entrypoint for the new unit test project. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTestReflectionMetadataGeneratorTests.cs | Adds comprehensive behavior/regression tests, including the “no init accessors” guard. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests.csproj | Introduces the new unit test project and references the generator project (via IVT). |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/MSTest.AotReflection.SourceGeneration.csproj | Adds analyzer release tracking files and InternalsVisibleTo for unit tests. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Model/TestClassModel.cs | Extends the generator model to represent data rows + assembly metadata. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/TestClassModelBuilder.cs | Updates model-building logic (inheritance walk, attribute inheritance, data row extraction, diagnostics). |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/MSTestReflectionMetadataGenerator.cs | Adjusts incremental pipeline to flow diagnostics and assembly-level metadata. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/MetadataRegistryEmitter.cs | Emits support types with set accessors and updates emitted method invokers / data rows / assembly attributes. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Diagnostics/LocationInfo.cs | Adds equatable location surrogate for incremental-generator diagnostics. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Diagnostics/DiagnosticInfo.cs | Adds equatable diagnostic payload that’s reified during source output. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Diagnostics/DiagnosticDescriptors.cs | Adds the AOTSG0001–AOTSG0005 diagnostic descriptors + lookup. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/AnalyzerReleases.Unshipped.md | Registers the new diagnostics for release tracking. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/AnalyzerReleases.Shipped.md | Adds shipped rules tracking stub. |
Copilot's findings
- Files reviewed: 15/15 changed files
- Comments generated: 2
Comment on lines
+147
to
+153
| private static bool IsAccessibleFromConsumer(ISymbol symbol) | ||
| => symbol.DeclaredAccessibility is | ||
| Accessibility.Public | ||
| or Accessibility.Internal | ||
| or Accessibility.Protected | ||
| or Accessibility.ProtectedOrInternal | ||
| or Accessibility.ProtectedAndInternal; |
Comment on lines
+61
to
+64
| sb.AppendLine("/// <summary>Materialized argument tuples from <c>[DataRow]</c> attributes (empty for non-data-driven tests). Each <c>object?[]</c> corresponds to one <c>[DataRow]</c> application.</summary>"); | ||
| sb.AppendLine("public IReadOnlyList<object?[]> DataRows { get; set; } = Array.Empty<object?[]>();"); | ||
| sb.AppendLine("/// <summary>Direct invoker — replaces <see cref=\"System.Reflection.MethodInfo.Invoke(object, object[])\" />. Always returns a non-null <see cref=\"Task\" /> so the caller can <c>await</c> regardless of whether the underlying test method is <c>void</c>, <c>Task</c>, <c>Task<T></c>, <c>ValueTask</c>, or <c>ValueTask<T></c>; the result value (if any) is discarded.</summary>"); | ||
| sb.AppendLine("public Func<object?, object?[]?, Task> Invoke { get; set; } = static (_, _) => Task.CompletedTask;"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The support types (
TestClassReflectionInfo,TestMethodReflectionInfo,TestPropertyReflectionInfo,TestConstructorReflectionInfo) emitted byMSTest.AotReflection.SourceGenerationare new public-shaped API in the consumer assembly. They are emitted asinternal sealedinto the consuming compilation, but they are surfaced through the staticMSTestReflectionMetadataregistry that the adapter (and, soon, the shippingSourceGeneratedReflectionDataProviderbridge in #1837) consumes.The repo guideline is clear:
This PR replaces every
{ get; init; }inMetadataRegistryEmitter.EmitSupportTypeswith{ get; set; }. The change is purely textual on the emitted code — the existing object-initializer call sites in the emitter continue to compile unchanged.A focused regression test,
Generator_SupportTypes_DoNotUseInitAccessors, asserts that the generatedMSTestReflectionMetadata.SupportTypes.g.csfile contains no{ get; init; }so future contributions can't accidentally reintroduce the pattern.Dependencies
This PR is stacked on top of #9014 (analyzer diagnostics). Once that lands the diff here will collapse to just the emitter and the one regression test.
Part of #1837.