Azure / typespec-azure

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

typespec-autorest does not honor @clientName #442

Open tadelesh opened 4 months ago

tadelesh commented 4 months ago

See: https://cadlplayground.z22.web.core.windows.net/cadl-azure/?c=aW1wb3J0ICJAYXp1cmUtdG9vbHMvdHlwZXNwZWMtY2xpZW50LWdlbmVyYXRvci1jb3JlIjsKdXNpbmcgQcQzLkPFJEfIIy5Db3JlOwoKQMY8TmFtZSgiVGVzdFJlbmFtZSIpCmVudW0gxBJ7CiBBLAogQgp90y9Nb2RlbMk0bcQOxTXFGCB7CiAgcHJvcDE6IHN0cmluZwp9&e=%40azure-tools%2Ftypespec-autorest&options=%7B%7D

timotheeguerin commented 4 months ago

This was partially by design that the only thing it support was what it supported with @projectedName and we just wanted parity. However it would probably make sense to expand the usage.

markcowl commented 4 months ago

+1, we should either add x-ms-client-name or change the schema name, since it isn't actually part of the API

tadelesh commented 4 months ago

Previously, we have @projectedName("client", "xxx") to rename the type in swagger with x-ms-client-name. So my idea is do the same thing for @clientName.

timotheeguerin commented 4 months ago

That didn't work on the model name See playground, that is my point, we only kept parity.

but I don’t have anything against adding more cases

tadelesh commented 4 months ago

That didn't work on the model name See playground, that is my point, we only kept parity.

but I don’t have anything against adding more cases

Oh, I see. Thanks for explanation.

@markcowl then we should add both @extension("x-ms-client-name", "xxx") and @clientName("xxx") to keep client generated from swagger and tsp to be the same.

timotheeguerin commented 4 months ago

Why do you need to add x-ms-client-name on the model, we should just change the definition name

tadelesh commented 4 months ago

Oh. I see. Thanks for the explanation.

tadelesh commented 4 months ago

For model/enum/interface/union renaming, since name is not used in wire, we should just rename the name. For parameters/properties renaming, we should use @clientName.

timotheeguerin commented 4 months ago

Confused here @tadelesh, don’t we need this? The point was more that autorest should rename the definition not add x-Ms-client-name

tadelesh commented 4 months ago

Oh, I got your point. My view is from convert swagger to TypeSpec, we don't need to add @clientName, instead just rename the name for that not impact wire when conversion.

From autorest emitter view, the requirements are:

  1. for name that is used in wire, @clientName emit with x-ms-client-name
  2. for name that is not used in wire, @clientName change the swagger definition name directly