fix(http-client-csharp): support int64 enum/union base types#10648
Open
haiyuazhang wants to merge 1 commit into
Open
fix(http-client-csharp): support int64 enum/union base types#10648haiyuazhang wants to merge 1 commit into
haiyuazhang wants to merge 1 commit into
Conversation
Extend InputEnumTypeValueConverter to handle InputPrimitiveTypeKind.Int64 by reading values via GetInt64() and widening InputEnumTypeIntegerValue's IntegerValue from int to long. Also include the offending kind, enum, and value names in the default-arm JsonException message so unsupported kinds are diagnosable from build output. Fixes Azure/azure-sdk-for-net#59168. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
haiyuazhang
requested review from
JoshLove-msft,
ShivangiReja,
christothes,
jorgerangel-msft,
joseharriaga,
jsquire and
m-nash
as code owners
microsoft-github-policy-service
Bot
added
the
emitter:client:csharp
commit: |
Contributor
|
No changes needing a change description found. |
Contributor
jorgerangel-msft
left a comment
jorgerangel-msft
left a comment
There was a problem hiding this comment.
Can we please add unit tests + trigger a regen preview ?
| internal class InputEnumTypeIntegerValue : InputEnumTypeValue | ||
| { | ||
| public InputEnumTypeIntegerValue(string name, int integerValue, InputPrimitiveType valueType, string? summary, string? doc, InputEnumType? enumType = default) | ||
| public InputEnumTypeIntegerValue(string name, long integerValue, InputPrimitiveType valueType, string? summary, string? doc, InputEnumType? enumType = default) |
Contributor
There was a problem hiding this comment.
why did we change this to long if it's an int value?
Contributor
There was a problem hiding this comment.
Looks like we are using the same type for both.
Contributor
There was a problem hiding this comment.
Which is causing downstream issues in the tests. Can we also add explicit test cases for long?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes Azure/azure-sdk-for-net#59168.
Problem
The C# generator crashes during code generation when a TypeSpec
union/enumdeclares its base type asint64(or any primitive other thanstring/int32/float32). The failure surfaces as aTypeInitializationExceptiononModelReaderWriterContextDefinitionwith the innerJsonExceptionswallowed, making it hard to diagnose.Root cause:
InputEnumTypeValueConverter.CreateEnumTypeValueonly handlesString,Int32, andFloat32in itsswitch. Anything else falls into a barethrow new JsonException()with no message.Fix
InputPrimitiveTypeKind.Int64arm usingrawValue.Value.GetInt64().InputEnumTypeIntegerValue.IntegerValue(and its ctor parameter) frominttolongso it can hold both Int32 and Int64 values. The class isinternaland downstreamEnumProvider.IsIntValueTypealready acceptstypeof(long), so no further changes are required.JsonExceptionmessage to include the offending kind, enum name, and value name so future unsupported kinds are diagnosable from build output.Other kinds (
Int16/Int8/Float64/Decimal/Decimal128/etc.) are intentionally out of scope for this change —int64is the kind blocking real specs today (see Azure/azure-rest-api-specs#43069). The improved error message makes any remaining unsupported kinds easy to identify.