-
Notifications
You must be signed in to change notification settings - Fork 10.1k
add deprecation marks #37999
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
base: main
Are you sure you want to change the base?
add deprecation marks #37999
Conversation
internal/lang/marks/marks.go
Outdated
|
|
||
| type DeprecationMark struct { | ||
| Message string | ||
| Origin *hcl.Range |
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.
Should we try tracking some source address and attribute path info here, maybe something like a globalref.Reference? While we can render the source position once the diagnostic is printed, that doesn't directly tell the user which type and attribute path is being used.
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.
Currently the origin is sort of unused, we just show the expression containing the mark where it is used. I wanted to do a bit more work towards a good error message that show both where it's used and the origin, but that is more aspirational. I could remove origin for now while it's unused?
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.
Yeah, maybe we leave it out for now, rather than have a half-solution. Maybe because we render these at the first point of usage, there's enough context for users to know where the problematic source is?
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.
I would think so. I definitely want to do a spike on a nice "this is the usage and this over here is the origin of the mark" diagnostic message but I haven't had the time and energy to delve into that yet 😅
we will add sth like this again when we will be able display a nicer diagnostic with the origin
| var Deprecation = NewDeprecation("") | ||
|
|
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.
Is there any reason for this to be a public API? If it is only used internally to type-check, then maybe not
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 has to be, so that you can do e.g. marks.Has(value, marks.Deprecation)
This PR prepares deprecations as a concept in TF. For the full end-to-end implementation see #37795, this PR splits up that work into more reviewable chunks.
Fixes #
Target Release
1.15.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry