[AotReflection 2/N] Walk inheritance chain for methods and properties#9006
Open
Evangelink wants to merge 4 commits into
Open
[AotReflection 2/N] Walk inheritance chain for methods and properties#9006Evangelink wants to merge 4 commits into
Evangelink wants to merge 4 commits into
Conversation
Adds a focused unit-test project for the AotReflection source generator PoC introduced in microsoft#8574. The PoC had no test coverage until now. Coverage highlights (13 tests): - Support types emission (TestClassReflectionInfo, TestMethodReflectionInfo, TestPropertyReflectionInfo, TestConstructorReflectionInfo). - Registry emission shape and namespace (MSTest.SourceGenerated). - Empty registry when no [TestClass] is present. - Skipping of static / abstract / open-generic test classes. - Constructor invoker, parameter types / names, async return shape. - Class-level attribute capture; property getter & setter delegate text. - Compile-clean snapshot (catches CS errors the generator may introduce). - Incrementality: support-types step is cached when input is unchanged. Also: - Adds MSTest.AotReflection.SourceGeneration to TestFx.slnx and MSTest.slnf (missing since microsoft#8574). - Adds [InternalsVisibleTo] for the new test project (generator class is internal sealed). Part of microsoft#1837. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…8639)
The PoC's TestClassModelBuilder built its fully-qualified type names with SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier, then fed the resulting FQN into both casts (where '?' is harmless and merely cosmetic, since the emitted setter already uses 'value!') and 'typeof(...)' expressions (where '?' on a reference type is invalid C# and produces CS8639).
Removing the flag fixes 'typeof(string?)' / 'typeof(MyRef?)' while preserving 'typeof(int?)' (nullable value types are rendered via UseSpecialTypes, which is unaffected).
Adds a focused regression test that runs the Roslyn compiler over the generated source and asserts both the textual shape ('typeof(global::Sample.TestContext)', 'typeof(string)', 'typeof(int?)') and the absence of any compile errors.
Discovered while building tests for microsoft#8574; depends on microsoft#9004 for the test infrastructure.
Part of microsoft#1837.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Today TestClassModelBuilder only enumerates members directly declared on the `[TestClass]` type. As soon as a fixture extends a base class, MSTest members (`[TestInitialize]`, `[TestCleanup]`, `[TestMethod]`, the `[TestContext]` property, ...) declared on the base disappear from the generated registry. This change makes the builder walk the inheritance chain (stopping at `System.Object`): * Methods and properties are folded from base types into the model. * Iteration is derived-first; an override or `new`-shadowed member with the same signature/name wins over the base declaration. The signature key includes ref-kinds so genuine overloads survive. * Attributes are collected across the `OverriddenMethod` / `OverriddenProperty` chain (deduped by attribute class FQN) so an `override` that does not re-apply `[TestMethod]` still sees the base attribute - matching the runtime `GetCustomAttributes(inherit: true)` semantics. * Accessibility is broadened to include `Protected` / `ProtectedOrInternal` / `ProtectedAndInternal` so abstract bases can expose their hooks to the emitted code (which lives in the consumer's assembly). * Constructors are NEVER inherited (only taken from the leaf type). Adds 9 new tests covering: inherited methods, multi-level inheritance, overridden virtual (attribute inheritance + de-dup), `new`-hidden methods, overload preservation, inherited properties, no inherited constructors, abstract-base fold-down, and not walking past `System.Object`. Part of microsoft#1837. Depends on microsoft#9004 (test project), microsoft#9005 (typeof nullable). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends MSTest.AotReflection.SourceGeneration so TestClassModelBuilder walks a [TestClass] type’s inheritance chain (derived → base → …, stopping before System.Object) and folds inherited methods/properties into the generated metadata registry, improving NativeAOT friendliness by reducing runtime reflection fallback.
Changes:
- Update
TestClassModelBuilderto include inherited methods/properties, de-duplicate members with derived-first precedence, and collect attributes across override chains. - Add a new unit test project with coverage for inherited members, overrides/new-hiding, overloads, constructor non-inheritance, and stopping at
System.Object. - Wire the generator project + new unit test project into the repo solution filters.
Show a summary per file
| File | Description |
|---|---|
| TestFx.slnx | Adds the AotReflection generator project and its new unit test project to the full solution. |
| MSTest.slnf | Includes the AotReflection generator project in the MSTest solution filter. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/MSTest.AotReflection.SourceGeneration.csproj | Adds InternalsVisibleTo for the new unit test project. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/TestClassModelBuilder.cs | Implements inheritance-walking for methods/properties and override-chain attribute collection. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests.csproj | Introduces a new executable unit test project for the generator. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs | Test runner bootstrap for the new unit test project. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTestReflectionMetadataGeneratorTests.cs | Adds new generator behavior tests, including inheritance scenarios and compile-clean assertions. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 2
Comment on lines
+96
to
+102
| private static bool IsAccessibleFromConsumer(ISymbol symbol) | ||
| => symbol.DeclaredAccessibility is | ||
| Accessibility.Public | ||
| or Accessibility.Internal | ||
| or Accessibility.Protected | ||
| or Accessibility.ProtectedOrInternal | ||
| or Accessibility.ProtectedAndInternal; |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in 9084c28. IsAccessibleFromConsumer now only accepts Public, Internal, and ProtectedOrInternal. The generated invoker lives in a sibling static class (not a derived type), so Protected/ProtectedAndInternal members are not callable from it and would have caused CS-errors in the generated source. Added regression test Generator_SkipsProtectedAndPrivateProtectedMembers that asserts both the registry output and zero generated-source diagnostics.
Comment on lines
+216
to
+228
| foreach (AttributeData attribute in attributes) | ||
| { | ||
| if (attribute.AttributeClass is not { } attributeClass) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| string key = attributeClass.ToDisplayString(FullyQualifiedFormat); | ||
| if (seen.Add(key)) | ||
| { | ||
| builder.Add(attribute); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in 9084c28. AppendUnique now consults a new AllowsMultiple helper that reads AttributeUsage(AllowMultiple = …) from the attribute class (walking its base chain per CLI shadowing rules). Multi-instance attributes like [TestCategory] are kept in full; single-instance attributes still dedup by fully-qualified name. Added regression test Generator_KeepsAllowMultipleAttributes_AcrossOverrideChain.
- IsAccessibleFromConsumer now excludes Protected / ProtectedAndInternal members since the generated invoker class is not a derived type and cannot legally reference them. - Attribute aggregation now preserves all instances of attributes marked with AttributeUsage(AllowMultiple = true) (e.g. [TestCategory]) instead of collapsing them by attribute type. - Adds regression tests covering both behaviors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 (test project) and #9005 (typeof nullable fix).
What
Teach MSTest.AotReflection.SourceGeneration's TestClassModelBuilder to walk the inheritance chain of a [TestClass] type so that members declared on a base class are included in the generated metadata registry.
Why
Today the builder only enumerates members directly declared on the [TestClass] type. As soon as a fixture extends a base class, MSTest members (
[TestInitialize],[TestCleanup],[TestMethod], the[TestContext]property, ...) declared on the base disappear from the registry – which would make any non-trivial test suite fall back to runtime reflection.How
The new
Buildwalksderived → base → ... → object:overrideornew-shadowed member with the same signature/name wins. The signature key includes parameter ref-kinds so genuine overloads survive.MemberInfo.GetCustomAttributes(inherit: true): attributes from theOverriddenMethod/OverriddenPropertychain are collected and deduped by attribute class FQN. Anoverridethat does not re-apply[TestMethod]still sees the base attribute.Protected/ProtectedOrInternal/ProtectedAndInternalso abstract bases can expose their hooks. The emitted code lives in the consumer's assembly so it has access.System.Objectso we never emit entries forToString/Equals/...Tests
9 new tests covering:
Generator_IncludesMethodsFromBaseType)Generator_IncludesMethodsFromMultiLevelInheritance)Generator_OverriddenVirtualMethod_KeepsOnlyDerivedImplementation)new-hidden method (Generator_NewKeywordHiddenMethod_DedupsBySignature)Generator_OverloadsWithDifferentSignatures_AreAllPreserved)Generator_IncludesPropertiesFromBaseType)Generator_DoesNotInheritConstructors)Generator_AbstractBaseWithConcreteDerived_FoldsBaseMembers)System.Object(Generator_DoesNotWalkPastSystemObject)All 23 tests in the suite pass locally.