You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Exclude indexers from generated class property metadata - #7560
Public indexers on test classes produced invalid property getters, and overloaded indexers could make metadata initialization ambiguous. Exclude indexers from both generated property-metadata enumeration paths while retaining ordinary properties.
Add a regression snapshot and executable fixtures covering non-generic classes, generic classes with overloaded indexers, and non-generic overloaded indexers. Reflection mode already omits class properties; its broader metadata gap remains tracked in #7522.
No actionable comments were generated in the recent review.
Walkthrough
The source generator now excludes indexers from both property-metadata enumeration paths. Regression fixtures cover single, generic, and overloaded indexers, and verify that ordinary properties remain in generated metadata.
Changes: Indexer Metadata
Layer / File(s)
Summary
Exclude indexers from metadata src/TUnit.Core.SourceGenerator/Utilities/MetadataGenerationHelper.cs
Both property-metadata enumeration paths now filter out indexers.
Fixtures cover single, generic, and overloaded indexers. The generator test and verified snapshot confirm that indexers are omitted and ordinary properties remain in generated metadata.
Issue 7553 describes generated property metadata that includes indexers. This change addresses the source-generator behavior, but does not change reflection-mode metadata.
Possibly related PRs
thomhurst/TUnit#7550: It adds reflection class-property metadata, while this change filters indexers from source-generated property metadata.
Merge Risk: Minimal · up to 1c338
The change addresses indexer metadata generation with targeted regression coverage and has no identified merge-blocking risk.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: excluding indexers from generated class property metadata.
Linked Issues check
The PR meets the coding requirements in [#7531]. MetadataGenerationHelper.cs now filters !p.IsIndexer in both property-metadata enumeration paths. The filter preserves public ordinary properties a…
Out of Scope Changes check
The changes stay within [#7531]. The generator change implements the required indexer filter. The snapshot test and _7531 fixtures provide the required regression and execution coverage. No unrelate…
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check
Explanation
The PR meets the coding requirements in [#7531]. MetadataGenerationHelper.cs now filters !p.IsIndexer in both property-metadata enumeration paths. The filter preserves public ordinary properties and existing exclusions. The new snapshot test checks that this[] and get_Item are absent and that Label and Value remain. The _7531 fixtures cover non-generic, generic, and overloaded indexers, with executable tests in source-generated and reflection modes as reported.
Full details: Out of Scope Changes check
Explanation
The changes stay within [#7531]. The generator change implements the required indexer filter. The snapshot test and _7531 fixtures provide the required regression and execution coverage. No unrelated production behavior or unrelated issue work is shown. The PR correctly leaves the separate reflection metadata gap in #7522 unchanged.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 3 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the metadata row, Indexers leave; plain properties show. Generic keys and overloads too, Snapshots keep the expected view. The rabbit hops where tests pass through.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: edee2c2a-1bbf-4ef8-973f-139607b64769
Commits
Reviewing files that changed from the base of the PR and between be37704 and 1c33838.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.
Review: exclude indexers from generated property metadata
The change is correct and well scoped. It adds !p.IsIndexer to both property enumeration paths in MetadataGenerationHelper (WritePropertyMetadataArray and GeneratePropertyMetadataArray). That is the right fix, and it matches what PropertyInjectionSourceGenerator already does at line 308.
Strengths
The regression covers a non-generic class, a generic class with overloaded indexers, and a non-generic class with overloaded indexers.
The snapshot test also asserts that the output has no this[] or get_Item, and that the ordinary properties are kept.
The EngineTest fixtures run in both source-generated and reflection modes.
Minor suggestions (non-blocking)
The same filter, Accessibility.Public && !IsIndexer && Name != "EqualityContract", now appears twice. A small shared helper, for example GetMetadataProperties(typeSymbol), would stop the two paths drifting apart again. That drift is how this bug appeared.
Other IPropertySymbol enumerations (TestMetadataGenerator.cs:4178, RequiredPropertyHelper) look unaffected. The first only checks for data-source attributes, which indexers cannot meaningfully carry. I did not trace RequiredPropertyHelper, but indexers cannot be required.
I did not build or run the tests. No prior review threads need addressing; the only earlier comment is the CodeRabbit draft notice. LGTM.
Reviewed all review and conversation surfaces for 1c33838: Claude and Codex both report no blockers, Greptile reports safe to merge, and there are no inline threads.
Disposition of Claude's optional helper extraction: retain the two existing enumeration paths for this focused fix. Both now apply the same indexer exclusion, and the regression snapshot covers both generic and non-generic paths. Extracting a helper is not needed for correctness here.
Marking ready for CI. The ready-for-review event also reruns Claude; auto-merge must wait for that review to complete successfully and its comment to be read.
Looks good. The fix is minimal and targeted: !p.IsIndexer is added to both property-metadata enumeration paths in MetadataGenerationHelper.cs (WritePropertyMetadataArray and GeneratePropertyMetadataArray), which keeps them consistent. I checked the other IPropertySymbol enumerations in the generator. They cover data-source and required-property paths, so indexers don't need handling there.
The snapshot covers the non-generic, generic-with-overloads and non-generic-overloaded cases. The extra DoesNotContain("this[]") and get_Item assertions guard against regressions.
The executable fixtures in TUnit.TestProject/Bugs/_7531 use [EngineTest], so both source-generated and reflection modes are exercised.
Non-blocking: if more property-enumeration sites appear later, a shared helper like GetMetadataProperties(typeSymbol) would keep the filter in one place. No changes needed for this PR.
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
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.
GitHub Actions
Inactive
Summary
Public indexers on test classes produced invalid property getters, and overloaded indexers could make metadata initialization ambiguous. Exclude indexers from both generated property-metadata enumeration paths while retaining ordinary properties.
Add a regression snapshot and executable fixtures covering non-generic classes, generic classes with overloaded indexers, and non-generic overloaded indexers. Reflection mode already omits class properties; its broader metadata gap remains tracked in #7522.
Closes #7531
Validation
_7531fixtures on net10.0: 3 passed in source-generated mode and 3 passed with--reflection.this[].Summary by CodeRabbit
Bug Fixes
Tests