Repository navigation
Fix C# date-time naming feedback - #11751
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
commit: |
|
No changes needing a change description found. |
There was a problem hiding this comment.
Pull request overview
Updates C# date-time naming normalization and preserves serialization for renamed operation parameters.
Changes:
- Normalize
Start*/End*date-time names toStartsOn/EndsOn. - Preserve exact
First*/Last*names. - Map normalized parameters to original input names during serialization.
- Update naming, provider, constructor, and request serialization tests.
A critical collision remains unresolved: distinct inputs such as startTime and startsOn can produce the same generated name.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Summary |
|---|---|
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/CSharpNameExtensionsTests.cs |
Tests naming normalization. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/PropertyProviderTests.cs |
Tests normalized property names. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ParameterProviderTests.cs |
Tests normalized parameter names. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/ConstructorParameterNormalizesDateTimeSuffix.cs |
Updates generated constructor output. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterPreservesDateTimeSuffix.cs |
Updates compatibility output. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs |
Updates constructor naming expectations. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/CSharpNameExtensions.cs |
Updates date-time naming rules; collision handling remains required. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs |
Tests header, query, and path serialization. |
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs |
Maps original parameter names for serialization. |
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/CSharpNameExtensions.cs:106
- This new mapping changes an existing public
StartTime/EndTimedate-time member from the previously emittedStartOn/EndOntoStartsOn/EndsOn. The model-property back-compat check only recognizes the rawStartTimename (PropertyProvider.cs:106-115), so a normal regeneration whose last contract containsStartOnwill not preserve the shipped property name (and there is no later property-name restoration). Please preserve the last-contract member name or otherwise add an explicit compatibility path before applying this rename.
["End"] = "Ends",
["Expiration"] = "Expires",
["Modification"] = "Modified",
["Start"] = "Starts"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/CSharpNameExtensions.cs:106
- These rewrites can make a valid operation un-generatable. For example, date-time parameter
startTimeand a separate parameterstartsOnnow both produce the requested C# namestartsOn(similarlyendTime/endsOn).RestClientProvider.BuildMessagethen callssignature.Parameters.ToDictionary(p => p.Name, ...), which throws on the duplicate beforeCodeWritercan disambiguate declarations. Please resolve normalized-name collisions when constructing parameters and map serialization by the originatingInputParameterrather than by the colliding string name.
["End"] = "Ends",
["Expiration"] = "Expires",
["Modification"] = "Modified",
["Start"] = "Starts"
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs:1672
- For next-link requests that reinject only one of two raw names differing by case (for example,
startTimebut notStartTime), the reduced map contains only the first exact key, yet this fallback also resolvesStartTimeto that value. Since query/header emission still iterates every operation parameter, both wire parameters are then serialized from the single reinjected argument. Exact case-collision handling therefore remains incorrect for reinjected subsets; preserve knowledge of case-colliding operation names and do not use the insensitive fallback for an absent exact alias (and add a paging regression where only one casing is reinjected).
public bool TryGetValue(string name, [NotNullWhen(true)] out ParameterProvider? parameter)
=> _exact.TryGetValue(name, out parameter) ||
_caseInsensitive.TryGetValue(name, out parameter);
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs:269
- Generated names are inserted before exact input-name aliases, so parameter order still determines correctness. For an operation ordered as
startsOn(string), thenstartTime(date-time), both providers initially use the generated keystartsOn; the second assignment overwrites the first, and the first parameter's laterTryAdd("startsOn", ...)cannot restore its exact alias. Consequently both wire parameters serialize the date-time argument. The current regression uses only the favorable reverse order. Give exactInputParameter.Namealiases precedence over generated-name keys (or store them separately) so lookup is order-independent.
foreach (var parameter in parameters)
{
paramMap[parameter.Name] = parameter;
}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
Input model names (wire name, original name and alias) are now registered before the normalized C# names, which are kept only as fallback aliases. Previously the generated names were written first with overwrite semantics, so a raw input name that matched another parameter's normalized name could not be recovered, causing both wire parameters to serialize the same value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
The reinjected parameter subset must not fall back to case-insensitive lookup: a parameter left out of the subset would otherwise resolve to another parameter whose name differs only by casing, emitting a spurious wire parameter on the next page request. ParameterProviderMap now takes an allowCaseInsensitiveFallback flag that the reinjected map disables, and the next request signature filter matches reinjected names with an ordinal comparer. Page size names remain case-insensitive because the operation and method parameters intentionally differ in casing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813a9e1d-ed7a-4e38-ac7a-2096d4933fa5
Summary
Start*andEnd*names toStartsOnandEndsOnFirst*andLast*date-time names while retaining compound-name normalizationAddresses feedback from Azure/azure-sdk-for-net#62390.
Validation
npm run buildeng/scripts/Generate.ps1npm testnpm run cop