Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Add MSTest reflection source generator (issue #1837) by Evangelink · Pull Request #8586 · microsoft/testfx · GitHub
Skip to content

Add MSTest reflection source generator (issue #1837) - #8586

Merged
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837
Jun 5, 2026
Merged

Add MSTest reflection source generator (issue #1837)#8586
Amaury Levé (Evangelink) merged 18 commits into
mainfrom
dev/amauryleve/sourcegen-reflection-issue-1837

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented May 25, 2026

Copy link
Copy Markdown
Member

Fixes part of #1837.

What

Introduces a new Roslyn IIncrementalGenerator (project MSTest.SourceGeneration, under src/Analyzers/) that discovers [TestClass] types at compile time and emits a [ModuleInitializer] registering a SourceGeneratedReflectionDataProvider for the user's assembly.

It also adds the runtime infrastructure in MSTestAdapter.PlatformServices (folder SourceGeneration/) to swap the IReflectionOperations and IFileOperations services for source-gen-backed implementations once metadata is registered.

This is the first step toward Native AOT support: when the generator runs, MSTest reads test metadata from compile-time-known data instead of doing reflection at runtime. The feature is opt-in — when no metadata is registered, the platform keeps using the existing reflection-based implementations.

Why

Issue #1837 tracks Native AOT support for MSTest. PR #8263 introduced the IReflectionOperations service abstraction as a prerequisite. This PR delivers the next building block by adding the source generator that will populate that abstraction without reflection.

Layout

  • Generator (new project): src/Analyzers/MSTest.SourceGeneration/ReflectionMetadataGenerator and its emitters/models/helpers, packaged as analyzers/dotnet/cs.
  • Runtime hook & shims: src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook, SourceGeneratedReflectionDataProvider, CompositeSourceGeneratedReflectionDataProvider, SourceGeneratedReflectionOperations, SourceGeneratedFileOperations, SourceGeneratorToggle.
  • Hook surface on PlatformServiceProvider: a single internalSetSourceGeneratedOperations method used by ReflectionMetadataHook to swap providers.
  • Removed: the legacy src/Adapter/MSTest.Engine/ project and its unit tests, which the new design replaces. (This is what shows up in the solution-file diff.)
  • Unit tests: test/UnitTests/MSTest.SourceGeneration.UnitTests/.
  • PublicAPI.Unshipped.txt updated.

Scope (MVP)

  • Generator emits a per-assembly [ModuleInitializer] that calls ReflectionMetadataHook.SetMetadata.
  • Composite provider supports multiple test assemblies registering concurrently.
  • Unit tests cover the happy path ([TestClass] + [TestMethod]), static/abstract classes being skipped, empty-assembly emission, and a Roslyn-compiles-cleanly smoke test.

Known follow-ups (out of scope for this PR)

These are intentionally deferred to keep this PR reviewable:

  • Populate the rest of the data bag (AssemblyAttributes, TypeAttributes, TypeProperties, TypeMethodAttributes, TypeConstructorsInvoker, TypeMethodLocations). The MVP only emits the assembly name, type list, and per-type method list.
  • [ModuleInitializer] polyfill for netstandard2.0 / older TFM consumers.
  • MSBuild gating (opt-in property like EnableMSTestSourceGeneration).
  • NuGet packaging of the generator into the MSTest analyzer set.
  • Wire up coverage of DataRow / DynamicData / inherited attributes / base-class roll-up.
  • AssemblyInitialize / ClassInitialize / TestInitialize lifecycle.

Validation

Built locally with build.cmd -c Debug (0 warnings, 0 errors).

Notes for reviewers

  • Public API is intentionally minimal: only ReflectionMetadataHook.SetMetadata and the SourceGeneratedReflectionDataProvider shape are public, because the generated module initializer needs to call them.
  • SourceGeneratorToggle uses Interlocked.Exchange to ensure exactly-once swap and to make the toggle race-free if multiple assemblies' module initializers run.
  • ReflectionMetadataGenerator skips static and abstract types — they cannot be discovered as test classes.
  • IReflectionOperations.GetType(string) delegates straight to the fallback so callers cannot bind to a same-named type from the wrong assembly via the composite TypesByName lookup; the assembly-qualified GetType(Assembly, string) overload is the one that uses the source-generated data.
  • Happy to split this into smaller PRs (e.g., runtime infra first, then generator) if that's easier to review.

CopilotAI review requested due to automatic review settings May 25, 2026 22:31

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an opt-in MSTest reflection metadata source generator (MSTestAdapter.PlatformServices.SourceGeneration) plus new runtime infrastructure in MSTestAdapter.PlatformServices to swap IReflectionOperations/IFileOperations to source-gen-backed implementations via a generated [ModuleInitializer]. This is intended as an early building block toward NativeAOT support (#1837).

Changes:

  • Add a new incremental generator project that discovers [TestClass]/[TestMethod] and emits a module initializer registering metadata.
  • Add runtime “source-generated” reflection/file operations and a public hook (ReflectionMetadataHook) for generated code to register metadata.
  • Add unit tests for the generator and wire new projects into TestFx.slnx and MSTest.slnf, plus update PublicAPI.Unshipped.txt.
Show a summary per file
FileDescription
TestFx.slnxAdds the new generator project + its unit test project to the full solution.
MSTest.slnfAdds the new generator project + its unit test project to the MSTest solution filter.
src/Adapter/MSTestAdapter.PlatformServices/PlatformServiceProvider.csAdds internal hook to swap IReflectionOperations/IFileOperations.
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txtDeclares new public API surface (ReflectionMetadataHook, SourceGeneratedReflectionDataProvider).
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/*.csAdds runtime hook + source-gen-backed reflection/file operations + toggle.
src/Adapter/MSTestAdapter.PlatformServices.SourceGeneration/*Adds generator models/helpers + incremental generator + emitter + banned symbols.
test/UnitTests/MSTestAdapter.PlatformServices.SourceGeneration.UnitTests/*Adds generator unit tests + test runner program + test project.

Copilot's findings

  • Files reviewed: 19/19 changed files
  • Comments generated: 14

@Evangelink

Copy link
Copy Markdown
MemberAuthor

🔍 Build Failure Analysis

Summary — The build failed due to a missing interface method and enforced code style violations across multiple target frameworks.

Root cause 1: Missing interface method SetSourceGeneratedOperations

The code in ReflectionMetadataHook.cs calls SetSourceGeneratedOperations on an IPlatformServiceProvider interface reference, but this method is not declared in the interface — it only exists as an internal method on the concrete PlatformServiceProvider class (line 181).

Affected files / errors

Proposed fix

Cast PlatformServiceProvider.Instance to the concrete type before calling the internal method:

- PlatformServiceProvider.Instance.SetSourceGeneratedOperations(reflectionOperations, fileOperations);+ ((PlatformServiceProvider)PlatformServiceProvider.Instance).SetSourceGeneratedOperations(reflectionOperations, fileOperations);

Alternative fix (if you want to expose this as part of the interface contract):

Add the method to IPlatformServiceProvider.cs:

 ITestContext GetTestContext(ITestMethod? testMethod, string? testClassFullName, IDictionary<string, object?> properties, IMessageLogger messageLogger, UTF.UnitTestOutcome outcome);
++ /// <summary>+ /// Swaps the cached reflection and file operations with source-generated implementations.+ /// </summary>+ void SetSourceGeneratedOperations(IReflectionOperations reflectionOperations, IFileOperations fileOperations);
}

And change the PlatformServiceProvider implementation from internal to public.


Root cause 2: Code style violations (IDE0032 — Use auto property)

Four fields are declared with explicit backing fields but could be converted to auto-properties. The analyzer rule IDE0032 is being enforced as an error.

Affected files / errors

Proposed fix

Convert explicit backing fields to auto-properties. These are readonly fields that are only assigned once in the constructor, so they can use the newer auto-property syntax with initializers.


Root cause 3: Code style violations (IDE0046 — Simplify conditional expression)

Three locations have if statements that can be simplified to conditional expressions. The analyzer rule IDE0046 is being enforced as an error.

Affected files / errors

Proposed fix

Simplify if/return patterns to ternary expressions or null-coalescing operators where appropriate.


Build overview
  • Project: MSTestAdapter.PlatformServices.csproj
  • Target frameworks: net8.0, net9.0 (errors occur in both)
  • Configuration: Debug
  • Exit code: 1 (failure)
  • Total unique errors: 7 distinct issues (multiplied across TFMs = 14 total occurrences)

The build failed during the CoreCompile target when compiling the newly added SourceGeneration code.

All MSBuild errors (14 occurrences)
CodeProjectFile:LineMessage
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net8.0)
CS1061MSTestAdapter.PlatformServicesReflectionMetadataHook.cs:37'IPlatformServiceProvider' does not contain a definition for 'SetSourceGeneratedOperations'... (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratorToggle.cs:13Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:15Use auto property (net9.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net8.0)
IDE0032MSTestAdapter.PlatformServicesSourceGeneratedFileOperations.cs:16Use auto property (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:26'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:257'if' statement can be simplified (net9.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net8.0)
IDE0046MSTestAdapter.PlatformServicesSourceGeneratedReflectionOperations.cs:291'if' statement can be simplified (net9.0)

🤖 Generated by the Build Failure Analysis workflow · commit 6887de2

Generated by Build Failure Analysis for issue #8586 · ● 1.3M ·

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Build Failure Analysis for issue #8586 · ● 1.3M

@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 6887de2 to 73050eeCompareMay 28, 2026 16:46
Consolidates the experimental MSTest.Engine package into the existing MSTest.SourceGeneration package, addresses PR review comments, and adds correctness fixes for ref/out/in parameter signatures, inaccessible nested test classes, file-local test classes, open generic test classes, overridden method deduplication, and composite-provider null handling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 28, 2026 17:03
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/sourcegen-reflection-issue-1837 branch from 73050ee to a670c54CompareMay 28, 2026 17:03

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 171/172 changed files
  • Comments generated: 2

Comment threadTestFx.slnx
- Delegate IReflectionOperations.GetType(string) to fallback so simple
type names cannot bind to a same-named type from the wrong assembly
via the composite TypesByName lookup (matches Type.GetType semantics).
- Simplify HasByRefParameter to a single LINQ Any() expression.
- Drop unused reassignment of GeneratorDriver in the generator test
harness's RunGeneratorAndGetCompilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…gen-reflection-issue-1837
# Conflicts:
#	src/Adapter/MSTest.Engine/Engine/TestFrameworkEngine.cs
#	src/Adapter/MSTest.Engine/PublicAPI/PublicAPI.Unshipped.txt
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeActionTaskParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionParameterizedTestNode.cs
#	src/Adapter/MSTest.Engine/TestNodes/InternalUnsafeAsyncActionTaskParameterizedTestNode.cs
#	test/UnitTests/MSTest.Engine.UnitTests/BFSTestNodeVisitorTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 08:04
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 1, 2026 08:04

CopilotAI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 8578ce3. All conflicts were "deleted by us" — files in MSTest.Engine that our PR intentionally removed but main had modified. I kept our deletions and also removed the new InternalUnsafeParameterizedTestNodeBase.cs file that main added to the deleted directory. All other changes from main were auto-merged cleanly.

…flection
- ReflectionMetadataEmitter: emit [DynamicDependency(All, typeof(T))] per test
class on the [ModuleInitializer], so the trimmer keeps constructors and other
reflected members alive (otherwise discovery fails with 'Cannot find a valid
constructor for test class').
- ReflectionMetadataEmitter: annotate ResolveMethod's Type parameter with
[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)] to satisfy
IL2070 in the generated module initializer.
- SourceGeneratedReflectionOperations: stop routing fallback through
_fallback.GetCustomAttributesCached. ReflectionOperations.NotCachedReflectionAccessor
reads PlatformServiceProvider.Instance.ReflectionOperations, which after
SetMetadata is the source-gen wrapper itself -- causing infinite mutual recursion
and a StackOverflowException at runtime. Use _fallback.GetCustomAttributes
(direct reflection) instead.
- MSTest.Sdk NativeAOT.targets: add MSTest.TestAdapter package reference and set
EnableMSTestRunner/IsTestingPlatformApplication = true (mirroring ClassicEngine.targets)
so MSTestAdapter.PlatformServices.dll (the source-generator runtime hook host) is
available to NAOT-published apps.
- NativeAotTests / SdkTests / TrimTests: tolerate upstream IL warnings from
Microsoft.TestPlatform.ObjectModel and System.Private.DataContractSerialization
(warnAsError: false) and assert via shared TrimAndAotAssertions.MSTestOwnedSourceFiles
that MSTest-owned source files do not appear in publish output, mirroring the
pattern established in PR #8686. Rename Publish_ShouldNotProduceTrimWarnings to
Publish_WithSourceGeneration_DoesNotSurfaceMSTestOwnedTrimWarnings.
- NativeAotTests: use AssertOutputContainsSummary helper (current MTP output format).
- samples/NativeAotRunner/TestProject1: convert to MSTest.Sdk shape and drop the
pinned MSTest.SourceGeneration 2.0.0-alpha.26228.3 reference (which emitted now-
removed Microsoft.Testing.Framework.TestNode types and broke the WindowsSamples
CI legs).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 1, 2026 13:11

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 174/175 changed files
  • Comments generated: 1

A [TestMethod] declared as Test<T>(T value) would have its parameter
types collected as ypeof(T), but the generated module initializer is
non-generic, so T does not bind there and the compilation fails.
Reflection mode handles generic test methods at runtime, so opting into
the generator must not turn a valid program into a build error.
Add an IsGenericMethod guard alongside the existing open-generic-class
and by-ref-parameter guards, and add a unit test covering both the
generic-with-params and generic-without-params shapes.
Addresses PR review feedback:
#8586 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 3, 2026 12:55
…ver)]
The single public Register entry point exists only because the source
generator's [ModuleInitializer] needs to call across the assembly
boundary into MSTestAdapter.PlatformServices. Hand-written code should
never use it. Marking the type and method as EditorBrowsable.Never plus
strengthening the XML docs makes that intent obvious to anyone browsing
the API.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 180/181 changed files
  • Comments generated: 1

Comment threadsrc/Analyzers/MSTest.SourceGeneration/Models/TestAssemblyMetadata.cs Outdated
Evangelinkand others added 2 commits June 4, 2026 00:19
Returning IEnumerator<T> from a struct enumerator goes through an
IEnumerable<T> cast and allocates on every foreach. The source-generator
pipeline iterates metadata.Classes, cls.Methods, and method.ParameterTypes
on every incremental tick, so this was a real allocation hot spot.
Return ImmutableArray<T>.Enumerator (a struct) by value so the foreach
binds to the duck-typed pattern with no boxing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The toggle's UseSourceGenerator getter was never read; Enable() was
called once from ReflectionMetadataHook.Register but had no observable
effect. The actual provider swap is done in the same code path via
PlatformServiceProvider.SetSourceGeneratedOperations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 12:35

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 179/180 changed files
  • Comments generated: 2

…ress review feedback
- Add docs/source-generator/design.md covering scope, emitter gaps,
the three fallback categories (A/B/C), discovery limitations,
trim/AOT story (warnings vs. runtime), recommended pairing with
`TrimmerRootAssembly`, perf positioning vs. delegate-based
generators, what wiring the AotReflection PoC unlocks beyond perf,
and a sunset plan for the current alpha packages.
- Emit `[DynamicDependency(All, typeof(BaseType))]` for every accessible
non-generic base in a test class's inheritance chain so members
declared on an abstract base (`[ClassInitialize]`,
`[AssemblyInitialize]`, `TestContext` setter, ...) survive
trimming under PublishAot / PublishTrimmed.
- Categorize every fallback in SourceGeneratedReflectionOperations
(A = generator-gap, closable; B = contract-mismatch, by design;
C = cross-assembly, unavoidable) and label each call site so future
contributors do not silently fall through.
- Address PR review threads:
- Simplify GetRuntimeMethod to delegate to the reflection fallback.
The previous custom loop duplicated GetRuntimeMethods' scan and
risked diverging from Type.GetMethod binder semantics
(overload resolution, generic / by-ref handling).
- Route GetType(Assembly, string) through a new per-provider
TryGetTypeByName virtual so two assemblies with the same
fully-qualified type name no longer shadow each other in the
composite's merged snapshot. TypesByName is no longer merged
centrally; the per-provider lookup is consulted directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Pushed c896beb covering the remaining review feedback plus the design documentation we've been discussing.

Review threads addressed (both now resolved)

  • GetRuntimeMethod no longer does its own GetRuntimeMethods scan + manual parameter match. It delegates directly to the reflection fallback, which avoids the duplicate scan (we were scanning twice — once in our loop, once again in _fallback.GetRuntimeMethod on miss) and keeps Type.GetMethod's binder semantics for overload resolution / generic / by-ref handling. Labelled as Category B in the class-level XML doc.
  • GetType(Assembly, string) no longer reads from the merged TypesByName (where same-FQN entries from two assemblies would shadow each other). It now goes through a new SourceGeneratedReflectionDataProvider.TryGetTypeByName(Assembly, string, out Type) virtual that the composite overrides to route the lookup through ProvidersByAssembly (same pattern as GetAssemblyAttributes). The merge step no longer carries TypesByName at all — collisions are impossible by construction rather than tolerated.

Documentation

  • docs/source-generator/design.md — comprehensive design doc covering scope, what the emitter populates today and what it doesn't (mapped to each provider field), the three-category fallback rule (A generator-gap, B contract-mismatch, C cross-assembly), discovery limitations (inherited [TestClass], generics, by-ref params, private/file-local, etc.), the trim/AOT story (warnings vs runtime), recommended pairing with TrimmerRootAssembly, perf positioning vs. delegate-based generators like TUnit, what wiring the MSTest.AotReflection.SourceGeneration PoC unlocks beyond perf (Category-A fallbacks disappearing, [DynamicDependency] math becoming obsolete, compile-time [DataRow] validation, ref/out/in support, IDE source navigation, etc.), and a sunset plan for the current alpha packages (delete from main behind a git tag rather than gating — git is the time machine, not gated dead code).
  • Linked from docs/README.md under a new Design notes section.
  • Every fallback in SourceGeneratedReflectionOperations now carries a // Category A/B/C: <reason> comment so the design surface stays auditable.

Abstract base [DynamicDependency] chain

  • The generator now walks the inheritance chain and emits [DynamicDependency(All, typeof(BaseType))] for every accessible non-generic base. This means [ClassInitialize] / [AssemblyInitialize] / TestContext setters declared on abstract bases now survive trimming under PublishAot / PublishTrimmed. Five new tests cover full-chain, dedup, System.Object skip, inaccessible-base skip, generic-base skip.

Verification

  • MSTest.SourceGeneration.UnitTests: 31/31 passing on net8.0.
  • MSTestAdapter.PlatformServices.UnitTests: 851/851 passing on net8.0.
  • MSTestAdapter.PlatformServices.csproj builds clean on net462, net8.0, net8.0-windows10.0.18362.0, net9.0, net9.0-windows10.0.17763.0 (UAP needs desktop msbuild, unrelated).

- SourceGeneratedReflectionOperations.GetRuntimeMethod: drop redundant manual loop and delegate directly to the fallback provider (which already handles partial source-gen data).
- InheritedTestClassAttributeWithSourceGeneratorAnalyzer.HasDirectAttribute: simplify foreach to LINQ Any() per code-quality bot suggestion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 4, 2026 22:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

markdownlint MD033 forbids inline HTML except for <a> tags. Escape the
placeholder text '<link>' so it renders as literal angle brackets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 10:10

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 181/182 changed files
  • Comments generated: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved based on Amaury Levé (@Evangelink) 's request. Did not review, only approved to enable further work on this issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Evangelink@azat-msft