fix(mocks): emit 'where T : default' for interface-constrained generic methods - #6458
#6458fix(mocks): emit 'where T : default' for interface-constrained generic methods#6458thomhurst merged 1 commit intomainthomhurst/TUnit:mainfrom fix/6456-mock-default-constraintthomhurst/TUnit:fix/6456-mock-default-constraintCopy head branch name to clipboard
Conversation
…c methods
Explicit interface implementations inherit constraints, but 'T?' in them is
parsed as Nullable<T> unless the type parameter is known to be a reference or
value type, or a 'where T : default' clause is present. The generator only
emitted 'default' for fully unconstrained type parameters, so interfaces like
Kiota's IRequestAdapter ('where ModelType : IParsable') generated mocks that
failed with CS0539/CS0535/CS0453/CS0314.
Gate the clause on ITypeParameterSymbol.IsReferenceType/IsValueType instead,
which Roslyn computes transitively across base-class and type-parameter
constraints.
Fixes #6456
Greptile SummaryThis PR fixes mock generation for nullable generic methods constrained only by interfaces. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (1): Last reviewed commit: "fix(mocks): emit 'where T : default' for..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code review
Summary: Fixes #6456 by broadening the gate for emitting where T : default on explicit interface implementations from "fully unconstrained" (IsUnconstrained()) to "not known to be a reference or value type" (IsReferenceType/IsValueType). This correctly covers interface-only constraints like Kiota's where ModelType : IParsable. The root-cause analysis is sound, and IsReferenceType/IsValueType are computed transitively by Roslyn, so the fix also correctly handles base-class/type-parameter constraint chains, not just literal class/struct keywords. I checked the two existing snapshots that already exercise the default clause path (Interface_FluentUI_Shape_Nullable_Warnings.verified.txt, Interface_With_Unconstrained_Nullable_Generic.verified.txt) — both are fully-unconstrained cases, so they're unaffected by the widened gate. No regressions expected there.
Suggestion — test dependency choice: KiotaMockTests.cs (and the runtime counterpart Issue6456Tests.cs) pull in a real third-party package (Microsoft.Kiota.Abstractions, new PackageVersion + two new PackageReferences) purely to reproduce the interface-constraint shape. That's a departure from this test suite's existing convention: Interface_FluentUI_Shape_Nullable_Warnings in tests/TUnit.Mocks.SourceGenerator.Tests/MockGeneratorTests.cs:1961 deliberately uses a synthetic in-source interface shaped like FluentUI's IDialogService rather than referencing the real FluentUI package — with a comment noting earlier synthetic repros needed iteration to match the real-world shape exactly.
The same approach would work here: a local interface with where ModelType : ISomeMarker reproduces "interface-only constraint on an explicit-impl generic method" without a live NuGet dependency. The bug is fundamentally about Roslyn constraint-resolution semantics, not anything Kiota-specific, so a synthetic repro would be just as effective at pinning the regression while avoiding restore/network dependency and version-drift risk in the source-generator test project.
I'd keep the one small end-to-end runtime test against real Kiota (Issue6456Tests.cs) for exact bug-report fidelity, but convert the source-generator-level compile check (KiotaMockTests.cs) to a synthetic repro consistent with the FluentUI precedent — that drops one of the two new package dependencies while keeping equivalent coverage.
Everything else — the rename (IsUnconstrainedWithNullableUsage → NeedsDefaultConstraintForNullableUsage), the explanatory comment, and the tests' actual assertions — looks solid and low-risk. No CLAUDE.md compliance issues found (this touches TUnit.Mocks.SourceGenerator, not core engine metadata collection, so the dual-mode rule doesn't apply).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f933b06ab
ℹ️ 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".
Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.61.15 to 1.61.38. <details> <summary>Release notes</summary> _Sourced from [TUnit.Core's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.61.38 <!-- Release notes generated using configuration in .github/release.yml at v1.61.38 --> ## What's Changed ### Other Changes * fix: apply 'unmanaged' constraint handling to remaining source generators by @thomhurst in thomhurst/TUnit#6474 * feat: expose data source attributes on TestContext by @thomhurst in thomhurst/TUnit#6477 ### Dependencies * chore(deps): update tunit to 1.61.35 by @thomhurst in thomhurst/TUnit#6475 **Full Changelog**: thomhurst/TUnit@v1.61.35...v1.61.38 ## 1.61.35 <!-- Release notes generated using configuration in .github/release.yml at v1.61.35 --> ## What's Changed ### Other Changes * fix(mocks): omit 'struct' constraint when type parameter has 'unmanaged' constraint by @thomhurst in thomhurst/TUnit#6473 ### Dependencies * chore(deps): bump fast-uri from 3.1.2 to 3.1.4 in /docs by @dependabot[bot] in thomhurst/TUnit#6466 * chore(deps): update tunit to 1.61.29 by @thomhurst in thomhurst/TUnit#6467 * chore(deps): update dependency verify.tool to v0.8.0 by @thomhurst in thomhurst/TUnit#6470 * chore(deps): update dependency docusaurus-plugin-llms to ^0.5.1 by @thomhurst in thomhurst/TUnit#6472 **Full Changelog**: thomhurst/TUnit@v1.61.29...v1.61.35 ## 1.61.29 <!-- Release notes generated using configuration in .github/release.yml at v1.61.29 --> ## What's Changed ### Other Changes * perf: remove EnumerableAsyncProcessor dependency by @thomhurst in thomhurst/TUnit#6465 ### Dependencies * chore(deps): bump shell-quote from 1.8.4 to 1.10.0 in /docs by @dependabot[bot] in thomhurst/TUnit#6462 * chore(deps): bump body-parser from 1.20.5 to 1.20.6 in /docs by @dependabot[bot] in thomhurst/TUnit#6461 * chore(deps): bump webpack-dev-server from 5.2.5 to 5.2.6 in /docs by @dependabot[bot] in thomhurst/TUnit#6460 * chore(deps): update dependency microsoft.kiota.abstractions to v2 by @thomhurst in thomhurst/TUnit#6464 * chore(deps): update tunit to 1.61.23 by @thomhurst in thomhurst/TUnit#6463 **Full Changelog**: thomhurst/TUnit@v1.61.23...v1.61.29 ## 1.61.23 <!-- Release notes generated using configuration in .github/release.yml at v1.61.23 --> ## What's Changed ### Other Changes * fix(mocks): emit 'where T : default' for interface-constrained generic methods by @thomhurst in thomhurst/TUnit#6458 ### Dependencies * chore(deps): update tunit to 1.61.15 by @thomhurst in thomhurst/TUnit#6450 * chore(deps): update actions/checkout action to v7.0.1 by @thomhurst in thomhurst/TUnit#6453 * chore(deps): update verify to 31.26.0 by @thomhurst in thomhurst/TUnit#6454 * chore(deps): update verify to 31.27.0 by @thomhurst in thomhurst/TUnit#6457 * chore(deps): update react to ^19.2.8 by @thomhurst in thomhurst/TUnit#6459 **Full Changelog**: thomhurst/TUnit@v1.61.15...v1.61.23 Commits viewable in [compare view](thomhurst/TUnit@v1.61.15...v1.61.38). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
Fixes #6456 — mocking
Microsoft.Kiota.Abstractions.IRequestAdapterfailed to compile with CS0539/CS0535/CS0453/CS0314.Root cause
Kiota's generic methods carry an interface constraint:
In an explicit interface implementation, constraints are inherited and
T?is parsed asNullable<T>unlessTis known to be a reference/value type or awhere T : defaultclause is present. The generator only emitteddefaultfor fully unconstrained type parameters (IsUnconstrained()gate), so an interface-only constraint likeIParsable— which leavesTneither known-reference nor known-value — got no clause. The compiler then readTask<ModelType?>asTask<Nullable<ModelType>>, the signature no longer matched the interface member, and everything cascaded.Fix
Gate the
defaultclause onITypeParameterSymbol.IsReferenceType/IsValueTypeinstead, which Roslyn computes transitively (base-class constraints, type-parameter constraints). Type parameters constrained only by interfaces/notnull/new()now correctly getwhere T : defaultwhenT?appears in the signature; known-reference (class, base-class) and known-value (struct,unmanaged) parameters still don't.Tests
KiotaMockTests(source-gen): generates a mock for the realMicrosoft.Kiota.Abstractions.IRequestAdapter(new package reference), asserts thewhere ModelType : defaultclause is emitted and none of the four reported error codes are produced.Issue6456Tests(runtime): mocksIRequestAdapter, verifiesSendNoContentAsyncand a configuredSendAsync<TestModel>with anIParsable-constrained model — mirrors the issue repro.TUnit.Mocks.SourceGenerator.Tests(78) andTUnit.Mocks.Tests(1160) pass on net10.0; all TFMs (net472/net8/net9/net10) build.