Skip to content

Fix property lowering (static/indexer/set-only) in MSTest.AotReflection.SourceGeneration#9077

Merged
Evangelink merged 2 commits into
mainfrom
dev/amauryleve/aot-srcgen-property-lowering
Jun 12, 2026
Merged

Fix property lowering (static/indexer/set-only) in MSTest.AotReflection.SourceGeneration#9077
Evangelink merged 2 commits into
mainfrom
dev/amauryleve/aot-srcgen-property-lowering

Conversation

@Evangelink

Copy link
Copy Markdown
Member

The MSTest.AotReflection.SourceGeneration registry emitted invalid C# for some otherwise-valid [TestClass] shapes, which would block the registry from compiling once it is consumed:

  • static properties produced ((T)instance).Prop (CS0176 — cannot access a static member through an instance),
  • indexer properties produced ((T)instance).this[] (garbage),
  • set-only / non-readable properties emitted an unconditional getter referencing a missing accessor.

Fix

  • Static members are now accessed through the type name (T.Prop).
  • Indexers are skipped entirely (they cannot be represented by the name-based Get/Set delegate shape).
  • The Get delegate throws when there is no accessible getter, instead of referencing one that does not exist.
  • Adds IsStatic and HasGettableValue to TestPropertyModel.

Tests

Three new tests (static access, indexer skip, set-only getter) plus existing coverage; 66/66 generator unit tests pass.

Part of #1837.

…t.AotReflection.SourceGeneration

The generated MSTestReflectionMetadata registry emitted invalid C# for some
otherwise-valid [TestClass] shapes:

- static properties produced ((T)instance).Prop (CS0176)
- indexer properties produced ((T)instance).this[] (garbage)
- set-only / non-readable properties emitted an unconditional getter

Now static members are accessed through the type name, indexers are skipped
(they cannot be modeled by the name-based Get/Set delegate shape), and the
Get delegate throws when there is no accessible getter. Adds IsStatic and
HasGettableValue to TestPropertyModel and three covering tests.

Part of #1837.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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

Fixes C# emission bugs in the MSTest.AotReflection.SourceGeneration generator so the generated MSTestReflectionMetadata.Registry.g.cs compiles for additional valid [TestClass] shapes (static properties, indexers, and set-only properties), improving robustness of the AOT reflection-metadata registry work for #1837.

Changes:

  • Emit correct property access for static properties (type-qualified access) and correct setter targets for both static/instance properties.
  • Skip indexer properties during model building so the registry doesn’t emit invalid “this[...]” access.
  • For non-readable properties, emit a throwing Get delegate instead of referencing a missing/inaccessible getter, and extend TestPropertyModel with IsStatic + HasGettableValue to support this.
Show a summary per file
File Description
test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTestReflectionMetadataGeneratorTests.cs Adds unit tests covering static property access, indexer skipping, and set-only property getter behavior.
src/Analyzers/MSTest.AotReflection.SourceGeneration/Model/TestClassModel.cs Extends TestPropertyModel with IsStatic and HasGettableValue so codegen can decide correct access patterns.
src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/TestClassModelBuilder.cs Filters out indexers and computes IsStatic/HasGettableValue for properties during model creation.
src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/MetadataRegistryEmitter.cs Adjusts property Get/Set emission to handle static properties and set-only properties safely.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

@Evangelink

This comment has been minimized.

…ring (#9078)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink Evangelink merged commit 0d677c5 into main Jun 12, 2026
12 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/aot-srcgen-property-lowering branch June 12, 2026 13:45
@Evangelink

Copy link
Copy Markdown
Member Author

🧪 Test quality grade — PR #9077

4 new test methods graded across 1 file. 3 earn an A and 1 earns a B. Assertion variety and naming are strong throughout; the single B is caused solely by a slightly long body driven by the multi-step Roslyn driver setup, not by any weakness in what is verified.

ΔTestGradeBandNotes
new MSTestReflectionMetadataGeneratorTests.
WiringGenerator_
EmitsModuleInitializer_
RegisteringAssembly_
AndCompilesAgainstHook
B 80–89 Thorough assertions (emitted tokens + compilation check); body is ~40 lines due to multi-step Roslyn driver setup — consider extracting driver wiring into a helper.
new MSTestReflectionMetadataGeneratorTests.
Generator_
EmitsStaticPropertyAccess_
WithoutInstanceCast
A 90–100 Clear AAA; positive + negative containment assertions verify exact static-access syntax and absence of the CS0176-prone instance cast.
new MSTestReflectionMetadataGeneratorTests.
Generator_
EmitsThrowingGetter_
ForSetOnlyProperty
A 90–100 No issues found.
new MSTestReflectionMetadataGeneratorTests.
Generator_
SkipsIndexerProperty
A 90–100 No issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 228.1 AIC · ⌖ 12.8 AIC · [◷]( · )

Sign up for free to 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.

2 participants