Share class-row type inference between the source generator, analyzer and reflection #7377

Activity

1. thomhurst

   added

   enhancementNew feature or requestNew feature or request

   on Nov 2, 2026
2. github-actions

   added

   P3Low: cleanup, minor enhancement, or rare edge caseLow: cleanup, minor enhancement, or rare edge case

   on Nov 2, 2026
3. github-actions commented on Nov 2, 2026

   

   github-actionsBot

   on Nov 2, 2026 – with GitHub Actions

   Contributor

   More actions

   Related to #7371, which tracks the same drift between source-gen and reflection inference more broadly. Not an exact duplicate: this issue is the narrower refactor of sharing the class-row matching helper.
4. thomhurst

   marked this as blocked on Nov 2, 2026

   • Class-level [Arguments] rows don't infer T through a T[] or List<T> constructor parameter, and the analyzer rejects them with TUnit0001 #7223
5. thomhurst commented on Nov 2, 2026

   

   thomhurst

   on Nov 2, 2026

   Last edited by thomhurst

   OwnerAuthor

   More actions

   Triage update: #7346 has merged and #7223 is closed. All three inference implementations are now on origin/main; removed the completed prerequisite link. Refreshed the body with the review follow-ups and the overlap with #7370. The helper consolidation remains open.
6. thomhurst commented on Nov 2, 2026

   

   thomhurst

   on Nov 2, 2026

   OwnerAuthor

   More actions

   Additional scope from the #7346 review: when the inference is shared, add a both-modes test that pins [Arguments(null)] on a generic class (reflection reads it as one null value and skips one-value rows; the generator infers nothing from it), and hoist the per-recursion inferred.Keys.ToImmutableArray() in the analyzer's Substitute.
7. thomhurst commented on Nov 2, 2026

   

   thomhurst

   on Nov 2, 2026

   OwnerAuthor

   More actions

   From the Claude review of #7346 at 6990710: in the analyzer's Substitute, inferred.Keys.ToImmutableArray() runs on every recursive call. Compute it once and pass it down. This touches the analyzer path only and is low priority. Please fold it into this consolidation.
8. thomhurst

   unmarked this as blocked on Nov 3, 2026

   • Class-level [Arguments] rows don't infer T through a T[] or List<T> constructor parameter, and the analyzer rejects them with TUnit0001 #7223
9. thomhurst commented on Nov 3, 2026

   

   thomhurst

   on Nov 3, 2026

   OwnerAuthor

   More actions

   Review of #7422 (conversation comment 6594272506, concerns 3–4) adds a concrete consolidation case: ClassRowSources caches inference, but CollectConcreteInstantiations and EmitConcreteDataSources still repeatedly resolve attribute-to-entry membership with IsSpecificClassDataSource, FindClassMethodDataSource, AreSameAttribute and GetClassDataSourcePartition. Resolve that mapping once as part of this issue's shared class-row model; retain the closed constructor mapping or report an actionable discovery failure rather than silently selecting a different constructor.

   Preserve the both-mode coverage added in #7422 for interface rows, unsealed rows, covariant arrays and nullable elements. The explicit-instantiation runtime partition also needs declared constructor-based inference (for example C(T[] values)) rather than taking the first runtime value's type.
10. thomhurst commented on Nov 3, 2026

    

    thomhurst

    on Nov 3, 2026

    OwnerAuthor

    More actions

    Additional parity case found while handling #7422 (head 737b5eb): an external source returning IEnumerable with row 3, on C with public constructors C(object unrelated) followed by C(T value), produces a type-inference discovery failure in generated mode but one passing test in reflection mode. Use a parameterless [Test] method and [MethodDataSource(typeof(Rows), nameof(Rows.Integers))] on the class to reproduce. Generated declared-type inference selects the first constructor and cannot infer T; reflection falls back to runtime-value inference after declared inference fails. Expected: both modes make the same fallback decision. An own-class source for the same constructor pair reports an inference failure in both modes, now covered by #7422's shared fixture. Include the external fallback case in this issue's consolidation matrix; no public API decision is needed.
11. thomhurst commented on Nov 3, 2026

    

    thomhurst

    on Nov 3, 2026

    OwnerAuthor

    More actions

    Nonblocking follow-up from #7422, Claude review #7422 (comment) (suggestions 1–3): when consolidating generator class-row inference, extract the class-only pruning and uncovered-source reporting into named steps, and name the replayable external MethodDataSource selection rather than keeping its attribute-form predicate inline. Keep own-class sources excluded when an invalid class cannot be closed. Also give data-source emission an expression-only form instead of trimming a trailing comma in EmitClassConstraintViolationEntries. Current Tests7333 snapshots cover the emitted expression; these are maintainability suggestions, not merge blockers. Reuse issue-7333-class-method-source if #7422 has not landed when working this issue.
12. thomhurst commented on Nov 3, 2026

    

    thomhurst

    on Nov 3, 2026

    OwnerAuthor

    More actions

    Additional nonblocking follow-up from #7422, Claude review #7422 (comment) (points 1, 2 and 4): expose one cached decision per (entry, class source), including role, declared types and selected constructor, so emission does not recompute partition/filter/fallback rules. Compare attributes against known TUnit symbols rather than simple names. Evaluate caching class-level attribute enumeration/inference per type symbol, since ClassRowSources is currently constructed per test method. Preserve all current parity fixtures and measure allocations before choosing the cache lifetime. These extend the existing mapping/consolidation scope rather than blocking the correctness fix.
13. thomhurst commented on Nov 4, 2026

    

    thomhurst

    on Nov 4, 2026

    OwnerAuthor

    More actions

    Additional constructor-row partition reproduction found while validating #7571 (df2cf32 plus optional-row regression changes):

    [GenerateGenericTest(typeof(string), typeof(int))]
    [GenerateGenericTest(typeof(string), typeof(IEnumerable<string>))]
    [Arguments(new[] { "literal" }, 1)]
    [MethodDataSource(nameof(Rows))]
    public class C<T, U>(IEnumerable<T> first, U second = default!)
    {
        public static IEnumerable<TestDataRow<IEnumerable<string>[]>> Rows() =>
        [new([new[] { "omitted" }]), new([new[] { "first" }, new[] { "second" }])];
    
        [Test, GenerateGenericTest(typeof(bool))]
        public void Run<V>() { }
    }

    Generated mode sends the literal integer to the C<string,IEnumerable<string>> constructor and a supplied string[] optional value to C<string,int>, producing invalid casts. Reflection also sends the supplied string-array row to the integer constructor. The nested constructor shape and runtime row partitioning need parity coverage alongside the shared matching work here; fixing omitted direct generic parameters in #7571 does not solve this supplied nested-row partition case. Expected: each supplied row runs only on compatible closed constructor types, with omitted values taking defaults.
14. thomhurst commented on Nov 4, 2026

    

    thomhurst

    on Nov 4, 2026

    OwnerAuthor

    More actions

    PR #7613's nonblocking Claude review identifies another instance of this consolidation work: CompilationExtensions.ClassRowSuppliesNoTypeArguments duplicates the generator's constructor-row rule. At f3758fa, both select a public [TestConstructor] first, count generic Arguments as one value, treat a null argument array as one value, and inspect only the bound constructor prefix. Please include this analyzer helper in the shared Roslyn implementation or cross-implementation parity tests.

    The same review suggests computing HasIndependentlySuppliedClassType once per class type parameter instead of scanning inherited attributes for each parameter/row, consolidating the three nullable-class-parameter patterns, and adding a where T : class / T? regression. These are follow-ups, explicitly nonblocking in the review; the nullable-value fix remains scoped to #7610. The new branch matches System.Nullable<T> only, so nullable reference annotations do not enter it. InferTypeArguments has one caller, updated in the PR.
15. thomhurst commented on Nov 4, 2026

    

    thomhurst

    on Nov 4, 2026

    OwnerAuthor

    More actions

    Nonblocking follow-up from PR #7614, Claude review #7614 (comment): consolidate the generator and reflection predicates ClassRowSuppliesNoTypeArguments, ClassRowHasValueTypeFallback and ContainsTypeParameter, or enforce their agreement with shared table-driven coverage in both discovery modes. Preserve the _7573 nullable and constructor-precedence cases. Also extract the repeated GetAttributesIncludingBaseTypes().Any(...) constructor-source predicate in TestMetadataGenerator so its call sites cannot diverge. These extend the existing inference-consolidation scope; the reviewer explicitly found no blockers for #7614.
16. thomhurst commented on Nov 5, 2026

    

    thomhurst

    on Nov 5, 2026

    OwnerAuthor

    More actions

    Additional follow-up from the ready review of #7614 (#7614 (comment)): alongside the existing helper consolidation, replace per-attribute LINQ chains in ClassRowSuppliesNoTypeArguments and ClassRowHasValueTypeFallback with loops, measuring generator allocations before and after. Keep partial constructor fallback and constraint reporting unchanged. The existing comment 6644379558 covers concerns 1 and 4; this records concern 2. Concern 3 is now covered by an inherited source on an explicitly generated generic class in the _7573 fixture.