Copilot/add expression decorations#9747
Conversation
…ter support Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…ge; add tests Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…decorated expressions Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…ting ones Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
…ample snapshot Co-authored-by: johanste <15110018+johanste@users.noreply.github.com>
|
All changed packages have been documented.
Show changes
|
commit: |
|
You can try these changes here
|
witemple-msft
left a comment
There was a problem hiding this comment.
I think it is potentially a big problem to allow decorating type references. The implementation in this PR will allow you to overwrite metadata without controlling the original declaration.
It seems like we only want to allow decorating type literals, i.e. expressions that create unique instances of a Type.
I'm also a little concerned about the union case. We have potentially a lot of code that assumes that a union can only be decorated if it is a named declaration. We do not even allow augmentations like:
alias A = string | int32;
@@doc(A, "Example");
~ > error augment-decorator-target: Cannot augment union expressions.If the other PR allowing named model expressions lands, then I would support allowing decorating type expressions that start with a keyword: @doc("Example") model { ... }, @dec union { string, int32 }, etc.
|
yeah we did talk about this might be the only way to allow inline decorator was when using the keyword form of expressions. I also feel like from a quick look that while it does seem to pass the test I don't think copilot really did the change correctly, I don't think we need a new Decorator node in the tree |
Decorating expressions
Decorators can be applied to inline model expressions and other type expressions used in property types, aliases, and
is/extendsclauses. Place the decorator before the expression: