Migrate @azure-tools/typespec-go from autorest.go into this repo#4628
Open
tadelesh wants to merge 59 commits into
Open
Migrate @azure-tools/typespec-go from autorest.go into this repo#4628tadelesh wants to merge 59 commits into
tadelesh wants to merge 59 commits into
Conversation
tadelesh
requested review from
bterlson,
markcowl and
timotheeguerin
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Collaborator
|
All changed packages have been documented.
Show changes
|
commit: |
Collaborator
|
You can try these changes here
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix 13 spelling typos carried over from upstream source (acceptible,
credendial, defef, deferenced, depracated, Disciminator, encounted,
enveloipe, furture, hepler, Iterface, specifiy, uhandled).
- Rename formatValue's defef parameter to deref (callers pass positionally).
- Rename trackDisciminator function to trackDiscriminator.
- Add cspell ignorePaths for packages/typespec-go/{test,temp,.scripts}/**.
- Add legitimate Go SDK identifiers (azfake, azidentity, GOPATH, Errorf,
Fatalf, polymorphics, Retriable, Unmarshaller, etc.) and Go stdlib
package names (cmplx, fcgi, flate, gofmt, pkix, etc.) to dictionary.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ran pnpm sync-labels to regenerate triage policy files and CONTRIBUTING.md labels reference with the new emitter:go area label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The filtered install (--filter @azure-tools/typespec-go...) doesn't pull in core's workspace-root devDependencies (prettier-plugin-organize-imports), which @typespec/xml's gen-extern-signature -> tspd -> prettier requires at build time. Match the build/test jobs and install everything. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- nooptionalbody/testdata/generate/transforms.go: hand-authored helper invoked by after_generate.go's //go:generate directive. Without it, the emit's post-step go generate fails, cleanGeneratedFiles wipes all zz_*.go except zz_version.go, and the orphan triggers a downstream golangci-lint 'unused const' error. - rawjson/go.mod and go.sum: rawjson uses containing-module=rawjson/v2 and emits into subpkg/. The module root (one level up) must exist for the emit's post-step go mod tidy to find go.mod. Also: surface emit failures in tspcompile.js by setting process.exitCode = 1. Previously a failed emit only logged 'exec error' and let the script exit 0, hiding the failure until a downstream lint job complained. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
verify-labels CI runs pnpm sync-labels --github --check which requires the banner emitted by core/eng/common/scripts/labels/definitions.ts:295 to match exactly. PR #4563 had hand-patched the banner to a different path that the script never reproduces, so any PR touching CONTRIBUTING.md or eng/config/labels.ts fails the check. Re-running pnpm sync-labels produces this content; committing verbatim. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…seline
After pnpm regenerate finishes, the locally regenerated test artifacts under packages/typespec-go/test/{local,http-specs,azure-http-specs}/ are automatically mirrored into a sparse checkout of Azure/azure-sdk-assets@typespec-go in packages/typespec-go/temp/baseline/. Developers can then inspect codegen deltas with a plain git -C temp/baseline status / git diff — no separate command needed.
- New .scripts/sync-baseline.js: idempotent shallow+blobless sparse clone (test/local, test/http-specs, test/azure-http-specs); mirror copies the local tree into the checkout after regen.
- tspcompile.js: kicks off the sync at startup and runs the mirror in beforeExit so it fires after every emit's async callback has drained. Skipped on regen failure (process.exitCode != 0) so partial output doesn't pollute the baseline.
- ci-go.yml: TYPESPEC_GO_SKIP_BASELINE=1 on the Regenerate step so CI doesn't need network access to azure-sdk-assets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Are there plans to get more granular WRT what runs in CI? At present it looks like quite a few things ran that's not germane to this emitter. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…shot unit tests Replace most hand-written local spec integration tests with fast, in-memory markdown-driven snapshot unit tests under test/unittest/scenarios. Each scenario compiles TypeSpec in-process and snapshots the generated Go source, run via vitest. - Add the scenario harness (scenario-runner.ts), per-file suite generator (gen-scenario-suites.mjs), and a drift guard (scenario-suites.guard.test.ts). - Migrate ClientOption, Internal.Pager, ModelsOnlyWithBaseTypes, NoOptionalBody, PageableLROs, Random.Management, RawJson and Test.Management (ARM) specs into focused scenarios, including sample-generation and example-input support. - Migrate the Regressions code-gen cases into scenarios; slim the remaining integration test to fake-server runtime behavior and rename it to fakeserver. - Keep gogenerate as a post-generation go-generate integration test. - Document the testing strategy (scenarios first; local specs only for runtime Go behavior) in docs/development.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…pshots Adds a `test:update` script that runs vitest with SCENARIOS_UPDATE=true to rewrite the expected output blocks in test/unittest/scenarios/*.md in place. Uses a small .scripts/test-update.js wrapper (matching the existing script wrappers) to set the env var cross-platform without a new dependency. Update the development doc to reference the command. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tspcompile.js now only sources two service specs from azure-rest-api-specs (keyvault Keys → azkeys, storage BlobStorage → azblob); the other 15 RM/data-plane spec paths are no longer referenced. Drop them from the sparse-checkout list so the sync fetches only what the tests need. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collapse wrapped prose paragraphs to single lines and correct the lint:go description to note it covers test/local, test/http-specs and test/azure-http-specs (not just test/local). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The old intro claimed everything under test/ is generated from specs. Describe the actual mix (in-memory unit tests, local spec tests, external Spector specs) and note that regenerate only writes the Go fixtures under test/local, test/http-specs and test/azure-http-specs, while test/unittest is updated via pnpm test:update. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… vs tests The old Step 3 (Regenerate) and Step 4 (Test) were both loosely "about tests" and awkwardly ordered, while the closely-related Step 5 (Review the generated-code diff) sat after them. Restructure so the steps follow the actual workflow: - Step 3 "Regenerate and validate the generated code" now groups everything about the generated artifacts: regenerate, review the baseline diff (merged from the old Step 5), run the Go tests, lint the generated Go, and debug. - Step 4 "Add or update tests" is now purely about authoring tests: choosing the right tier and writing an emitter unit-test scenario. - Renumber the remaining steps and fix all cross-references and the TOC. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerating fixtures, running the Go tests and linting are all part of the same testing story as the unit-test scenarios (they exercise tiers 2 and 3 of the test taxonomy), so splitting them across two top-level steps was artificial. Merge them into one "Step 3: Test your changes" ordered from cheapest to heaviest: choose the tier, write an in-memory scenario, then (for the generated-Go tiers) regenerate, review the diff, run the Go tests, and lint. Renumber docs/PR to Steps 4/5 and fix the TOC and cross-references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ports Azure/autorest.go#2022 into the migrated emitter: emit `fmt.Errorf("unmarshalling type %T: %s", recv, err.Error())` instead of the `%v` + err form in the three JSON-unmarshaller code paths. This makes the intent to propagate the error message explicit and avoids tripping errorlint in the generated code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bump the package version to 0.14.3 and add the 0.14.3 changelog section to match the upstream Azure/autorest.go release (adapting Access.internal polymorphic returns; JSON unmarshalling errors now use %s + err.Error()). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve conflict in eng/pipelines/jobs/build-for-publish.yml: main (#4824) decoupled emitter e2e + spector-coverage from the publish critical path by moving those steps into eng/pipelines/jobs/spector-coverage.yml. Take main's version of build-for-publish.yml and add the Go e2e steps as a new `spector_go` job in spector-coverage.yml, matching the Python/Java/TS jobs (turbo-cache restore + GoTool + regenerate + test:go:e2e + upload-spector-coverage).
| To (re)generate the expected output blocks from the current emitter, run the update command and review the produced snapshots as part of your change: | ||
|
|
||
| ```terminal | ||
| pnpm test:update |
Member
There was a problem hiding this comment.
I'm not able to get this to work. I added a new testing.md file with a tsp block (simple operation that returns a model). After executing pnpm test:update the tests run/pass but I don't see any Go code in the .md file.
Member
Author
There was a problem hiding this comment.
Did you add empty block with which generated Go file you want to check? I added a warning for empty Go block case.
Member
There was a problem hiding this comment.
It could be the case that I misspelled the file. I have it working now (erroring on no blocks is a nice addition and makes it clear the author did something wrong). But this makes me think that maybe we need some tweaks here.
- if no Go blocks are specified, it would be great if the failure output could list the possible block names
- it would also be good to have a simple way to say "all blocks". maybe something like
go all, or even just an unnamed Go block
… fix scenario UX - Rename the npm script `regenerate` back to `tspcompile` (matches the original name and the `.scripts/tspcompile.js` file); update ci-go.yml and the Go spector-coverage job to call `pnpm run tspcompile`. - Remove the azure-sdk-assets baseline diff scheme entirely: delete .scripts/sync-baseline.js, drop the sync/mirror calls from tspcompile.js, the `TYPESPEC_GO_SKIP_BASELINE` env, and the "Review the generated-code diff" doc section (decided not to use this approach). - Scenario usability: `test:update` now regenerates the per-scenario suites first so a newly added `.md` is picked up automatically; the harness now fails explicitly when a scenario has a `tsp` block but no `go <name>` output block (previously it silently passed producing no Go); make gen-scenario-suites emit prettier-stable output so regenerating never drifts; document the output-block requirement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wrap the verbose `turbo run build --filter=@azure-tools/typespec-go...` (build the emitter plus its workspace deps) in a `build:deps` package script. Update the development guide's Step 2 to `pnpm build:deps` and the four ci-go.yml build steps to `pnpm --filter "@azure-tools/typespec-go" run build:deps`, giving a single source of truth for how to build the emitter. Addresses the PR review comment asking for a package.json script. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jhendrixMSFT
approved these changes
Jul 16, 2026
|
|
||
| // DiscriminatedBaseNoSubTypes - A discriminated base type with no subtypes. | ||
| type DiscriminatedBaseNoSubTypes struct { | ||
| // REQUIRED; Some property. |
Member
There was a problem hiding this comment.
Nit: indentation is missing
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
5 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.
Migrates
@azure-tools/typespec-gofromAzure/autorest.gointo this monorepo. See Azure/autorest.go#1982 for full background and plan.What's in this PR
Source code (Phase 3 of the migration plan)
packages/typespec-go/containing the emitter and its three previously-private workspace dependencies inlined:codegen.go→src/codegen/codemodel.go→src/codemodel/naming.go→src/naming/README.md,LICENSE,CHANGELOG.md,vitest.config.ts,tsconfig.json(mirrorstypespec-python)..chronuschange entry.Tests
The emitter is tested with three tiers, in order of preference (documented in
docs/development.md):test/unittest/scenarios/*.md: each.mdcompiles a small TypeSpec in memory and snapshots the generated Go, with no Go toolchain or mock server. ~29 focused scenarios cover naming, models/serialization, client & options shapes, polymorphism, client customizations (@@alternateType/@@clientName), containing-module, sample generation (example JSON input →*_example_test.go), and the formerRegressionscode-gen cases.scenario-runner.ts— in-memory compile + snapshot harness (SCENARIOS_UPDATE=trueregenerates expected blocks; ARM specs auto-routed to an ARM tester).gen-scenario-suites.mjs— generates one vitest suite per.md;scenario-suites.guard.test.tsfails if they drift.Azure/azure-rest-api-specsat a pinned commit via a sparse, blobless, shallow clone:.scripts/azure-rest-api-specs.json— pinned commit + sparse paths.scripts/sync-azure-rest-api-specs.js— idempotent sync (skipped when cache marker matches).scripts/tspconfig.yaml— stub config that suppresses upstream tspconfig bleed (the per-language@azure-tools/typespec-gooptions that targetazure-sdk-for-go)gogenerate(test/tsp/GoGenerate→test/local/gogenerate) — the post-generationgo generatehook.fakeserver(test/tsp/FakeServer→test/local/fakeserver) — fake-server runtime behavior (context cancellation, dispatch races, path/query decoding); a minimal 2-operation spec.Removed local specs (migrated into scenarios):
ClientOption,Internal.Pager,ModelsOnlyWithBaseTypes,NoOptionalBody,PageableLROs,Random.Management,RawJson, and theTest.ManagementARM provider. The oldRegressionsgrab-bag was split into scenarios plus the slimmedfakeserverruntime test. The localExactNametest was dropped (duplicated by an azure-http-specs scenario).Build / regenerate
.scripts/tspcompile.js— Always invoked with--config=.scripts/tspconfig.yamlso upstream tspconfigs don't bleed..scripts/semaphore.js— 8-way parallel compile primitive..scripts/spector.js— reused start/stop wrapper aroundnpx tsp-spector.CI
.github/workflows/ci-go.yml— jobs:build,test(Go matrix1.26.1/1.25.8with regenerate +lint:go+ spector-backedtest:go:e2e),unit(vitest),docs(regen-docs)..github/actions/setup-go/action.yml— composite wrappingactions/setup-go@v5.eng/config/labels.ts— addsemitter:goarea label + path mapping.eng/pipelines/jobs/build-for-publish.yml— adds Go regen + e2e +upload-spector-coverage.ymltemplate call (GeneratorName=@azure-tools/typespec-go) after the Python block.tsconfig.ws.jsonincludes the new package.mise.tomlpinsgo = "1.26.1"for local development; in CI Go is installed viaactions/setup-go(GH Actions) andGoTool@0(ADO release pipeline)..gitignoreignores generatedzz_*.go/go.mod/go.sum/LICENSE.txt/testdataunderpackages/typespec-go/test/.Skipped per discussion
test/local/stays in this repo — not relocated toazure-sdk-for-go.Validation (local)
pnpm build✅pnpm test(vitest unit) ✅ 47/47 (29 scenario suites + guard + emitter tests)pnpm regenerate✅ all tsp compilespnpm format:check✅ clean for allpackages/typespec-go/**filespnpm lint:go✅0 issues(golangci-lint v2.11.4 + shadow, every generated module)pnpm test:go:e2e✅ spector start → hand-written test modules (azblob / azkeys / gogenerate / fakeserver) PASS → spector stopFollow-ups (out of scope; tracked in autorest.go#1982)
codegen.go/codemodel.go/naming.goinautorest.goonce Phase 4 lands.autorest.go(after Phase 4).