[java] ImmutableField: False positive with lombok (fixes #4254)#4474
Conversation
adangel
left a comment
There was a problem hiding this comment.
Thanks for the PR.
This is unfortunately not so simple:
- with #4175 we actually deprecated the property. Unfortunately this didn't make it into the pmd/7.0.x branch back then. But end result is: this property shouldn't exist at all anymore.
- since we currently have some other lombok annotations hardcoded in a separate list (see "INVALIDATING_CLASS_ANNOT"), we can maybe do the same for these special cases and add a "INVALIDATING_FIELD_ANNOT" list.
- end result should be - there is no property "ignoredAnnotations" for this rule anymore, since the property is deprecated.
Generated by 🚫 Danger |
|
Hey, thank you for your feedback! Funnily enough, I had initially added the I've now pushed a commit to apply your feedback. However, I have one question regarding the I think that a field (or class) being annotated with the I have added the annotation to |
adangel
left a comment
There was a problem hiding this comment.
Thanks for the update, looks good. I'll merge it tomorrow.
However, I have one question regarding the
@Getterannotation.I think that a field (or class) being annotated with the
@Getter
annotation should not be a reason to exempt from the ImmutableField
rule. After all, it only generates a getter, meaning the field can
still be marked final.I have added the annotation to INVALIDATING_FIELD_ANNOT because it is
also in INVALIDATING_CLASS_ANNOT, ensuring the behavior is consistent
for the annotation regardless of its location in the source.
I'd keep it for now as it is to be consistent.
I think, you are right, that a field, that's only annotated with @Getter could be made final - but in real code, there probably a bit more ongoing with that field (e.g. some other method will set it to some value) - and then it can't be immutable anyway. A field, that is only annotated with @Getter and has no write access anywhere is useless, as it will always return the initial value or null - but true, in that case, it could be final...
Describe the PR
This PR fixes a false positive for the ImmutableField rule when it is annotated with Lombok's
@Setterannotation, which adds a generated setter for that specific field to the class.Previously, the setter annotation (and similar ones) were only supported when used on the class level.
Related issues
Ready?
./mvnw clean verifypasses (checked automatically by github actions)