[thomhurst]

Fixes #7117

Summary

On a generic class, [GenerateGenericTest] on a generic test method produced generated code that did not compile (CS0246 for the method type parameter, or CS0411). This PR fixes that. Source-gen mode now produces the same tests as reflection mode for these shapes.

Root cause

CollectConcreteInstantiations splits each set of type arguments into class arguments and method arguments by position. Some sources supply type arguments for the method only: the method's [GenerateGenericTest], a method [Arguments] row, a typed data source, [MethodDataSource], or IInfersType<T>. Their types went through the same split, so they became the class type arguments and the method stayed open. The generated invoker then called CastHelper.Cast<TMethod>(...) or instance.Run(...) with no type arguments.

The workaround in the issue does not work either. With a class-level [GenerateGenericTest], a method [Arguments] row still closed the class over the row's type and left the method open.

Fix (generator only; TestMetadataGenerator.CollectConcreteInstantiations)

• TryAddInstantiation skips a set of type arguments for a generic method of a generic class unless the set covers both the class and the method.
• For a generic method of a generic class, every class source is paired with every method source, the way reflection mode closes the method on each concrete class:
  • Class-level [GenerateGenericTest] is paired with each method source that infers the method type arguments ([Arguments] row, typed data source, IInfersType<T>, [MethodDataSource]). The cartesian product with method [GenerateGenericTest] is unchanged.
  • Class [Arguments] row is paired with each of those method sources, and with each method [GenerateGenericTest] type.
• Method data sources are processed before [GenerateGenericTest] types, in reflection mode's order. A type that a source infers runs only that source. Any other [GenerateGenericTest] type, and IInfersType<T>, runs every method data source, as on a non-generic class. For example, class row [Arguments(1)] with [GenerateGenericTest(typeof(long))] [Arguments(2)] gives Run<int>(2) and Run<long>(2) in both modes.

Reflection mode already behaved this way, so TUnit.Engine needs no change.

Tests

• New tests/TUnit.Core.SourceGenerator.Tests/Bugs/7117/GenerateGenericTestOnMethodOfGenericClassTests.cs compiles the generated output for the issue's shape and related shapes, and asserts there are no compiler errors. Before the fix it failed with 8 CS0246/CS0411 errors.
• New tests/TUnit.TestProject/Bugs/_7117 classes and tests/TUnit.Engine.Tests/Issue7117Tests.cs check test names and counts in both modes (6 shapes). Both modes produce the same 8 tests and all pass.
• Run on net10.0, all passing:
  • TUnit.Core.SourceGenerator.Tests, full run: 193 passed, 1 skipped, 0 failed, and no snapshot changes.
  • TUnit.Engine.Tests filters Issue7117Tests, Issue7106Tests, Issue7092Tests, Issue7096Tests, Issue7105Tests, Issue7107Tests, Issue7121Tests, GenericConstraintValidationTests and GenericMethodWithDataSourceTests.
  • TUnit.TestProject namespaces Bugs._4431, Bugs._4440, Bugs._7092 and Bugs._7099, in source-gen and reflection modes. The counts match, and the only failure is the intentional ExpectedResult.Failure test in 4431.

[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-10-19T21:26:15.195766Z	5e9976d	PR opened

ℹ️ 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.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ffc5f4e2-5867-4b8f-9b36-843426a3aee7

📥 Commits

Reviewing files that changed from the base of the PR and between 78c9c14 and b1a7aac.

📒 Files selected for processing: 4

• src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs
• tests/TUnit.Core.SourceGenerator.Tests/Bugs/7117/GenerateGenericTestOnMethodOfGenericClassTests.cs
• tests/TUnit.Engine.Tests/Issue7117Tests.cs
• tests/TUnit.TestProject/Bugs/_7117/GenerateGenericTestOnMethodOfGenericClassTests.cs

• 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

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e9976d275

ℹ️ 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: 5/5

[Medium impact] The PR appears safe to merge; both previous findings are addressed.

Summary

Fixes generic test methods on generic classes by pairing class sources with method sources.

• Generic test methods now close on each concrete class type.

Both previous findings are addressed. No new actionable issues were found.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  C["Class type arguments"] --> P["Pair class and method types"]
  M["Method source types"] --> S["Replace class type parameters"]
  S --> P
  P --> V{"Constraints pass?"}
  V -->|Yes| T["Generate closed test calls"]
  V -->|No| F["Report failures for the matching rows"]

Loading

_{Reviews (2) · Last reviewed commit: "fix(sourcegen): close inferred method ty..." · Reviewed by Greptile}

[thomhurst]

Review dispositions as of b1a7aac:

• Greptile P1 "Generated calls keep open types": fixed. Class type parameters in inferred method types are now substituted.
• Greptile P1 "Constraint failures lose source rows": fixed. Failures are reported once per row, for both [MethodDataSource] and IInfersType.
• Codex P1 "typed-source inference": no divergence existed, because the generator never inferred typed sources on a generic class. I removed the dead loop and added a parity test.

The CodeRabbit comment was rate-limited and has no findings. The CI failures on 5e9976d were all cancelled runs, not test failures, so the new push re-runs them.

[github-actions]

Review of #7152: generic method on generic class (#7117)

I found no blocking problems. I reviewed the diff only and did not build it or run any tests. The /code-review skill failed to run, so this is a manual read. The PR description says the generator tests pass, with no snapshot changes.

What I like

• The root cause is clearly identified: method-only type-argument sources were split positionally into class arguments.
• The guard in TryAddInstantiation is a simple invariant: a generic method on a generic class needs class args plus method args.
• Pairing every class source with every method source matches reflection mode, so TUnit.Engine needs no change.
• Coverage is good. A compile-the-output generator test catches the CS0246/CS0411 class of bug, which snapshot tests don't. The engine tests run both modes and check names and counts, including the constraint-violation cases.

Suggestions (non-blocking)

1. The early return in TryAddInstantiation drops entries silently. If a later change adds a new source kind and forgets to build a full-length argument set, the test silently disappears instead of failing to compile. Consider reporting a diagnostic or asserting in debug builds for that branch.
2. CollectConcreteInstantiations is getting heavy. It already has many flags (isClassRow, methodRow, fromGenerateGenericTest, methodViolationRowSource, methodViolationAllMethodDataSources). Two of them are now passed together. Grouping them into a small InstantiationSource record (origin, row source, violation policy) would make the pairing logic easier to read and extend. It would also make it harder to pass inconsistent combinations.
3. The typed-data-source behaviour is deliberate but surprising. Typed data sources (DataSourceGeneratorAttribute<T>) don't infer method type args on generic classes. This matches reflection mode, and the tests cover it. A short note in the docs would help users, because Run<long>(42) rather than Run<int> is not obvious.
4. The engine test relies on exact test-name strings such as Run<bool> twice. This is fine today, but it will break if display-name formatting changes. Asserting on counts plus generic arguments would be more robust.

Overall this is a solid, well-tested fix.

🤖 Generated with Claude Code

[thomhurst]

Dispositions for the latest claude-review comment (on b1a7aac):

1. Early return in TryAddInstantiation: no change. The return is not a lossy path for the inputs this PR handles: a method-only or class-only source on a generic method of a generic class is deliberately skipped there and then paired with every class source in the IsGenericType: true, IsGenericMethod: true branch, which builds full-length argument sets through CloseOverClass. A debug assertion would fire on those legitimate partial sources. Unresolvable instantiations being dropped without a diagnostic is the generator's existing behaviour (concreteInstantiations.Count == 0 just returns in GenerateTestMetadata), and Source-gen silently drops a generic test method when its type arguments cannot be inferred from the data source #7163 already tracks adding a diagnostic or a failing placeholder for it.
2. Group the TryAddInstantiation flags into a record: not in this PR. It is a refactor of a long-standing local function with no behaviour change, and this PR is a targeted fix. Happy to do it separately if the owner wants it.
3. Typed data sources on generic classes: I opened Typed data sources do not infer method type arguments when the test class is generic #7164 rather than documenting it here. The behaviour predates this PR and matches between modes, but the difference between generic and non-generic classes looks like a gap worth fixing, with docs as the fallback.
4. Exact test-name assertions in Issue7117Tests: no change. The display name is the only place the engine-test results expose the concrete method type argument and the row values, which are exactly what Source generator: [GenerateGenericTest] on a method of a generic class fails to compile (CS0246) #7117 got wrong (for example Run<int>(2) against Run<long>(2)). Counts alone would not catch a wrong instantiation. Many existing engine tests assert display names the same way.

No code changes, so no new commit.