[tcgc] overwrite original value when set multiple value for same decorator

[tadelesh]

fix: #1036

[azure-sdk]

All changed packages have been documented.

• ✅ @azure-tools/typespec-client-generator-core

Show changes

@azure-tools/typespec-client-generator-core - fix ✏️

overwrite original value when set multiple value for same decorator

[azure-sdk]

You can try these changes here

🛝 Playground	🌐 Website	📚 Next docs

[tadelesh]

@timotheeguerin could you help to have a review?

[timotheeguerin]

I think this current logic has some issue. Do you think this should emit a warning?

https://cadlplayground.z22.web.core.windows.net/typespec-azure/prs/1606/?c=aW1wb3J0ICJAYXp1cmUtdG9vbHMvdHlwZXNwZWMtY2xpZW50LWdlbmVyYXRvci1jb3JlIjsKCnVzaW5nIEHENC5DxSVHyCQuQ29yZTsKCkDGPU5hbWUoIkZvbzEiKQptb2RlbCBGb28gewoKfc8kQmF6ySNCYXIgaXPFKsgrzCxGb28sIMRVQXVnMSIpOw%3D%3D&e=%40azure-tools%2Ftypespec-autorest&options=%7B%7D

[tadelesh]

yes. all client decorator value should be scope orthometric. they should always be used in client.tsp, so augment decorator is more common.

[timotheeguerin]

That is an azure decision though to have that client.tsp. Someone else could decide to use it inline.

[tadelesh]

got it. if the value could overlap each other, then the order should be important, which may be confuse for customer i think. let me raise this discussion in the weekly sync after our national holiday.

[timotheeguerin]

Just for clarity on the ideal design for decorators:

• Order on the element itself shouldn't be depended on. It is confusing(inverted). And if you don't allow multiple decorators(you just override) it is good to validate you don't define those twice
• Using the decorator on model is or op is to override the base model/op should work and not complain about duplicate
• Similarly using augment decorator to override what is defined locally shouldn't ideally complain either
• Multiple augment decorator are run in the order of declaration so later overrides. I think it can be ok to validate to don't augment twice but you might have to be careful also on the scope. E.g. if a library augmented a type and you want to reaugment it again might not want it to complain again.

[tadelesh]

ok, i got what you mean. then i think it is more effort than it's worth to check duplicate definition. i will change the logic to be incremental setting for all settings for one decorator, overwrite when same scope. what do you think?

[tadelesh]

i've changed the logic, please help to review.

[iscai-msft]

@tadelesh the release just went out, I feel it would be good to release this before next month's release, thoughts on updating this to the release branch?

[tadelesh]

@tadelesh the release just went out, I feel it would be good to release this before next month's release, thoughts on updating this to the release branch?

sure. changed branch.