Fix Go example generator emitting invalid Go for RFC7231 datetime values - #4984
Fix Go example generator emitting invalid Go for RFC7231 datetime values#4984JiaqiZhang-Dev with Copilot wants to merge 9 commits into
Conversation
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: JiaqiZhang-Dev <194873822+JiaqiZhang-Dev@users.noreply.github.com>
Co-authored-by: JiaqiZhang-Dev <194873822+JiaqiZhang-Dev@users.noreply.github.com>
|
…update dev doc Co-authored-by: JiaqiZhang-Dev <194873822+JiaqiZhang-Dev@users.noreply.github.com>
Addressed all three in the latest commit:
|
commit: |
|
waiting for this pr merged: #4991 |
|
@copilot resolve the merge conflicts in this pull request |
…o-rfc7231-datetime # Conflicts: # packages/typespec-go/docs/development.md Co-authored-by: JiaqiZhang-Dev <194873822+JiaqiZhang-Dev@users.noreply.github.com>
Resolved the merge conflicts by merging |
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
Go emitter diffBaseline No changes to generated output. Rendered diff: inline on the run summary, or the emitter-diff-go-html artifact. Informational check (core/eng/emitter-diff); does not block the PR. |
Co-authored-by: JiaqiZhang-Dev <194873822+JiaqiZhang-Dev@users.noreply.github.com>
| Location: to.Ptr("eastus"), | ||
| Properties: &testmodule.WidgetProperties{ | ||
| Description: to.Ptr("my widget"), | ||
| LastExecuted: to.Ptr(func() time.Time { t, _ := time.Parse(time.RFC1123, "05 May 2021 12:00:23 GMT"); return t }()), |
There was a problem hiding this comment.
Callers should never need to manually parse time.Time into the wire format (we go to great lengths to not leak that implementation detail). Why do we need to do this here?
There was a problem hiding this comment.
Since the example value is in wire format: 05 May 2021 12:00:23 GMT, we need to manually parse it into time.Time. For sdk customers, they don't need this logic, just give the time.Time is ok.
I'm not sure if you mean that we should analyze the wire format in the code, and then generate unified code like to.Ptr(time.Date(2021, time.May, 5, 12, 0, 23, 0, time.UTC))
There was a problem hiding this comment.
Your example is correct.
The emitted code handles conversion to the correct wire format. All we should need to know is if we need to emit time.UTC or time.Local (the former should be the vast majority of cases.
There was a problem hiding this comment.
FYI I'm going to fix this as part of #4956 which is in progress.
|
I don't see any changes in the generated diff, is this expected? |
Yes, since we haven’t had a test about RFC7231 before, there’s no change in the generated diff. |
The example generator produced uncompilable Go for
utcDateTimeproperties encoded asrfc7231.getTimeValue'sformatMap(inpackages/typespec-go/src/codegen/core/example.ts) only handledRFC1123andRFC3339, soRFC7231fell through to the Unix-timestamp path — wrapping an unquoted, non-numeric date string instrconv.ParseInt. This regressed in Azure/autorest.go#1992, which switched the TypeSpec adapter to emitRFC7231but updatedservers.ts/operations.ts/models.ts/type.tswhile missingexample.ts.Actual (broken):
Now:
Changes
core/example.ts: AddRFC7231: helpers.RFC1123FormattogetTimeValue'sformatMap, mirroring the existing mapping infake/servers.ts. Both share the Gotime.RFC1123layout.fixchronus entry for@azure-tools/typespec-go.