Refactor operations.ts - #5185
Merged
Joel Hendrix (jhendrixMSFT) merged 1 commit intoAug 7, 2026
Merged
Conversation
microsoft-github-policy-service
Bot
added
the
emitter:go
Copilot started reviewing on behalf of
Joel Hendrix (jhendrixMSFT)
August 7, 2026 15:34
View session
Contributor
|
All changed packages have been documented.
Show changes
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the TypeSpec Go emitter’s operation-generation implementation by extracting request/response handler generation into dedicated modules and introducing barrel (index.ts) exports for clearer API surfaces. The intent is improved readability/maintainability with no intended functional change, but I found a real behavioral bug introduced/retained in the new response-handler implementation.
Changes:
- Split request/response handler generation out of
core/operations.tsintocore/request-handler.tsandcore/response-handler.ts. - Added barrel exports for
core/*andfake/*and updated the main emitter to import through them. - Moved shared helpers (e.g.,
fixUpMethodName,getClientReceiverDefinition, media/time helpers) intocore/helpers.tsand updated call sites.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/typespec-go/src/codegen/fake/servers.ts | Switches fixUpMethodName usage to helpers.fixUpMethodName after helper relocation. |
| packages/typespec-go/src/codegen/fake/index.ts | New barrel exporting fake server generators. |
| packages/typespec-go/src/codegen/emitter.ts | Refactors imports to use core/index.ts + fake/index.ts barrels. |
| packages/typespec-go/src/codegen/core/response-handler.ts | New extracted response-handler generator (contains a confirmed unmarshal target bug). |
| packages/typespec-go/src/codegen/core/request-handler.ts | New extracted request-handler generator (large move of existing logic). |
| packages/typespec-go/src/codegen/core/operations.ts | Integrates new request/response handler modules and removes inlined implementations. |
| packages/typespec-go/src/codegen/core/index.ts | New barrel exporting core generators used by the emitter. |
| packages/typespec-go/src/codegen/core/helpers.ts | Adds/moves shared helper APIs (method naming, receiver formatting, media/time helpers). |
| packages/typespec-go/src/codegen/core/example.ts | Updates example generation to use helpers.fixUpMethodName. |
| .chronus/changes/go-op-refactor-2026-7-6-15-44-32.md | Adds internal changelog entry for the refactor. |
Suppressed comments (2)
packages/typespec-go/src/codegen/core/response-handler.ts:160
- generateResponseUnmarshaller() assigns array-of-datetime results to
result.<field>instead of the providedunmarshalTarget, so callers that unmarshal into a temp var (e.g.valin the anyResult switch) won't work correctly.
unmarshallerText += `${indent.get()}result.${helpers.getResultFieldName(method)} = cp\n`;
packages/typespec-go/src/codegen/core/response-handler.ts:173
- generateResponseUnmarshaller() assigns map-of-datetime results to
result.<field>instead of the providedunmarshalTarget, which will generate incorrect code when the unmarshaller is used with a local variable target.
unmarshallerText += `${indent.get()}result.${helpers.getResultFieldName(method)} = cp\n`;
Contributor
📦 Package size report2 packages changed size compared to the base branch.
43 unchanged package(s)
Packed = gzipped |
commit: |
Contributor
|
You can try these changes here
|
Split monolithic file into three files so it's easier to reason about. Added barrel files to clarify external APIs used by the emitter. Moved shared functions to helpers.ts. Simplified a few bits of code to avoid more shared helpers.
Joel Hendrix (jhendrixMSFT)
force-pushed
the
go-op-refactor
branch
from
August 7, 2026 15:51
169c46c to
3013ad9
Compare
Joel Hendrix (jhendrixMSFT)
marked this pull request as ready for review
August 7, 2026 16:03
Joel Hendrix (jhendrixMSFT)
enabled auto-merge
August 7, 2026 16:44
Jeff Fisher (xirzec)
approved these changes
Aug 7, 2026
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
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.
Split monolithic file into three files so it's easier to reason about. Added barrel files to clarify external APIs used by the emitter. Moved shared functions to helpers.ts.
Simplified a few bits of code to avoid more shared helpers.
No functional changes.