-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fix incorrect code generated by the -Xpropertyccessors option #1286
base: master
Are you sure you want to change the base?
Conversation
I signed off the agreement, how do I recheck? |
You can go to https://www.eclipse.org/legal/ECA.php and click "Validation tool" in the nav panel. Have you --signoff(ed) the commit? |
Can you please add a test for the "accessors" plugin execution asserting new added code feautures? |
JFieldVar fld = flds.next(); | ||
String fldName = fld.name(); | ||
Set<JAnnotationUse> annotsCopy = new HashSet<JAnnotationUse>(fld.annotations()); | ||
String getterName = "get" + fldName.substring(0, 1).toUpperCase() + fldName.substring(1); |
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.
Can there be "is" instead of "get" for booleans?
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.
There probably could, but note that this is not what the plugin does. The plugin doesn't generate the getters, it merely finds the appropriate ones to insert the annotations. If you want to change the generated names, you should probably direct that to the maintainer of the code that actually generates the getters.
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.
You won't find appropriate getters if you are searching by the wrong name. Consider following property:
private boolean positive;
public boolean isPositive();
public setPositive(boolean positive);
In that case it will skip moving annotations from field to method.
getterMethod.annotate(annot); | ||
} | ||
String properGetterName = "get" + fldName.substring(0, 1).toUpperCase() + fldName.substring(1); | ||
if (!properGetterName.equals(getterName)) { |
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.
What is the reason for normalizing names? When does it happen that field / getter does not match by javabean convention?
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.
Again, I do not know. I guess this has something to do with the convention that Java class property names are lowercase, so uppercase-named elements are reduced to lowercase properties and then the getter gets the appropriate name by convention.
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.
My point is not about javabean convention, it is why there is that particular code. First property name is read from field name. Second property name is reread from XmlElement("name") value (if present). Than method is looked up by this name, and if found, name is recalculated according to field name and rewritten. Lets consider following example:
@XmlElement(name = "abc")
private Long id;
public getId();
public void setId(Long id);
- getterName is first set to "getId" by the field name + getPrefix
- getterName is reset to
name
fromXmlElement
annotation and is nowgetabc
(uppercasing 'a' looks to be omitted here). - next we are looking for
getabc
which is not present in the class and the code actually exits without doing anyting forid
field.
Following will happen after, only when XmlElement(name) is not set or is set to the field name:
- Found getter by
getId
name - Move annotations from field to getter
- Recalculate
properGetterName
by the field name togetId
and overwrite method name toproperGetterName
(same for setter)
Either, I don't understand what the code is doing, which is quite possible due to missing tests to prove it, or the code is working only partially and needs revisiting.
Signed-off-by: pwilkin <piotr.wilkin@syndatis.com>
Still can't seem to get the validation to work, my ECA checks fine in the validation tool, I've added a signed-off-by, but apparently the validation tool doesn't like it... |
Your first commit is not signed off. That is probably the reason validation is still failing. According to Eclipse ECA validation your email address has an ECA record. |
Fixes #1285