-
Notifications
You must be signed in to change notification settings - Fork 234
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
Relocate diagnostic from node to property. #2299
Conversation
Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status): Website: https://cadlwebsite.z1.web.core.windows.net/prs/2299/ |
You can try these changes at https://cadlplayground.z22.web.core.windows.net/prs/2299/ Check the website changes at https://cadlwebsite.z1.web.core.windows.net/prs/2299/ |
@@ -1454,7 +1454,7 @@ export function createChecker(program: Program): Checker { | |||
createDiagnostic({ | |||
code: "intersect-duplicate-property", | |||
format: { propName: prop.name }, | |||
target: node, | |||
target: prop, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does it make it better for the trait issue?
This seems worse to me now, it shows the error somewhere far from usage.
I think the solve the trait issue it need the underlying stacktrace of diagnostic so it can report the error higher up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it just displays the error on the import statement, which is the absolute worse. I can't find the underlying issue for that but I thought we had one. This was just an experiment to get the playground up so I could see the difference. If there's no easy fix, I'll just have to defer until that larger issue is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shows at the top in the playground because it reports the error in another file that the main.tsp
If you run this code in the command line you get this
Issue in question to solve those #1321
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, perfect. In that case, I'll close this and leave that issue to address the stack trace.
Necessary to support https://github.com/Azure/typespec-azure/pull/3414