Preserve standalone date and timestamp names during C# normalization - #11741
Preserve standalone date and timestamp names during C# normalization#11741Jorge Rangel (jorgerangel-msft) with Copilot wants to merge 14 commits into
Conversation
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
|
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. |
There was a problem hiding this comment.
Pull request overview
Preserves standalone C# date/time names while retaining compound-name normalization.
Changes:
- Avoids replacing an entire date/time name with
On. - Updates property, parameter, and utility coverage.
- Updates XML serialization/deserialization expectations.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
CSharpNameExtensions.cs |
Preserves names consisting solely of a recognized suffix. |
CSharpNameExtensionsTests.cs |
Covers standalone Date and Timestamp variants. |
PropertyProviderTests.cs |
Verifies property-name preservation. |
ParameterProviderTests.cs |
Verifies parameter-name preservation. |
XmlSerializationTests.cs |
Updates serialization assertion for Timestamp. |
XmlDeserializationTests.cs |
Updates deserialization assertion for timestamp. |
XmlSerializationHandlesNullableDateTimeOffsetProperty.cs |
Updates generated serialization baseline. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
commit: |
Jorge Rangel (jorgerangel-msft)
left a comment
There was a problem hiding this comment.
Copilot we need to handle another scenario as part of this pull request. The issue is described in Azure/azure-sdk-for-net#62333 (comment). It seems the CodeGenMember is not working as expected on model properties, and possibly, other members. To validate, we should add a unit test to mirror the issue in the comment, and validate that a custom property can replace a generated one whose name is normalized as part of the normalization process.
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Implemented in 5f37608. |
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Implemented in 6d45b4f.
Implemented in 6d45b4f. |
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Jorge Rangel (jorgerangel-msft)
left a comment
There was a problem hiding this comment.
Copilot lets update the pr description and title to reflect all the changes we made
|
Jorge Rangel (@jorgerangel-msft) Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
Copilot update the pr description and title to reflect the changes we've made |
| if (specPropertiesByName.TryGetValue(originalName, out var inputProperty) && !inputProperty.IsExactName) | ||
| { | ||
| customNames.Add(originalName.NormalizeCSharpAcronyms(inputProperty.Type.IsDateTimeInputType())); | ||
| } |
There was a problem hiding this comment.
Copilot validate if this is true, and if so, address the feedback
There was a problem hiding this comment.
Validated — this was reproducible when [CodeGenMember] used the raw spec casing (for example "creationDate"). I addressed it in 1392c25 by resolving OriginalName through identifier casing before normalized matching, and added regression coverage (CustomCodeReplacesDateNormalizedPropertyUsingRawSpecName). I also re-ran the full npm run test:generator suite after npm ci; it passed.
I can’t edit PR metadata directly from this environment, but here’s an updated proposal you can apply: PR title PR description
|
… normalization Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Date naming normalization could rename standalone members such as
TimestamporDateto the ambiguousOn.CreatedAttoCreatedOn.