Default value for the type annotation attribute should be Object.class #1381
Conversation
|
To be reviewed after 25.0.9 is created. |
|
The target of this PR should probably be a feature branch. @TrevCraw I thought this was the plan, right? |
TrevCraw
changed the base branch from
main
to
lsp4jakarta-0.2.5-integration
@Rejoice472 Can you provide the correct link to the defect? #580 is clearly an unrelated issue in this project. |
| if ("type".equals(attributeName)) { | ||
| return PsiElementFactory.getInstance(annotation.getProject()) | ||
| .createExpressionFromText("Object.class", annotation); | ||
| } else { |
There was a problem hiding this comment.
@Rejoice472 Does this only apply to all or only specific annotations? Seems like this change has been made to generic code and would be concerned if this is being applied to a type attribute that does not have the same Java type or semantics as a specific case you are trying to address.
There was a problem hiding this comment.
I agree. This targets all annotations that declare an attribute named type. This logic comes from the insert annotation as part of quick-fix. Initially, we used empty strings as default values for all annotation attributes. Later, I found that most annotations with an attribute named type require Object.class as the initial value to avoid compilation errors. For now, I’ve applied a simple approach to address the existing issue.
Since the user's intended values are unknown at this stage, these placeholders (e.g., name = "", type = Object.class) are meant to be updated by the user as needed and do not imply semantic correctness for all annotations.
This can cause a problem in the future only if we add an 'insert annotation' quickfix for any new annotation that has a type attribute with a value other than class.
In Jakarta EE annotations, when an attribute is named type, it is almost always declared as class.
So far i did not see any variation to that.
There was a problem hiding this comment.
@Rejoice472 This is known as a bug waiting to happen because of a design choice that has flaws. At the very least there should be a clear explanation in the code explaining what the limitations are and why they might be acceptable as well as opening an issue to track improvement. We really should be delivering robust code to prevent future problems that a developer may spend hours debugging only to reach the same conclusions we are discussing right here.
There was a problem hiding this comment.
@Rejoice472 I have opened eclipse-lsp4jakarta/lsp4jakarta#611 which describes the solution that would be needed to correctly generate legal default values for annotation attributes.
|
@arunvenmany-ibm Please review again. |
|
If any changes are made to this PR they should be synchronized back to the LSP4Jakarta project. |
Sure |
|
4 participants
This is to fix defect #580
The same fixed in lsp4jakarta too
reference - eclipse-lsp4jakarta/lsp4jakarta#581