[tcgc] allow last param in decorator to accept scope or other options - #5305
Open
iscai-msft wants to merge 19 commits into
Open
[tcgc] allow last param in decorator to accept scope or other options #5305iscai-msft wants to merge 19 commits into
iscai-msft wants to merge 19 commits into
Conversation
Addresses Azure#5254 by centralizing the type used for the `scope` argument across all scoped TCGC decorators into a single `Azure.ClientGenerator.Core.Scope` alias, defined once in decorators.tsp and reused by legacy.tsp. This lets the scope type evolve (e.g. toward a typed options bag) in one place instead of updating every decorator signature individually. No behavior change; all decorators keep accepting the existing string scope syntax. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
iscai-msft
requested review from
Laurent Mazuel (lmazuel),
m-nash,
Mark Cowlishaw (markcowl),
Timothee Guerin (timotheeguerin) and
Jeff Fisher (xirzec)
as code owners
August 25, 2026 17:46
microsoft-github-policy-service
Bot
added
the
lib:tcgc
…alias-255 # Conflicts: # packages/typespec-client-generator-core/lib/decorators.tsp
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
All changed packages have been documented.
Show changes
|
Contributor
📦 Package size report4 packages changed size, +15.68 KB (+0.1%) packed overall.
9 package(s) with no notable change
Packed = gzipped |
iscai-msft
changed the title
commit: |
Per issue feedback, the shared Scope alias is now ScopeOptions | string
instead of a plain string alias, so decorators accept either the legacy
string scope or a typed options bag (e.g. #{ scope: "csharp" }).
Individual decorators can later define their own options model extending
ScopeOptions without breaking others. Centralizes scope normalization in
internal-utils.ts, updates all decorator implementations and generated
defs/docs, and adds tests covering the options-bag form and its
equivalence with the legacy string form.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
Contributor
|
You can try these changes here
|
- Add scope to @client's ClientOptions with a conflicting-scope diagnostic when it disagrees with the legacy positional argument - Add an invalid-scope diagnostic for malformed scope strings (empty, whitespace-only, empty grouped negation, empty comma-list entries), for both legacy string and options-bag forms - Add a scope-options-migration linter rule (enabled by default) that flags legacy positional string scope arguments and offers a code fix to rewrite them as the typed options bag - Document the deprecation policy and migration guidance in design-docs/scope-options-migration.md - Expand scope compatibility tests to cover more decorators and grouped/negated/mixed forms - Update the changeset to describe the full set of changes Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
microsoft-github-policy-service
Bot
added
int:azure-specs
…-options behavior - Removed the default-enabled scope-options-migration linter rule and its codefix, doc, and tests. The issue only asked for migration guidance, not a default-on rule that would surface warnings across existing specs. - Trimmed the changeset description to the main points. - Documented that passing a value typed with additional properties beyond ScopeOptions to a decorator still using the shared Scope alias will not type-check; a decorator must update its own scope parameter type to accept an extended options model. - Regenerated linter reference docs and README after removing the rule. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
- Removed the invalid-scope diagnostic and isValidScopeString validation. There was no prior scope-format validation, and the issue didn't call for adding it; malformed scope strings are now handled the same way they always were (parsed as-is, with no diagnostic). - Changed conflicting-scope from an error to a warning. When @client's ClientOptions.scope disagrees with the legacy positional scope argument, TCGC now uses the options bag value and ignores the legacy argument, instead of skipping the decorator entirely. - Updated diagnostic docs, design doc, changeset, and tests accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
Updated the stale doc comment for @client's options param (still said "produces a diagnostic" without noting it's now a warning that prefers the options bag value and regenerated derived docs/generated-defs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39 EOF )
- Rename the shared scoped decorator options model to DecoratorOptions. - Remove the public Scope alias and use DecoratorOptions | string directly in scoped decorator signatures to avoid collisions with service models named Scope. - Update legacy signatures, generated definitions, tests, reference docs, and migration/deprecation guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances TCGC’s “scoped decorator” story by introducing a shared, typed DecoratorOptions options-bag model for decorator scope, updating all scoped decorator signatures to accept either the legacy string scope or an options bag, and extending @client to also accept scope on ClientOptions with a warning diagnostic when the two scope mechanisms conflict.
Changes:
- Add
Azure.ClientGenerator.Core.DecoratorOptionsand update all scoped decorators (including Legacy) to acceptDecoratorOptions | stringfor the finalscopeparameter. - Centralize scope-shape normalization in
normalizeScopeand introduce theconflicting-scopewarning for@clientwhenClientOptions.scopedisagrees with the legacy positional scope argument. - Add/extend unit tests, update generated defs, and refresh documentation + a changeset to reflect the new options-bag shape.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/index.mdx | Adds DecoratorOptions to the reference index. |
| website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/decorators.md | Updates decorator signatures/docs to reflect `DecoratorOptions |
| website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/data-types.md | Documents the new DecoratorOptions model. |
| packages/typespec-client-generator-core/test/decorators/scope.test.ts | Adds coverage for legacy vs options-bag scope equivalence across decorators. |
| packages/typespec-client-generator-core/test/decorators/client.test.ts | Adds coverage for @client scope via ClientOptions.scope and conflict warnings. |
| packages/typespec-client-generator-core/src/lib.ts | Registers the conflicting-scope warning diagnostic message. |
| packages/typespec-client-generator-core/src/internal-utils.ts | Adds normalizeScope helper to unify handling of string vs options-bag scope. |
| packages/typespec-client-generator-core/src/interfaces.ts | Introduces runtime TS DecoratorOptions interface for decorator implementations. |
| packages/typespec-client-generator-core/src/diagnostics/conflicting-scope.md | Adds diagnostic documentation for conflicting @client scopes. |
| packages/typespec-client-generator-core/src/decorators.ts | Updates all scoped decorators to accept options-bag scope and implements @client conflict warning behavior. |
| packages/typespec-client-generator-core/README.md | Updates README decorator reference signatures to include the options-bag scope type. |
| packages/typespec-client-generator-core/lib/legacy.tsp | Updates legacy extern decorator signatures to accept `DecoratorOptions |
| packages/typespec-client-generator-core/lib/decorators.tsp | Defines DecoratorOptions, updates extern decorator signatures, and makes ClientOptions extend DecoratorOptions. |
| packages/typespec-client-generator-core/generated-defs/Azure.ClientGenerator.Core.ts | Updates generated decorator types and emits DecoratorOptions interface. |
| packages/typespec-client-generator-core/generated-defs/Azure.ClientGenerator.Core.Legacy.ts | Updates generated legacy decorator types to accept `DecoratorOptions |
| packages/typespec-client-generator-core/design-docs/decorator-options-migration.md | Adds migration/deprecation policy guidance for the options-bag approach. |
| .chronus/changes/tcgc-scope-alias-2026-8-25-13-40-0.md | Adds a feature changeset describing the new evolvable scope/options shape. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…on doc Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9d81d1b-44c2-44db-87a1-87df6a805d39
This was referenced Aug 31, 2026
Canonicalize each scope string into the set of emitters it actually selects before reporting a conflict, so mixed forms like "!java" and "csharp, !java" no longer produce a false-positive warning. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Revert @client, @operationGroup, and @clientInitialization scope parameters to accept only string (LanguageScopes), not DecoratorOptions - Make ClientInitializationOptions extend DecoratorOptions for consistency - Move scope configuration to the options model properties instead of the scope parameter - This eliminates confusion from having two option bags and provides a single, unified options interface Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81756fca-9e84-4b3d-8480-b53d855093d2
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81756fca-9e84-4b3d-8480-b53d855093d2
…zation Both @client (ClientOptions) and @clientInitialization (ClientInitializationOptions) already have their own options bag. Keep a single options bag per decorator: scope lives in that bag (model extends DecoratorOptions) and the legacy positional scope argument stays a plain string purely for backward compatibility. - Extract a shared resolveScopeFromOptions helper that reads options.scope, reports the conflicting-scope diagnostic when it disagrees with the legacy positional scope, and prefers the options bag value. - Wire both $client and $clientInitialization through the helper. Previously ClientInitializationOptions extended DecoratorOptions but its scope property was silently ignored. - Regenerate extern signatures and reference docs; update changeset and migration doc. - Add @clientInitialization scope-in-options tests (positive, conflict, equivalent). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81756fca-9e84-4b3d-8480-b53d855093d2
Addresses JoshLove-msft's review batch: - P1: getTypeDecorators now normalizes the scope argument (legacy string or DecoratorOptions bag) before materializing decorator arguments, so an options-bag scope on an allowlisted decorator like @clientoption no longer crashes SDK generation with 'scope.match is not a function'. - P1: @clientInitialization only reads scope from a genuine ClientInitializationOptions bag. A legacy raw client-parameters model that happens to contain a scope property is preserved as a real client parameter instead of being consumed as an emitter selector. - P2: a scope-only options bag ({ scope: 'csharp' }) is no longer misclassified as the legacy parameters model, so it does not surface an unintended 'scope' client initialization parameter. - P2: @operationGroup now accepts 'DecoratorOptions | string' and normalizes the scope before delegating to @client, so @operationGroup(#{ scope: 'python' }) works. Adds regression tests for each case and regenerates extern signatures and docs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81756fca-9e84-4b3d-8480-b53d855093d2
…ns provenance Distinguish a real `ClientInitializationOptions` options bag from a legacy raw client-parameters model using model provenance (ancestry) instead of the model name. A named model that derives from `ClientInitializationOptions` is an options bag even when it only sets `scope`, so its scope selects the emitter and never leaks as a client parameter. An anonymous scope-only literal is kept as a legacy raw parameters model so its `scope` remains a real client parameter, preserving backward compatibility for that previously supported form. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81756fca-9e84-4b3d-8480-b53d855093d2
| return false; | ||
| } | ||
| for ( | ||
| let current: Model | undefined = options.baseModel; |
Member
There was a problem hiding this comment.
P2: Traverse source-model provenance when identifying initialization options
This follows only baseModel, so standard model transformations lose the options-bag identity. For example:
model Base extends ClientInitializationOptions {
scope: "csharp";
}
model Copy is PickProperties<Base, "scope">;
@@clientInitialization(MyService, Copy);Copy has source-model provenance but no baseModel, so it is treated as legacy client parameters. The generated clients expose a public scope parameter for both C# and Python instead of using it to select C#. Please traverse explicit sourceModel/sourceModels provenance for named models as well as baseModel, and add coverage for PickProperties/OmitProperties or model is.
--generated by Copilot
| legacyScope?: string, | ||
| ): string | undefined { | ||
| const optionsScopeConfig = | ||
| options?.kind === "Model" ? options.properties.get("scope")?.type : undefined; |
Member
There was a problem hiding this comment.
P2: Resolve scope through the effective options inheritance chain
This reads only the leaf model's properties, which excludes properties inherited with extends. For example:
model Base extends ClientInitializationOptions {
scope: "csharp";
}
model Final extends Base {
parameters: Params;
}Using Final with @clientInitialization applies the customization to Python as well as C#, and an inherited scope conflicting with the positional argument produces no warning. The same issue affects an inherited ClientOptions.scope on @client. Please resolve the nearest effective scope through the model inheritance chain and add multi-level inheritance tests for both decorators.
--generated by Copilot
| // remaining decorator arguments, otherwise the options-bag form is run through | ||
| // `getDecoratorArgValue` (which converts it to an SDK model) and later handed to the | ||
| // string-only `isScopeApplicable`, crashing with `scope.match is not a function`. | ||
| if (parameterName === "scope") { |
Member
There was a problem hiding this comment.
P2: Preserve type-level string scopes on allowlisted decorators
This special-cases every parameter named scope, but existing custom decorators can declare a type-level parameter:
extern dec scopedMarker(target: unknown, scope: string);
@scopedMarker("python")
model Payload {}For this signature, jsValue is a compiler string-literal type rather than a JavaScript string. Passing it directly to normalizeScope produces undefined, so emitted decorator metadata contains scope: undefined and the decorator is included for every emitter instead of only Python. Please preserve the normal getDecoratorArgValue conversion for compiler string literals, or restrict this normalization path to primitive strings and actual DecoratorOptions values.
--generated by Copilot
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.
Summary
First small step toward #5254 (Make TCGC decorator scope arguments evolvable with typed options).
This PR centralizes the type used for the
scopeargument across all scoped TCGC decorators into a singleAzure.ClientGenerator.Core.Scopealias, defined once indecorators.tspand reused fromlegacy.tsp. Having one shared alias means the scope type can evolve (e.g. toward a typed options bag as described in the issue) in one place instead of updating every decorator signature individually.Changes
alias Scope = string;indecorators.tspwith doc comments describing supported language identifiers and valid patterns (moved from the previous per-decorator@param scopedocs).scope?: valueof stringparameter indecorators.tspwithscope?: valueof Scope.scope?: valueof stringparameter inlegacy.tspwithscope?: valueof Azure.ClientGenerator.Core.Scope(different namespace).test/decorators/scope.test.tsverifying the alias can be referenced from user TypeSpec and that scoped decorators continue to behave identically.Compatibility
No behavior or public API change — decorators still accept the existing string scope syntax (
"python","python, java","!csharp", etc.). Verifiedgenerated-defs/*.tsis unchanged after rebuilding, and the full TCGC test suite (1405 tests) passes.Testing
pnpm --filter @azure-tools/typespec-client-generator-core buildpnpm --filter @azure-tools/typespec-client-generator-core test(1405 passed, 2 skipped)pnpm --filter @azure-tools/typespec-client-generator-core lintpnpm formatCloses part of the first workstream in #5254; the remaining deliverables (typed options bag, diagnostics for conflicting scopes, codefixes, migration docs) are left for follow-up PRs.