Skip to content

Support metadata parameters in body models and use methodParameterSegments for accessor paths#3848

Merged
JialinHuang803 merged 11 commits intoAzure:mainfrom
JialinHuang803:fix/override-group-parameters
Apr 1, 2026
Merged

Support metadata parameters in body models and use methodParameterSegments for accessor paths#3848
JialinHuang803 merged 11 commits intoAzure:mainfrom
JialinHuang803:fix/override-group-parameters

Conversation

@JialinHuang803
Copy link
Copy Markdown
Member

@JialinHuang803 JialinHuang803 commented Mar 17, 2026

Problem

Two issues in the Modular emitter prevented correct handling of body models containing metadata properties (@header, @query, @path):

  1. emitModels.ts: The isMetadata() filter unconditionally removed all metadata properties from model interfaces. When a body model contains @header or @query properties, these were stripped from the generated interface — making it impossible for users to pass those values.

  2. operationHelpers.ts: Parameter accessor paths were built using bare param.name instead of walking TCGC's methodParameterSegments. This produced incorrect code when parameters are nested inside body models or grouped via @@override.

As a result, the @@override decorator's group parameters feature (issue #3540) is also resolved, since grouping @query/@header params into a model is a specific case of metadata properties in a body model.

Fix

emitModels.ts

  • Conditionally keep metadata properties for input models (users need to pass these values)
  • Filter metadata for output/exception models (deserialized separately via response headers)
  • Add ARM resource model workaround: skip inherited @path name property (handled by ARM infrastructure)
  • Add isArmResourceModel() helper to detect ARM resource base types by walking ancestor chain

operationHelpers.ts

  • Add getMethodParamExpr() to build property accessor expressions from methodParameterSegments
  • Add getParamAccessor() as a unified entry point for resolving parameter accessor paths (client-level, method-level with segments, or fallback)
  • Refactor getOptional, getRequired, getPathParamExpr, buildHeaderParameter, getCollectionFormatForParam to use the new paramAccessor string instead of reconstructing paths from optionalParamName + param.name
  • Handle required vs optional method params correctly (no spurious options?. prefix on required body params)
  • Fix optional path parameter accessor to use options?.param instead of options["param"]

Tests

  • Unskipped "Should handle parameter grouping when using @@override" unit test
  • Added 5 new bodyMetadataExtraction unit test scenarios:
    1. Required @header + @query in @bodyRoot model
    2. Required @path in body model
    3. Optional @header + @query in @bodyRoot model
    4. Optional @path in body model
    5. Optional body model with @header + @query metadata
  • Fixed existing tests for client-level header params, optional path params, and parameter normalization

Fixes #3540

- Keep @header/@query/@path metadata properties in model interfaces for
  input models while filtering them for output/exception models
- Use TCGC methodParameterSegments to build correct property accessor
  paths for grouped/nested parameters (getMethodParamExpr)
- Refactor getOptional, getRequired, getPathParamExpr, buildHeaderParameter
  to use the new accessor logic
- Add ARM resource model workaround for inherited @path name property
- Add unit tests for override group parameters and body metadata extraction
- Update generated smoke test code to reflect metadata property additions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JialinHuang803 JialinHuang803 force-pushed the fix/override-group-parameters branch from cab2711 to 904fae6 Compare March 17, 2026 08:40
@qiaozha qiaozha self-assigned this Mar 19, 2026
@qiaozha qiaozha added P1 priority 1 p0 priority 0 HRLC and removed P1 priority 1 labels Mar 19, 2026
@JialinHuang803 JialinHuang803 force-pushed the fix/override-group-parameters branch from 99eccef to e12fd76 Compare March 25, 2026 09:07
Comment thread packages/typespec-ts/test/modularUnit/scenarios/operations/override.md Outdated
Comment thread packages/typespec-ts/src/modular/emitModels.ts
@@ -0,0 +1,497 @@
# Should extract header and query from bodyRoot model containing header and query metadata
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if you could add some test cases where the property has been moved to client via @clientLocation parameter.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is not supported yet and we couldn't get sufficient information from TCGC. There is no spector test case for it as well. I think we can put it as low priority since there is no real case from services.

@JialinHuang803 JialinHuang803 enabled auto-merge (squash) April 1, 2026 08:48
@JialinHuang803 JialinHuang803 merged commit 9d36ded into Azure:main Apr 1, 2026
16 checks passed
@JialinHuang803 JialinHuang803 deleted the fix/override-group-parameters branch April 1, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

HRLC p0 priority 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[modular] @override group is not supported yet.

2 participants