Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TCGC] client type for TypeSpec empty model #846

Closed
Tracked by #1356
tadelesh opened this issue May 16, 2024 · 4 comments
Closed
Tracked by #1356

[TCGC] client type for TypeSpec empty model #846

tadelesh opened this issue May 16, 2024 · 4 comments
Assignees
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area

Comments

@tadelesh
Copy link
Member

tadelesh commented May 16, 2024

In modeler4, we have anyObject type to represent an empty model definition despite whether it is a named definition or not (see the type of prop1 and prop2 for the following example). And most languages I believe will treat it as any type.

"definitions": {
  "EmptyModel": {
    "type": "object"
  },
  "Test": {
    "type": "object",
    "properties": {
      "prop1": {
        "type": "object"
      },
      "prop2": {
        "$ref": "#/definitions/EmptyModel"
      }
    },
    "required": [
      "prop1",
      "prop2"
    ]
  }
}

But in TypeSpec, we could clearly say a model is a named model without any properties or an anonymous model without any properties (type of prop1 and prop2).

model Test {
  prop1: {};
  prop2: EmptyModel;
}
model EmptyModel {}

So, we need to get consensus on what type we should generate for TypeSpec empty model, and also it should support non-breaking change for brown field services.

Proposal:
Introduce anyObject type in TCGC. Anonymous model without any properties will be converted to anyObject. For now, unknown is converted to any in TCGC.
Named model without any properties will be kept as a named model.
For brown field service, converter should always use anonymous model in TypeSpec without any properties to represent anyObject type in m4.

cc: @Azure/dpg-devs

@tadelesh
Copy link
Member Author

tadelesh commented May 20, 2024

Conclude all possible cases in the following table, example is in this playground:

Swagger m4 TypeSpec TCGC example
{"type": "object"} anyObject {} anonymous model with no properties prop1
{"type": "object"} anyObject model EmptyModel {} named model with no properties prop2
{} any unknown any prop3
{"type": "object", "additionalProperties": {}} dictionary with any type Record<unknown> dictionary with any type prop4
{"type": "object", "additionalProperties": {}} dictionary with any type model EmptyModelWithAdditionalProperties extends Record<unknown> {} named model with only additional properties prop5
{"type": "object", "additionalProperties": {}} dictionary with any type model EmptyModelWithAdditionalProperties is Record<unknown> {} named model with only additional properties prop6
{"type": "object", "additionalProperties": {}} dictionary with any type model EmptyModelWithAdditionalProperties {...Record<unknown>} named model with only additional properties prop7

@timotheeguerin could you help to confirm this is the right behavior for swagger/m4?
@Azure/dpg-devs: please confirm the behavior for TCGC.

Since swagger/m4 one expression could map several TypeSpec expression, for brown field service converting to TypeSpec, we will use the following mapping:

m4 TypeSpec
anyObject {}
any unknown
dictionary with any type Record<unknown>

@iscai-msft
Copy link
Contributor

Agree with the mapping at the end, we shouldn't do any extra conversion bc of the possibility of breaking changes

@pshao25
Copy link
Member

pshao25 commented May 30, 2024

Decision: everything keeps the same, only in conversion tool, change anyObject in M4 to {} in TypeSpec.

@tadelesh
Copy link
Member Author

tadelesh commented May 31, 2024

since typespec has more expression ability than swagger, we decided to use more accurate result when converting from swagger to typespec. if the conversion result is not the one service want, typespec author needs to change manually.
regarding how languges' emitter deal with the empty model is depends on languages' architect. tcgc will keep the full info and do no implicit conversion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area
Projects
None yet
Development

No branches or pull requests

4 participants