[thomhurst]

Summary

Source-gen mode now picks the same [MethodDataSource] overload as reflection mode when overloads are spread across a class hierarchy.

• ResolveMethodDataSourceMember scores methods from the target type and every base type, the same set reflection's GetMethods(FlattenHierarchy) sees. It skips private base members and base methods that a more derived type overrides or hides by signature. It also skips base methods the generated class cannot call (for example protected ones), so choosing one cannot break the build. On an exact tie the most derived declaration wins, as with the reflection binder. The property/field fallback still reads the first type that declares the name.
• The generated factory calls the method through its declaring type (BaseProvider.Split(...), or ((BaseProvider)instance).Split(...)), but only when a type below the declaring type also declares that name without overriding it. Without the cast, C# overload resolution on the target type drops the base overload. The common case keeps its current output, so no snapshot changes.
• An override is treated as declared where its virtual method is. A derived override Overridden(string, int) next to a derived Overridden(string, long) was already a pre-existing divergence: C# does not count overrides as overload candidates on the derived type, so for 1, source-gen called the long overload. The new rows exposed this case, and it is fixed here too.
• When an override is called through its base declaration, the call is bound to that declaration at compile time. Two things are therefore restored from the override, as reflection invokes it:
  • The override's optional-parameter defaults are passed explicitly, positionally. Otherwise C# would supply the base's defaults, and the base's parameter names would apply.
  • A covariant return type is cast back, so a Task<...> override of a base object method still binds to FromTask<T>.

Tests

DataSourceOverloadParityTests gets six new rows:

• a better static overload on the base
• the same for an instance method
• a private base overload (invisible in both modes)
• an overridden virtual next to a derived overload
• an override that renames and re-defaults an optional parameter
• a covariant Task<> override

Issue7108Tests now expects 31 rows.

• DataSourceOverloadParityTests: 31/31 pass in source-gen and reflection modes (net10.0). Before the fix, the default row failed in source-gen (int = 1 instead of 2), and the covariant row failed to compile (CS0411).
• TUnit.Core.SourceGenerator.Tests: all pass on net10.0 and net472. No snapshot changes.
• TUnit.Engine.Tests Issue7108Tests (net10.0): source-gen and reflection variants pass. The AOT variants run only in CI.

The earlier net472 AotConverterScanTests polyfill is dropped from this PR: main fixed that in #7330.

Closes #7202

[coderabbitai]

Caution

Review failed

The pull request is closed.

• 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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-01T17:40:52.146612Z	29819df	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Review

Solid fix. Scoring candidates across the whole hierarchy matches reflection's GetMethods(FlattenHierarchy). Emitting the call through the declaring type only when a lower type shadows the name keeps existing snapshots unchanged. Treating an override as declared where its virtual method is also fixes the pre-existing override(int) vs long divergence. The net472 IsExternalInit polyfill fix is correct.

Non-blocking observations:

1. IsHiddenBySignature and generic methods. For generic methods, parameter types that are method type parameters (T) belong to different symbols on each method, so SymbolEqualityComparer reports them as unequal. A derived Foo<T>(T x) would not hide a base Foo<T>(T x), and both would stay candidates. Reflection hides the base one. A call through the base type would still compile, so this is unlikely to break the build. It could pick the wrong overload on a tie. A small test with a generic hidden method, or comparing via OriginalDefinition plus type-parameter ordinals, would settle it.

2. IsShadowedOnTargetType is conservative. It returns true for any non-override member of the name on an intermediate type, including a property or field. The cast it then emits is harmless and semantically correct. The doc comment says "member", so this is only a clarity note.

3. Accessibility. IsSymbolAccessibleWithin(method, compilation.Assembly) also requires the declaring type to be accessible, which the (({receiverType})instance) cast needs. Consider one test comment or case for an internal base type in another assembly with InternalsVisibleTo. That case is not covered.

4. Scope. The method now interleaves collection, filtering and scoring. Extracting a CollectMethodDataSourceCandidates helper that returns the visible, de-hidden candidates would let scoring and the property/field fallback each be tested on their own. This is optional.

The test matrix (static, instance, private base, overridden virtual) covers the main paths in both modes. LGTM, pending CI.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df983f2589

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change affects source-generated data-source selection and invocation.

The override-default fix should preserve date defaults before merging.

Summary

The PR searches base types when choosing a MethodDataSource overload and calls the chosen method through the type needed to reach it.

• Source-generated data sources choose matching overloads across a provider’s class hierarchy.

_{Reviews (2) · Last reviewed commit: "fix(sourcegen): keep an override's defau..." · Reviewed by Greptile}

[thomhurst]

Disposition of the non-blocking notes in the Claude review:

1. Generic methods in IsHiddenBySignature. No change. ScoreDataSourceOverload returns -1 for any generic method, so a generic overload never becomes bestMethod. A generic method is used only as the fallback firstMethod, and that fallback is limited to the first declaring type. A base generic method left as a candidate therefore cannot be chosen.
2. IsShadowedOnTargetType is conservative. No change. A property or field of the same name on an intermediate type also gets the cast, which is harmless and correct, as the review says.
3. Internal base type through InternalsVisibleTo. No change. IsSymbolAccessibleWithin(method, compilation.Assembly) checks the declaring type's accessibility too, so the ((Base)instance) cast is emitted only when the type is nameable.
4. Extracting a candidate-collection helper. Not done in this PR, to keep the diff focused.

The Codex (optional defaults) and Greptile (covariant return type) findings are fixed in 29819df and replied to in their threads.

[github-actions]

Review

The approach is sound. ResolveMethodDataSourceMember now scores the whole hierarchy, matching reflection's GetMethods(FlattenHierarchy). It skips private, inaccessible and signature-hidden base methods. Calls go through the declaring type only when a lower type shadows the name, so existing generator snapshots are unchanged. The override handling is careful: it uses the base's parameter names, passes the override's defaults explicitly, and casts back for covariant returns.

Notes:

• Stale PR description: it says the parity test has 29 rows and adds four new ones. The diff adds six rows (including Covariant and OverriddenDefault), and Issue7108Tests expects 31. Please update the description so the numbers match.
• Signature hiding is approximate: IsHiddenBySignature compares parameter types with SymbolEqualityComparer and ignores generic method type parameters. Two generic overloads that differ only in their constraints, or that use T in different positions, could be treated as hidden or not hidden incorrectly. This is an edge case. A short comment saying the check is deliberately conservative would help.
• Maintainability: EmitDataSourceFactoryInvocation now carries the shadowing, override-binding and covariant-cast logic inline. Pulling it into a small helper that returns a (receiverType, boundMethod, castType) record would make it easier to test and read.
• Unrelated fix: the IsExternalInit polyfill for AotConverterScanTests on net472 is fine. The description mentions it, but a separate PR would have kept this one focused.

No blocking issues.

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 29819df822

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Comments Outside Diff

These findings could not be posted inline.

•  Overrides receive the wrong date src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs:7355 ▶

  When TryFormatDefaultValue cannot write an override’s default, this path still leaves the argument out of the base-typed call. For example, give a virtual method an [Optional, DateTimeConstant(0)] DateTime parameter, change the override’s default to DateTimeConstant(1), and add a same-named sibling overload. With an earlier null argument and the date omitted, C# supplies the base’s date while reflection supplies the override’s date. The provider can then return different test rows in each mode.

  Emit the override’s date explicitly, or keep a call path that preserves its default.