Add support for code generator property name hints

[delvedor]

This will allow the JSON model to suggest to language generators how to call the requests body parameter if it's a generic and not a structured object.

It's extremely useful for clients that abstract away the HTTP lingo from the surface API of the client.

Example usage:

/**  * @rest_spec_name index  * @since 0.0.0  * @stability stable  * @class_serializer IndexRequestFormatter`1`  */ interface IndexRequest<TDocument> extends RequestBase { path_parts?: { ... } query_parameters?: { ... } /** @identifier document */ body?: TDocument }

How it will look like in the JSON model:

"bodyIdentifier": "document",

[swallez]

I don't think this PR is necessary. The spec writer already gives us an indentifier, which is the generic parameter name, from which we can derive the identifier.

This is examplified with TDocument having the document identifier.

In the case of TBody having the contents identifier, we should just rename the generic parameter to TContents: even from a spec perspective TBody isn't descriptive enough.

So let's keep things minimal, both for the spec writer and the model. The identifier can be derived with a simple rule like "if the generic parameter name starts with an uppercase T followed by an uppercase letter, remove the T". No need for an additional annotation and an additional property in the model.

And if we want to enforce the convention that open generics must start with a T, this can be a new validation rule.

[delvedor]

@swallez I'm afraid I have to disagree. I think it's better to be explicit rather than have a rule written somewhere in the documentation and force every language generator to rewrite this logic. The model is machine-friendly, and we should keep it in that way without having external conventions.
Furthermore, we have the @identifier tag in place already, so we add nothing more than a field in the json model.

[swallez]

we add nothing more than a field in the json model

This is what I'd like to avoid, only for 2 occurences, when the information is already out there. And if we consider that dropping the T is too complex (I don't), then we should just move away from that convention in the model.

FWIW this is the only occurrence of the hungarian notation in our specs.

Adding something to the json model should not be taken lightly and we should keep it as minimal as possible.

we have the @identifier tag in place already

This is a very different use case. This tag is necessary for enums when values on the wire are not case insensitive distinct and so cannot be used to generate distinct identifiers following a target language's naming conventions. These values are not defined by us (spec writers). The case of open generic parameters is different as we choose their names.

So in all likelyhood we will end up with:

/** @identifier foo */ body?: TFoo

This @identifier tag brings no real value if it's the same (minus the T) as the generic parameter name. And if it is different then we have to question ourselves why this is the case, furthermore considering that body will be the only place where this open parameter is used.

[delvedor]

After internal discussion we have decided to bench this one out and revisit it in the future.
Language generators should derive the body name from the generic where it applies.

[swallez]

Having thought more about this, I finally understood where my discomfort with the proposed change comes from. The change is as follows:

 body?: ValueBody | PropertiesBody behaviors?: Implements[] attachedBehaviors?: string[] /**  * The parameter name language generator should use,  * normally used when the entire body is a generic.  */ bodyIdentifier?: string

As it is, it is "polluting" the main request definition with a field that is only applicable for request bodies defined as a single value rather than a set of properties. And the doc comment further constrains the applicable scenarios to generic parameters only.

Now we actually have this distinction built into the body definition: body?: ValueBody | PropertiesBody. So this feature belongs to ValueBody and not to Request (and BTW the fact that we had to name it bodyIdentifier and not just identifier should have been a warning sign):

export class ValueBody { kind: 'value' value: ValueOf /** Identifier to use to represent the body in the request structure */ identifier?: string }

Now is this really needed?

I suggested to use the generic parameter name as the identifier. It does work with the use cases that we currently have, that is when ValueBody.body is a simple InstanceOf { type: <the open generic parameter>}, i.e. within the limits of the "when the entire body is a generic" constraint. But it doesn't work for more complex declarations that are permitted by the model and can also be expressed in the input spec.

So I'm changing my mind. If identifier is part of ValueBody as suggested above, I'll happily LGTM this PR.

Note also the change in the doc comment that no longer mentions generic parameters. It's just a property of body-as-a-value, whatever its type.

Modeling is hard, and metamodeling even more 😉

[delvedor]

I like your suggestion @swallez! I'll update the pr to implement it :)