AotReflection: strip nullable annotation from emitted typeof(...) (CS8639)#9005
Open
Evangelink wants to merge 2 commits into
Open
Conversation
Adds a focused unit-test project for the AotReflection source generator PoC introduced in microsoft#8574. The PoC had no test coverage until now. Coverage highlights (13 tests): - Support types emission (TestClassReflectionInfo, TestMethodReflectionInfo, TestPropertyReflectionInfo, TestConstructorReflectionInfo). - Registry emission shape and namespace (MSTest.SourceGenerated). - Empty registry when no [TestClass] is present. - Skipping of static / abstract / open-generic test classes. - Constructor invoker, parameter types / names, async return shape. - Class-level attribute capture; property getter & setter delegate text. - Compile-clean snapshot (catches CS errors the generator may introduce). - Incrementality: support-types step is cached when input is unchanged. Also: - Adds MSTest.AotReflection.SourceGeneration to TestFx.slnx and MSTest.slnf (missing since microsoft#8574). - Adds [InternalsVisibleTo] for the new test project (generator class is internal sealed). Part of microsoft#1837. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…8639)
The PoC's TestClassModelBuilder built its fully-qualified type names with SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier, then fed the resulting FQN into both casts (where '?' is harmless and merely cosmetic, since the emitted setter already uses 'value!') and 'typeof(...)' expressions (where '?' on a reference type is invalid C# and produces CS8639).
Removing the flag fixes 'typeof(string?)' / 'typeof(MyRef?)' while preserving 'typeof(int?)' (nullable value types are rendered via UseSpecialTypes, which is unaffected).
Adds a focused regression test that runs the Roslyn compiler over the generated source and asserts both the textual shape ('typeof(global::Sample.TestContext)', 'typeof(string)', 'typeof(int?)') and the absence of any compile errors.
Discovered while building tests for microsoft#8574; depends on microsoft#9004 for the test infrastructure.
Part of microsoft#1837.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a C# source-generation bug in the MSTest AOT reflection PoC where nullable reference annotations (?) were being emitted inside typeof(...), causing CS8639 during compilation. This supports the broader NativeAOT work tracked in #1837 and adds a focused unit-test project to prevent regressions.
Changes:
- Adjust
TestClassModelBuildersymbol display formatting to stop including nullable reference modifiers in fully-qualified type names, preventingtypeof(SomeRefType?)emission. - Add a new unit-test project (
MSTest.AotReflection.SourceGeneration.UnitTests) with a regression test that compiles the generated output and asserts the correctedtypeof(...)shapes. - Wire the generator and its unit-test project into
TestFx.slnxandMSTest.slnffor discoverability/build coverage.
Show a summary per file
| File | Description |
|---|---|
| TestFx.slnx | Adds the AOT reflection generator + its new unit-test project to the main solution. |
| MSTest.slnf | Includes the AOT reflection generator project in the MSTest solution filter. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs | Test host entry point for running the new unit-test project under MTP. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTestReflectionMetadataGeneratorTests.cs | Adds baseline tests plus the new regression test ensuring nullable annotations are stripped from typeof(...) and the emitted code compiles without errors. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests.csproj | New unit-test project referencing the generator and Roslyn for compile-clean snapshot validation. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/MSTest.AotReflection.SourceGeneration.csproj | Grants test project access to the internal generator via InternalsVisibleTo. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/TestClassModelBuilder.cs | Removes IncludeNullableReferenceTypeModifier from the fully-qualified display format to avoid emitting ? on reference types in typeof(...). |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1837. Depends on #9004 (introduces the test project used here).
Why
While building the AotReflection unit-test project in #9004, the new compile-clean snapshot test surfaced a real bug in the PoC: the generated source produced
CS8639: The typeof operator cannot be used on a nullable reference type.Root cause
TestClassModelBuilder.FullyQualifiedFormatwas built withSymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier. The resulting FQN string was then fed into two very different emission sites:(global::Sample.TestContext?)value!— where the?is harmless (thevalue!already suppresses the nullability warning).typeof(...)expressions — where?on a reference type is invalid C# (CS8639).Fix
Drop
IncludeNullableReferenceTypeModifierfromFullyQualifiedFormat. This:?from reference-type FQNs everywhere (no behavioural impact on casts).?on nullable value types (int?,DateTime?) because those are rendered viaUseSpecialTypes/ generics options, which are unaffected.The property-setter snapshot assertion is updated to match (
(global::Sample.TestContext)value!instead of(global::Sample.TestContext?)value!).Regression test
New
Generator_StripsNullableAnnotation_FromTypeofExpressionstest:TestContext?property, astring?parameter, and anint?parameter.typeof(global::Sample.TestContext),typeof(string),typeof(int?); and the absence oftypeof(global::Sample.TestContext?)/typeof(string?).Validation
dotnet build+dotnet runforMSTest.AotReflection.SourceGeneration.UnitTests:14/14passing,0warnings.