Add inline named models support#9694
Conversation
Add optional `id` field to `ModelExpressionNode` to support inline
named models in property type positions. Example:
```typespec
model Parent {
child: model Child {
age: int32;
}
}
```
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…rator and nesting support with tests Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…st, response, and template parameter Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…med-models-support
microsoft-github-policy-service
Bot
added
the
compiler:core
|
All changed packages have been documented.
Show changes
|
commit: |
|
You can try these changes here
|
witemple-msft
left a comment
witemple-msft
left a comment
There was a problem hiding this comment.
I think this is worth doing. Not just for model expressions, but for all keyword declarations.
| // When the model expression has a name, declare it in the enclosing namespace/script scope | ||
| // so it can be referenced by name (e.g. from augment decorators). | ||
| const prevScope = scope; | ||
| scope = getEnclosingDeclarationScope(); | ||
| declareSymbol(node as unknown as Declaration, SymbolFlags.Model | SymbolFlags.Declaration); | ||
| scope = prevScope; | ||
| } else { | ||
| bindSymbol(node, SymbolFlags.Model); | ||
| } |
There was a problem hiding this comment.
I'm not sure we really want to do this for expressions. It's going to make some weird lexical behavior:
It's not clear to me that in this example Y should escape the scope of X.y.
| function parseInlineNamedModelExpression(): ModelExpressionNode { | ||
| const pos = tokenPos(); | ||
| parseExpected(Token.ModelKeyword); | ||
| const id = parseIdentifier(); |
There was a problem hiding this comment.
I'd propose making the identifier optional. Not so much because of models since we already have bare {} model literals, but for other keyword-expression forms, like enum { ... } or union { ... }. I want to give all such things expression forms, and for symmetry I think it would be nice to allow the identifier to be optional. That does pose some challenges, as I think some of our Type variants have mandatory name.
There was a problem hiding this comment.
yeah +1 for optional identifier so it allows the other PR to work better too
| }; | ||
| } | ||
|
|
||
| function parseInlineNamedModelExpression(): ModelExpressionNode { |
There was a problem hiding this comment.
I feel like we also need to allow this for all the types(enum, union at least)
|
From design discussion
|
microsoft-github-policy-service
Bot
added
the
stale
|
Hi @@johanste. Your PR has had no update for 30 days and it is marked as a stale PR. If it is not updated within 30 days, the PR will automatically be closed. If you want to refresh the PR, please remove the |
|
Hi @@johanste. The PR will be closed since the PR has no update for 60 days. If this is still relevant please reopen. |
Asked copilot to enable support for inline model names:
is equivalent to:
@bterlson , @witemple-msft , I'm very curious to see what ya think...