Repository navigation
fix(generic): build generic class instances from class data, never from method rows - #7192
Merged
thomhurst merged 3 commits intoOct 20, 2026
Conversation
…om method rows A method-level [Arguments] row that infers a generic class's type arguments, where no class-level [Arguments] row infers them, gave the source generator's instantiation the method row's values as constructor arguments. Reflection mode built the instance from the class-level data instead. The generator now takes the constructor arguments of such an instantiation from every class-level data source, as for a [GenerateGenericTest] instantiation, converting them to the parameter types as reflection mode does. Reflection mode now scopes method rows to the instantiation they infer for any class-level data source, not only class-level [Arguments] rows, so a row inferring types a class-level [MethodDataSource] does not yield runs on its own instantiation, as in the generator. Fixes #7161
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review: 1
Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0489dfd0af
ℹ️ 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".
Contributor
 [Medium impact] The PR appears safe to merge; the previous array-construction issue is fixed. SummaryGeneric test classes now take constructor values from class data, not method rows.
Reviews (3) · Last reviewed commit: "fix(sourcegen): build a generic class fr..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…d class rows Reflection mode detected class-level [Arguments] rows with IsDefined(typeof(ArgumentsAttribute)), which misses the generic [Arguments<T>] form. The source generator matches both forms by name. Reflection now checks the class data sources for either form. The source generator also never inferred class type arguments from a class-level [Arguments<T>(value)] row, so every instantiation of such a class failed for lack of a class row. It now reads the value as a one-value class row, as reflection mode does. When a method row's instantiation takes its constructor arguments from every class data source, reflection mode now reuses the rows discovery already read, instead of evaluating the class data sources again in TestBuilder.
…row's array, not its elements
The constructor-argument shortcut read any array first argument as the values of a params
[Arguments] row, so a class-level [Arguments<int[]>(new[] { 1, 2 })] row constructed the class with
1 and 2 instead of the array. It now applies only to the non-generic [Arguments], and the generic
row reaches the constructor through the test's class arguments, as other [Arguments<T>] rows do.
This was referenced Oct 20, 2026
Contributor
Review of #7192I read the diff and did not run the tests. The skill-based review run failed, so this was done by hand. Verdict: looks good, no blocking issues.
Non-blocking suggestions
The PR is stacked on #7149, so merge order matters. |
thomhurst
merged commit Oct 20, 2026
c4ee3b6
into
fix/issue-7118-generic-row-pairing
6 checks passed
thomhurst
added a commit
that referenced
this pull request
Oct 20, 2026
…they infer (#7149) * fix(engine): run method rows only on the generic class instantiation they infer On a generic class with class-level [Arguments] rows, reflection mode ran every method-level [Arguments] row on every class instantiation. A method Test(T arg) with class rows (1) and ("a") and method rows (1) and ("a") therefore produced 4 tests in reflection mode, one failing on argument conversion, while source generation produced 2. Reflection mode now scopes method rows the way the generator does. A row that infers the class's type arguments runs only on that instantiation. Rows that infer none, such as [Arguments(null)] or rows of other data sources, run on every instantiation. A method row whose inferred types match no class row also differed between the modes. Reflection mode ran it against the other class rows and failed on conversion. The generator built an instance by passing the method row's own values to the constructor. Both modes now report such a row as a failed test that explains that no class row supplies its constructor arguments. The issue's own repro, where the method has its own type parameter, already gave the same 4 passing tests in both modes on main. It is kept as a regression test. Fixes #7118 * fix(engine): align generic row scoping edge cases across both modes - Skip an instantiation whose method rows all infer other class types, instead of running the method once without arguments (both modes). - Exempt only the types a class-level [GenerateGenericTest] lists from the missing-class-row failure, not every combination; reflection now reports that failure alongside [GenerateGenericTest] too. - Let a class row's instantiation stand for a [GenerateGenericTest] of the same types in reflection mode, and scope method rows on a [GenerateGenericTest] instantiation that they infer, as the generator does. - Apply the reflection scoping only with class-level [Arguments] rows, the condition the generator uses. - Avoid the .NET 8 trim analyzer crash (AD0001) on an array write through `??=`, which broke the Windows build. - Return the original data-source array when no row is filtered out. * refactor(engine): name the class row group type and assert method row alignment Replace the (Type[] TypeArguments, IDataSourceAttribute[] ClassDataSources, int RowCount) tuple, now spelled out in three signatures, with a ClassRowGroup record struct. In ScopeMethodRowsToClassTypeArguments, add a Debug.Assert so that a length mismatch between the two data source lists fails Debug builds instead of only falling back to keeping every source. * fix(sourcegen): let a matching class-level data source supply an instantiation that only a method row infers When only method [Arguments] rows inferred an instantiation's class type arguments, the generator marked it as having no class row, even if another class-level data source could supply the constructor arguments. Reflection groups those sources' rows by the types of their values and ran the test on them, so the two modes disagreed. A class-level typed source whose declared types infer the instantiation's class type arguments is now added as its class row. A typed source of another type does not count. Any other class data source is kept, because the generator cannot see its row types. * fix(generic): build generic class instances from class data, never from method rows (#7192) * fix(generic): build generic class instances from class data, never from method rows A method-level [Arguments] row that infers a generic class's type arguments, where no class-level [Arguments] row infers them, gave the source generator's instantiation the method row's values as constructor arguments. Reflection mode built the instance from the class-level data instead. The generator now takes the constructor arguments of such an instantiation from every class-level data source, as for a [GenerateGenericTest] instantiation, converting them to the parameter types as reflection mode does. Reflection mode now scopes method rows to the instantiation they infer for any class-level data source, not only class-level [Arguments] rows, so a row inferring types a class-level [MethodDataSource] does not yield runs on its own instantiation, as in the generator. Fixes #7161 * fix(generic): treat [Arguments<T>] as a class row and reuse discovered class rows Reflection mode detected class-level [Arguments] rows with IsDefined(typeof(ArgumentsAttribute)), which misses the generic [Arguments<T>] form. The source generator matches both forms by name. Reflection now checks the class data sources for either form. The source generator also never inferred class type arguments from a class-level [Arguments<T>(value)] row, so every instantiation of such a class failed for lack of a class row. It now reads the value as a one-value class row, as reflection mode does. When a method row's instantiation takes its constructor arguments from every class data source, reflection mode now reuses the rows discovery already read, instead of evaluating the class data sources again in TestBuilder. * fix(sourcegen): build a generic class from an [Arguments<T[]>] class row's array, not its elements The constructor-argument shortcut read any array first argument as the values of a params [Arguments] row, so a class-level [Arguments<int[]>(new[] { 1, 2 })] row constructed the class with 1 and 2 instead of the array. It now applies only to the non-generic [Arguments], and the generic row reaches the constructor through the test's class arguments, as other [Arguments<T>] rows do. * fix(generic): keep typed null class rows and uninferred method rows on typed-source instantiations - [Arguments<T>(null)] class row infers its declared T in both modes, so method rows of T run on it instead of failing for want of a class row. - Source-gen: when a typed class data source joins an instantiation that only method rows inferred, method rows that infer no type (e.g. [Arguments(null)]) join it too, as reflection runs them there. - Source-gen: with class-level [Arguments] rows, drop instantiations that only a method data source's declared return type inferred (no row, no [GenerateGenericTest], no matching class source); reflection has none. * fix(generic): map a typed null class row's declared type through the constructor parameter [Arguments<string[]>(null)] on C<T>(T[] values) made reflection build C<string[]> from the declared type, while source generation inferred no class from the row. Both modes now match the declared type against the constructor's first parameter (T, T[], or a constructed generic such as List<T>) and infer T = string. Regression fixture ArrayParameterNullTypedClassRow runs in both modes via Issue7118Tests; the analyzer gap (TUnit0001) and non-null rows are #7223.
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.
Fixes #7161
Summary
On a generic class, a method-level
[Arguments]row can infer the class's type arguments when no class-level[Arguments]row infers them. That happens when the class types come from a class-level[MethodDataSource]or from[GenerateGenericTest]. In those cases, both modes now build the instance from the class-level data. Neither mode passes the method row's values to the constructor any more.[MethodDataSource]yielding1; method rows(1),("a")C<string>built from"a", class data ignoredTest(a)runs onC<int>and fails:String cannot be converted to Int32C<int>(1)andC<string>("1")[Arguments(1)]+[GenerateGenericTest(typeof(string))]; method rows(1),("a")D<string>.Test(a)passes, built from"a"D<string>built from class row1→"1"D<string>built from class row1→"1", the[GenerateGenericTest]semantics, in both modesCause
EmitConcreteInstanceCreation): it bakedSpecificArgumentsAttribute's values into the constructor even when that attribute was a method row.EmitConcreteDataSourcesalso gave such an instantiation no class data sources, so the class-level[MethodDataSource]/[Arguments]rows never reached it.ReflectionTestDataCollector): fix(engine): run method rows only on the generic class instantiation they infer #7149 scopes method rows to the instantiation they infer only when the class has class-level[Arguments]rows. With any other class data source, every method row ran on every instantiation the data source yielded.Fix
[GenerateGenericTest]instantiation does. The exception is Generic classes with differently typed [Arguments] rows produce different test sets in source-gen and reflection modes #7118's "no class row" entry, which still fails with its message.CastHelper, as reflection's instance factory does. This reusesInstanceFactoryGenerator.GenerateInstanceFactoryBody. Without the conversion,D<string>with class row1failed withMissingMethodException.InferClassTypeArgumentsOfMethodRowsnow scopes method rows for any class-level data source.BuildTestsForMethodRowsWithoutClassRow, a method row whose inferred types no class row yields still fails with Generic classes with differently typed [Arguments] rows produce different test sets in source-gen and reflection modes #7118's message when the class has class-level[Arguments]rows. Otherwise it runs on its own instantiation with all class data sources, matching the generator, which cannot know the types a run-time source yields.Known limit: a class-level data source can yield rows of several types, such as an untyped
object[]source that yields both1and"x". For a method row("a"), reflection then givesC<string>only the string row, while the generator gives it every row. The generator cannot split such a source at compile time.Tests
tests/TUnit.TestProject/Bugs/_7161/ClassDataForMethodRowInstantiationTests.cscovers both shapes from the issue. Each asserts that the instance is built from the class data ("5"). Before the fix it failed in both modes: 3 of 4 failed in source-gen and 1 of 4 in reflection. After the fix, 4 of 4 pass in both modes.Issue7161TestsinTUnit.Engine.Tests, plus snapshot testTests7161.Tests7118.Test.verified.txtchanged in one place.GenerateGenericTestOfUninferredType<object>now uses a typed constructor call withCastHelper.Cast<object>instead ofActivator.CreateInstance(..., args). Behaviour is the same.TUnit.Core.SourceGenerator.Tests: passed on net10.0, net8.0 and net472.TUnit.Engine.Tests, Reflection leg:Issue7161Tests,Issue7118Tests,Issue7106Tests,Issue7107Tests,Issue7096Tests,Issue7105Tests,Issue7121Tests,Issue7123Tests,Issue7092Tests,GenericConstraintValidationTestsandGenericMethodWithDataSourceTestsall passed. The AOT leg is skipped locally.TUnit.TestProjectdirectly, with and without--reflection, and compared totals and failures. These are identical in both modes:Bugs._7107,_7096,_7118,_7161,_7092,_7105,_7121and_7123;GenericTests,GenericMethodTests,GenericTypeTests,TestMixedGenericParameters,SmartInferenceTests,ClassAndMethodArgumentsTests,GenericInstanceMethodDataSourceTestsandGenericConstraintValidationTests.Bugs._7106gives 57 tests in source-gen and 65 in reflection, the same as on fix(engine): run method rows only on the generic class instantiation they infer #7149. The extra tests are reflection-only classes.Review follow-ups (9149a7b)
[Arguments<T>]form as a class-level[Arguments]row, as the generator does.[Arguments<T>(value)]row. Before, it inferred none, and every instantiation of such a class failed. New test:Bugs/_7161/GenericArgumentsClassRowWithUnmatchedMethodRow.TUnit.Core.SourceGenerator.Testson net10.0, net9.0, net8.0 and net472. Also theBugs._7096/_7105/_7106/_7107/_7118/_7121/_7123/_7161/_7092and generic suites in both modes, with identical results, andIssue7161Tests/Issue7118Tests.