Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@clientName shouldn't carry over #233

Closed
timotheeguerin opened this issue Feb 7, 2024 · 8 comments · Fixed by #412
Closed

@clientName shouldn't carry over #233

timotheeguerin opened this issue Feb 7, 2024 · 8 comments · Fixed by #412
Assignees
Labels
bug Something isn't working lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area

Comments

@timotheeguerin
Copy link
Member

Similar to discussion we just had for @friendlyName, @clietnName shouldn't carry over model is

Playground

@timotheeguerin timotheeguerin added the lib:tcgc Issues for @azure-tools/typespec-client-generator-core library label Feb 7, 2024
@tadelesh tadelesh added the bug Something isn't working label Feb 22, 2024
@tadelesh
Copy link
Member

From TypeSpec doc: https://typespec.io/docs/language-basics/models#is, is will carry over the decorators, Does it mean it is done by compile?

@timotheeguerin
Copy link
Member Author

Yeah by design decorator get carried over by the compiler and we don't have a way to opt out of that behavior but there is other way to make it behave the same.

If you check the target of the decorator node and compare to the decorator node parent and make sure they match then it should only run in the case where client name was defined there

@tadelesh
Copy link
Member

Yeah by design decorator get carried over by the compiler and we don't have a way to opt out of that behavior but there is other way to make it behave the same.

If you check the target of the decorator node and compare to the decorator node parent and make sure they match then it should only run in the case where client name was defined there

Will compiler provide an API to distinguish if a decorator is a direct decorator? Have @friendlyName in core resolve such problem? I suppose it should be done in the decorator set function.

@timotheeguerin
Copy link
Member Author

timotheeguerin commented Feb 28, 2024

yes @friendlyName has the same problem but we can't change it as it will be breaking(it is one of the major issue with @friendlyName and why we tried to get rid of it), @clientName has the opportunity before its too late.

And no there will not be a compiler api to do the workaround. There will be an api at some point to make decorator not carry over. The workaround shouldn't be too hard to write.

if((context.decoratorTarget as DecoratorNode).parent !== target.node) {
return;
}

@tadelesh tadelesh self-assigned this Mar 12, 2024
@tadelesh
Copy link
Member

@timotheeguerin do you know how to deal with augment decorator? I could not find a way to get the exact node for the referred node.

@timotheeguerin
Copy link
Member Author

timotheeguerin commented Mar 12, 2024

Hhm yeah didn’t think of that…

this one might be more tricky and maybe mean we need to prioritize designing a better solution in the compiler.

what you can do is check the node of the augment decorator then see what it is the first argument and check the reference symbol to get the nodr

@tadelesh
Copy link
Member

When I tried to resolveTypeReference for a union variant, I got infinite loop. See code in this PR.

@service({})
namespace N {
  union TestUnion{
    "A",
    "B": "B_v",
    string
  }
  op x(body: TestUnion): void;
}
namespace Customizations;

@@clientName(N.TestUnion.B, "BRename");

@timotheeguerin
Copy link
Member Author

So I think this issue only matters for model and operations names so maybe could limit to that for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants