Azure
Better heuristics for generated names
We often end up with bad names, like:
- names with suffixes with incrementally higher numbers
- things like ApiVersion20200630Preview
- Acronyms looking bad: #430
We should come up with better heuristics or invent syntax to address these issues where we can't resolve them automatically
Example 1 KnownGet6ItemsItem, Get6ItemsItem, Head6ItemsItem, Get7ItemsItem, Head7ItemsItem
Example 2 Enum0, ArrayConstraintsClientApiV1ValueGetOptionalParams, ArrayConstraintsClientApiV1ValueGetResponse
Example 3 SubscriptionInCredentialsPostMethodGlobalNotProvidedValidOptionalParams, SubscriptionInCredentialsPostMethodGlobalNotProvidedValidOptionalParams, SubscriptionInCredentialsPostMethodGlobalNotProvidedValidOptionalParams
Example 4 MultipleResponsesGet200Model201ModelDefaultError200ValidOptionalParams, MultipleResponsesGet200Model201ModelDefaultError200ValidResponse, MultipleResponsesGet200Model201ModelDefaultError201ValidOptionalParams, MultipleResponsesGet200Model201ModelDefaultError400ValidOptionalParams, MultipleResponsesGet200ModelA201ModelC404ModelDDefaultError200ValidOptionalParams, MultipleResponsesGet200ModelA201ModelC404ModelDDefaultError200ValidResponse
Example 5 Paths1Cj7DxmIndexersIndexernameSearchResetdocsPostRequestbodyContentApplicationJsonSchema , Paths1Ju2XepSkillsetsSkillsetnameSearchResetskillsPostRequestbodyContentApplicationJsonSchema , Paths1Cj7DxmIndexersIndexernameSearchResetdocsPostRequestbodyContentApplicationJsonSchema
Names in Example #1 are coming from Modeler Four that way, so there is not much we can do in the Autorest.typescript plugin. This seems to happen whenever the enum doesn't have a x-ms-enum extension defining the name.
In Example #2 We have the same issue with the enums, the other names are also coming from the swagger names. We need to give unique names to the parameters, to do this we name them OperationId + OperationName + OptionalParameters in this case all the operations belong in the client (there are no operation groups). One way to improve here is to detect if the methods are in the client remove the OperationId from the name.
Example #3 and #4 long names are coming 100% from the swagger, it already defines long names. I don't see how we can improve here. for example https://github.com/Azure/autorest.testserver/blob/95532383d21e758867ab640731a8c3d33040e921/swagger/httpInfrastructure.json#L2103
Example #5 seems to come from an inline object definition that doesn't have a name property. This is coming from Modeler4 as well which has to calculate a name for the schema.
In summary, from the above examples, there is only one thing we could fix on our side, which is if the client has no operation groups and all operations live under the client, change the naming of the parameters from OperationId + OperationName + OptionalParameters to OperationName + OptionalParameters
@joheredi for the ones that are coming from modeler four, do you have any idea of the cost to tweaking them there or should we pull someone else in for that? Also I wonder how other languages are looking in this space.
For the ones where we need service devs to better annotate their swaggers, could we maybe put together a set of checks that we could add to the swagger linting CI jobs? I'm fine saying the swaggers need to get better, but I want to automate enforcement.
Will model name conflicts in modelerfour be included in this issue ? Currently, it will randomly rename one of them with a suffix ~AutoGenerated.
@xirzec, I'm not sure about the effort in M4, I think both issues are related as both are inline nameless definitions, in both cases, it seems like a swagger issue, but I agree that validation and enforcement would be great!
I've iled https://github.com/Azure/autorest/issues/4358 asking if there is a way that M4 can enforce these inline schemas to have a x-ms-client-name
@qiaozha I think the Autogenerated suffix comes from enabling lenient-model-deduplication, which is a workaround for a buggy swagger that shouldn't have duplicate names in the first place.
@qiaozha I think the Autogenerated suffix comes from enabling
lenient-model-deduplication, which is a workaround for a buggy swagger that shouldn't have duplicate names in the first place.
Totally agree, but the effort to ask them to change is very huge, Also, we don't have a clear guideline for the model name yet.
@qiaozha the problem is that without non-unique names provided in the swagger, the best thing that the generator can do is calculate a unique name. So I guess for cases where lenient-model-deduplication it would probably be reasonable to be okay with the calculated names until the swagger is fixed. What do you think?