Generate back-compat overload when a service method gains a new optional non-body parameter#10532
Generate back-compat overload when a service method gains a new optional non-body parameter#10532
Conversation
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/bb773994-13fe-4a5a-971a-b594a6d82388 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
commit: |
| } | ||
|
|
||
| var currentMethodSignatures = BuildCurrentMethodSignatures(originalMethods); | ||
| var materializedMethods = originalMethods as IList<MethodProvider> ?? [.. originalMethods]; |
|
|
||
| return [.. originalMethods]; | ||
| // Add back-compat overloads for methods that have gained one or more new optional non-body parameter(s) | ||
| // relative to the last contract. See https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter |
| /// For each public/protected method on the last contract that does not have an exact match in the | ||
| /// current contract, attempts to find a corresponding current method whose parameter list is the | ||
| /// previous method's parameter list (in the same order) plus one or more additional optional | ||
| /// non-body parameters. When such a current method is found, a hidden back-compat overload that | ||
| /// matches the previous signature is added; its body simply delegates to the current method, | ||
| /// passing default values for the new parameters. | ||
| /// | ||
| /// Per https://github.com/Azure/azure-sdk-for-net/blob/main/doc/DataPlaneCodeGeneration/ServiceDrivenEvolution.md#a-method-gets-a-new-optional-parameter | ||
| /// this back-compat behavior is intentionally restricted to non-body parameters. | ||
| /// </summary> |
| var existingSignatures = new HashSet<MethodSignature>(MethodSignature.MethodSignatureComparer); | ||
| foreach (var m in methods) | ||
| { | ||
| existingSignatures.Add(m.Signature); | ||
| } | ||
| if (CustomCodeView?.Methods != null) | ||
| { | ||
| foreach (var m in CustomCodeView.Methods) | ||
| { | ||
| existingSignatures.Add(m.Signature); | ||
| } | ||
| } |
There was a problem hiding this comment.
@copilot it seems there is some overlapping logic between this and BuildCurrentMethodSignatures. CAn we consolidate this and only calculate the signatures once to improve perf?
There was a problem hiding this comment.
Consolidated in 81e56f3 — currentMethodSignatures is now built once in BuildMethodsForBackCompatibility and passed into both ApplyParameterReorderingForBackCompat and AddBackCompatOverloadsForNewOptionalParameters. The latter reuses it for the existing-signature check and derives the by-name lookup from its values, so no duplicate signature collection happens.
…dict Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/98e4708e-374c-4348-8b5b-30e4c877bd56 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
| Previous version of the client: | ||
|
|
||
| ```csharp | ||
| public virtual ClientResult<string> GetData(int param2, string param1, CancellationToken cancellationToken = default); |
There was a problem hiding this comment.
@copilot we need to fix this example. The tsp example you have below has a body param. We should fix it to be
op getData(@query p1: int32, @body body: SampleModel, @query p2?: boolean): string;
And the generated methods should accept a BinaryContent param type for the body.
There was a problem hiding this comment.
Fixed in 779d663 — TSP example now uses @body body: SampleModel with @query p1 / @query p2?, and the generated methods accept BinaryContent for the body.
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/eaa4071b-877d-4406-a1ca-63af4e8c07c5 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
| - [Method Parameter Name Preserved from Last Contract](#scenario-method-parameter-name-preserved-from-last-contract) | ||
| - [Content-Type Parameter Ordering](#content-type-parameter-ordering) | ||
| - [Content-Type Before Body Preserved from Last Contract](#scenario-content-type-before-body-preserved-from-last-contract) | ||
| - [Service Method Evolution](#service-method-evolution) |
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/fdf69480-deaf-4b00-8d8a-e88ad22e8313 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
| **Generated Compatibility Methods:** | ||
|
|
||
| ```csharp | ||
| [EditorBrowsable(EditorBrowsableState.Never)] |
There was a problem hiding this comment.
@copilot we should include the newly generated client. Include inline csharp comments explaining what each overload is
There was a problem hiding this comment.
Updated the example in 12265c0 to include the current generated GetData/GetDataAsync methods alongside the back-compat overloads, with inline C# comments explaining each one.
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/df3eba50-228a-4ff8-88e1-756b943c65dc Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Adding an optional parameter to a service method is non-breaking in TypeSpec but breaks generated C# clients (callers binding to the old method signature fail to resolve). The generator now mirrors the Service-Driven Evolution guidance and emits a hidden overload matching the previous contract's signature.
Changes
ClientProvider.BuildMethodsForBackCompatibility— after parameter reordering, scansLastContractView.Methodsfor previous signatures whose parameters are a same-order subset of a current method, where every extra current parameter is optional and non-body. Emits a hidden[EditorBrowsable(Never)]ScmMethodProvideroverload (sameScmMethodKindas the current method) that delegates to the current method, passingdefaultfor each new parameter. Async overloads delegate withoutawaitso the back-compat method itself remains non-async.BackCompatibilityChangeCategory.MethodNewOptionalParameterOverloadAdded— new category surfaced in the emitter end-of-run summary.BackCompatibility_NewOptionalNonBodyParameterAdded,BackCompatibility_MultipleNewOptionalNonBodyParametersAdded,BackCompatibility_NewOptionalBodyParameterDoesNotAddBackCompatOverload,BackCompatibility_NewRequiredParameterDoesNotAddBackCompatOverload.backward-compatibility.md.Example
Previous contract:
Current TypeSpec adds an optional
@header param3?: boolean. Generated overloads:Defaults are stripped from the back-compat signature to avoid ambiguous call sites with the current method.