[TCGC] Require explicit union hierarchies - #5561
JoshLove-msft wants to merge 4 commits into
Conversation
Require model variants of union extends constraints to retain the declared base model in their nominal inheritance chain. Enable the guidance in client-sdk with regression tests and documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
All changed packages have been documented.
Show changes
|
📦 Package size report✅ No notable package size changes compared to the base branch. 13 package(s) with no notable change
Packed = gzipped |
commit: |
|
You can try these changes here
|
Require extends constraints on named unions and reject models reused across model-based union hierarchies. Rename the rule, reuse the shared tester, simplify redundant tests, and update documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Mark Cowlishaw (markcowl)
left a comment
There was a problem hiding this comment.
The new practice is to make linting rules off by default, and enable them ins separate pRs against the aure-rest-api-specs repos.
Mark Cowlishaw (markcowl)
left a comment
There was a problem hiding this comment.
Some leftover unnecessary text in the rule documentation, and also some questions about how to manage the impact of all current string unions in libraries and specs needing to be updated.
A much less disruptive change would use the getUnionAsEnum to except string valued enums from the rule, and reserve it for other enum usages.
Leave scalar, literal, and enum-only named unions unconstrained. Preserve model inheritance and ownership checks, expand regression coverage, and update rule documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require constraints on numeric and other non-string named unions while retaining the exemption for string-assignable unions. Preserve model hierarchy checks and expand tests and documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Fixes #5559. Related to #5390; expanded during review to include non-string union constraints and model ownership. String unions are exempt from the
extendsrequirement; numeric and other non-string named unions are not.@azure-tools/typespec-client-generator-core/use-union-hierarchy, enabled by theclient-sdkruleset only.extendsconstraint unless they are nonempty string unions. Use the compiler's string assignability check so string literals, extensible strings, derived string scalars, string templates, and nested string unions remain exempt. Numeric, model, boolean, enum, mixed, and nullable named unions still require constraints. Anonymous union expressions remain unaffected.union extends, require every model variant to be the exact declared base or inherit from it directly or transitively. Accept aliases andmodel iscopies that retain that inheritance chain. Reject structural matches, spreads, shape-onlymodel iscopies, unrelated hierarchies, and different model-template instantiations.Pet/PetBasein the docs, show both a constrained numeric union and an unconstrained string union, and remove the implementation-detail scope and enablement sections requested in review. Compiler-managed library exclusion and suppression remain unchanged. Regenerate reference listings and update the feature changeset.Uses the pinned compiler's typed
Union.baseTypeAPI, existing typekit assignability API, and actualunion-extendsfeature flag; no compiler dependency update or AST workaround.Validation
Latest revision (
ec593edb1):pnpm format,pnpm lint(including type-aware lint), and explicit spelling checks passed.int:azure-specsremains enabled for external integration on the updated commit. The latest CI result is pending.Earlier validation on this branch included dependency builds, context/internal-utils and ruleset tests, and
pnpm validate:pr --skip-build --skip-test. The full monorepo build/test suites were not rerun for this correction. An earlier full TCGC test-projecttsc --noEmitprobe reported existing errors in untouched tests/test helpers and Vitest project configuration; the package build and type-aware lint pass. Initial dependency restoration also encountered an unrelated Python preparation TLS failure after JavaScript dependencies were installed.