Azure / typespec-azure

About TypeSpec Azure Libraries
https://azure.github.io/typespec-azure/
MIT License
15 stars 41 forks source link

[TCGC] client type for TypeSpec empty model #846

Closed tadelesh closed 5 months ago

tadelesh commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

pshao25 commented 5 months ago

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

tadelesh commented 5 months ago

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.

XiaofeiCao commented 4 months ago

I want to confirm the difference between anyObject in M4 and named/anonymous model in TypeSpec.

In Swagger, {"type": "object"}(or anyObject) means Free-Form Object, which can have either arbitrary properties or none.

Screenshot 2024-07-12 at 10 07 28

So my questions are:

  1. If we add properties definition to a Free-Form Object in Swagger, is it a breaking change? I tend to believe it is, since it adds additional constraint to the model properties(something like adding a required property).
  2. Does TCGC consider named/anonymous model in TypeSpec both be represented as anyObject?
  3. Does TypeSpec consider adding properties to the empty model a breaking change?
tadelesh commented 4 months ago

although in m4 we treat {"type": "object"} as anyObject, but we do not believe all the service use it in the right way. so, for swagger converter, it will be converted to anonymous model. if service team think the result is wrong, they need to change it manually. and in typespec, i believe any model without any property is just an empty model, if want any then use unknown, if want any object, then use Record<unknown>.

XiaofeiCao commented 4 months ago

Thanks. My takeaways is,

  1. Empty model in TypeSpec means model without properties, not anyObject.
  2. For brownfields, by default we convert anyObject as TypeSpec (anonymous)empty model. If service really wants anyObject, TypeSpec's anyObject equivalent is Record<unknown>.

CMIIW.