-
Notifications
You must be signed in to change notification settings - Fork 37
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
[Bug]: precedence/ordering of @@access decorators is unclear #1036
Comments
@iscai-msft, @lmazuel asked that this be assigned to you specifically. |
this seems like a bug, let me take a look at it today |
@iscai-msft, just a note, there's another issue that @lmazuel filed about handling CSV lists inside of the 'scope' string as well. The use case was similar to what we were trying to accomplish here. Issue: #978 |
@richardpark-msft after looking into it more, we don't want people to set multiple access decorators, because it gets complicated if they conflict with each other. Instead, I think the way around this is adding the ability to do a list of languages, and also adding support for |
I think so. If the order is ambiguous then it seems like multiple decorators should cause an outright error otherwise your result might be non-deterministic. What do you think of that? |
Yeah that's definitely why we decided originally to not allow multiple instances of either augmentation or direct decorating with |
Just to make sure we're talking about the same thing - in the example I gave I am applying it twice though, and there wasn't (AFAICT) an error. Should there have been? Right now it kind of appears to work, but if the resolution of that is ambiguous, depending on order, it seems like it should fail or provide some sort of warning you're putting yourself in that situation. (the comma separated list is awesome, but I think the error condition is a separate issue) |
Oops I missed this comment. Yeah you're right, there should be a warning raised here. Let me make this a bug and make sure we raise |
from our test, this will report a diagnostic. i wonder if it is because the lint rule is not enabled when compiling. |
@timotheeguerin in tcgc, i used a core api to report diagnostics (code here). it could work when test, but with the real typespec compile, there is no diagnostic reported. could you help to identify if the usage is wrong? |
the root cause is previous check does not work on augment decorator. |
Describe the bug
I've got a question about
@@access
, when there are rules that overlap.In our service we were attempting to make an operation public, only for the scope "go", but internal for everyone else. However, we couldn't do this:
This generates this type graph:
You can fix this by just ordering them differently:
This generates this type graph:
I'm not entirely sure which behavior is the correct behavior, but I did find it surprising that the more specific directive was overridden by the least specific directive. The ordering of their declarations in the file doesn't seem like it should be a critical factor, especially given how these
@@access
's could be in separate files altogether.Including @l0lawrence and @lmazuel in case I've misstated the bug.
Reproduction
Playground Link
Checklist
The text was updated successfully, but these errors were encountered: