Require all parameters on hidden model factory back-compat overloads

[Copilot]

#11703 made model factory compatibility overloads preserve their published parameter optionality. The latest regen shows the result is inverted: the [EditorBrowsable(Never)] overload ended up fully optional while the newly generated overload became all-required. This flips it back — the hidden overload requires every parameter, and the generated overload keeps the optionality of the current model shape.

// Before
public static CompatibilityModel CompatibilityModel(string id, string description, string name) { }

[EditorBrowsable(EditorBrowsableState.Never)]
public static CompatibilityModel CompatibilityModel(string id, int? count = default) { }

// After
public static CompatibilityModel CompatibilityModel(string id = default, string description = default, string name = default) { }

[EditorBrowsable(EditorBrowsableState.Never)]
public static CompatibilityModel CompatibilityModel(string id, int? count) { }

An overload that receives an argument for every parameter is preferred over one that substitutes a default, so an all-required hidden overload serves exactly the published call sites it was compiled against and is unreachable by any shorter call — which is what lets the visible overload keep its defaults.

Changes

• MethodSignatureHelper
  • BuildBackCompatMethodSignature skips the minimum-prefix computation when hideMethod is true and requires every parameter instead. The visible (reorder-replacement) path still computes the minimum required prefix.
  • New AreAmbiguous(signature, other): walks the shared parameter-type prefix and reports the argument counts where both overloads are applicable and in the same default-substitution state, i.e. where overload resolution has no tie-breaker. Signatures with ref/out/params are skipped.
• ModelFactoryProvider
  • Newly generated overloads are no longer constrained against published signatures that are restored as compatibility overloads; those overloads either require every parameter or replace the generated one outright.
  • TryBuildCompatibleMethodForPreviousContract builds the signature once and rejects it when AreAmbiguous matches a custom overload (logged under ModelFactoryMethodSkipped). A rejected visible replacement falls back to the hidden all-required overload.
• Tests — flipped the assertions and baselines that encoded the inverted behavior, renamed the tests whose names now describe the wrong rule (with their TestData directories), and added coverage for the ambiguity guard end-to-end plus unit tests for AreAmbiguous.

Note the trade-off this restores: a published call that omitted a trailing optional argument no longer binds to the compatibility overload. That is the pre-#11703 behavior and supersedes #11667.

• Fixes Fix Back Compat Model Factory Methods to Have All Required Properties #11831

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@11832

commit: e3e2841

[Copilot]

🟡 Changes recommended

The ambiguity detector misses null-literal and named-argument ambiguity, allowing conflicting overloads to remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates C# model-factory back compatibility so hidden overloads require all parameters while current overloads retain their optionality.

Changes:

• Requires every parameter on hidden compatibility overloads.
• Adds custom-overload ambiguity detection and fallback behavior.
• Updates unit tests and generated baselines.

Paths are relative to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/.

File summaries

File	Description
src/Shared/MethodSignatureHelper.cs	Adds required-parameter and ambiguity logic.
src/Providers/ModelFactoryProvider.cs	Applies ambiguity fallback during compatibility generation.
test/Shared/MethodSignatureHelperTests.cs	Tests signature and ambiguity behavior.
test/Providers/ModelFactories/ModelFactoryProviderTests.cs	Updates and expands provider tests.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_UnknownDiscriminatorReturnTypeOverloadIsGenerated.cs	Updates hidden-overload baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedPartiallyOptionalParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs	Adds previous-contract fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedPartiallyOptionalParametersRequireAllParameters.cs	Updates optionality baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedOverloadKeptWhenStillInLastContract.cs	Updates compatibility baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters(Last)/SampleNamespaceModelFactory.cs	Adds fully optional fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ReorderedFullyOptionalParametersRequireAllParameters.cs	Updates required compatibility baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_PositionalPrefixOverloadRequiresAllParameters.cs	Updates positional-prefix baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_NewOverloadKeepsOptionalityAgainstCompatibilityOverload(Last)/SampleNamespaceModelFactory.cs	Adds previous-overload fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireAllParameters(Last)/SampleNamespaceModelFactory.cs	Adds multiple-overload fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_MultiplePreviousOverloadsRequireAllParameters.cs	Updates multiple-overload baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_LongPositionalPrefixOverloadKeepsGeneratedOptionality(Last)/SampleNamespaceModelFactory.cs	Adds long-prefix fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_LongPositionalPrefixOverloadKeepsGeneratedOptionality.cs	Updates generated optionality baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsCoexistWithRequiredCompatibilityOverload(Last)/SampleNamespaceModelFactory.cs	Adds published custom fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsCoexistWithRequiredCompatibilityOverload(Custom)/SampleNamespaceModelFactory.cs	Adds current customization fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadsCoexistWithRequiredCompatibilityOverload.cs	Adds custom coexistence baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CustomOverloadConstrainsNewlyGeneratedOverload.cs	Updates custom constraint baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsRequireAllParametersReversed(Last)/SampleNamespaceModelFactory.cs	Adds reversed-order fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsRequireAllParametersReversed.cs	Updates reversed-order baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsRequireAllParameters(Last)/SampleNamespaceModelFactory.cs	Adds coexisting-overload fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_CoexistingPreviousOverloadsRequireAllParameters.cs	Updates coexistence baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ChangedCustomOptionalityConstrainsGeneratedOverload(Last)/SampleNamespaceModelFactory.cs	Adds prior optionality fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_ChangedCustomOptionalityConstrainsGeneratedOverload(Custom)/SampleNamespaceModelFactory.cs	Adds changed customization fixture.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AmbiguousReorderedOverloadFallsBackToHiddenOverload(Last)/SampleNamespaceModelFactory.cs	Adds ambiguous prior overload.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AmbiguousReorderedOverloadFallsBackToHiddenOverload(Custom)/SampleNamespaceModelFactory.cs	Adds conflicting custom overload.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AmbiguousReorderedOverloadFallsBackToHiddenOverload.cs	Adds ambiguity-fallback baseline.
test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_AbstractReturnTypeOverloadIsGenerated.cs	Updates abstract-return baseline.

Review details

• Files reviewed: 19/30 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Jorge Rangel (jorgerangel-msft)]

Copilot why were so many testdata file renamed with no changes?

[Copilot]

Copilot why were so many testdata file renamed with no changes?

Those were renamed only because the test methods were renamed and the helpers default TestData lookup to the caller member name. In e4ac243e I moved the unchanged TestData back to the original names and made the renamed tests explicitly reference those existing asset names, so the cumulative PR diff no longer contains the no-content renames.

[Jorge Rangel (jorgerangel-msft)]

Those were renamed only because the test methods were renamed and the helpers default TestData lookup to the caller member name. In e4ac243e I moved the unchanged TestData back to the original names and made the renamed tests explicitly reference those existing asset names, so the cumulative PR diff no longer contains the no-content renames.

Copilot in that case, we can revert back to what you had if and only if, the test method names changed

[Copilot]

Those were renamed only because the test methods were renamed and the helpers default TestData lookup to the caller member name. In e4ac243e I moved the unchanged TestData back to the original nam...

Done in 3062a913. Since the test method names did change, I reverted to matching TestData names and removed the explicit old asset-name overrides.

[github-actions]

No changes needing a change description found.

[Jorge Rangel (jorgerangel-msft)]

Regen preview: Azure/azure-sdk-for-net#62661

[Copilot]

🟡 Changes recommended

Generic custom overloads can be falsely classified as ambiguous, causing avoidable source compatibility breaks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 19/30 changed files
• Comments generated: 1
• Review effort level: Balanced