tip of the day: eclipse save actions

Development
,
1

Hello fans of IDEs doing the repetitive work for us :smile:

Eclipse has Save Actions where you can configure Eclipse to format your java classes according to your formatter settings on every save!! You might say, why do I care we use the maven formatter plugin which does that for me when I build openmrs. Hold your horses! The maven formatter plugin formats the java classes but does not organize your imports and does not remove unused imports! And that’s just one of the niceties!

Check out the docs:

https://wiki.openmrs.org/display/docs/Developer+How-To+Setup+And+Use+Eclipse#DeveloperHow-ToSetupAndUseEclipse-AutomaticallyFormatonSave

Please try it and report back! Tell us if this is something you think is useful or if its just interfering with your dev workflow! Also you might have an idea on how to improve the settings I showed in the docs. Help us make the OpenMRS development journey easier. Lets get the boring stuff out of the way :sunny:

2 Likes
2

Hey @teleivo i tried it out immediately when i got notified of your wiki page edit. It is just awesome! :smile:

1 Like
3

An example of code very unreadable:

2017-03-07 23_11_13-workspace - Java - openmrs-api_src_main_java_org_openmrs_api_db_hibernate_Hibern.png889×182 9.95 KB

I don’t think the formatter or any of the tools can fix this. We should have specific rules to properly format this e.g.

The problem is that the Formatter destroys the layout again :tired_face:

1 Like
4

I know but thats another story described here https://issues.openmrs.org/browse/TRUNK-5095 with solution :wink:

2 Likes
5

@teleivo awesome though it is, this auto formatting on save just introduced a bug which is a null pointer exception. To reproduce, change this line https://github.com/dkayiwa/openmrs-module-xreports/blob/master/omod/src/main/java/org/openmrs/module/xreports/web/controller/FillParameterFormController.java#L259 by removing " + id + group" and save.

How did the auto formatting introduce the NPE? By automatically changing this line https://github.com/dkayiwa/openmrs-module-xreports/blob/master/omod/src/main/java/org/openmrs/module/xreports/web/controller/FillParameterFormController.java#L254 to these two below: report.getGroup(); report.getGroup().getGroupId();

And as you can see, there is no longer a null check on report.getGroup() before calling report.getGroup().getGroupId()

6

I don’t get it.

Are you saying that this line:

String group = report.getGroup() != null ? “&groupId=” + report.getGroup().getGroupId() : “”;

Was replaced by these?

report.getGroup();
report.getGroup().getGroupId();
7

am also confused.

  • what did you configure in the save actions?
8

Yes that is exactly what happened.

9

@teleivo i only did the tips you shared and no more. :slight_smile:

10

this is not only a formatting change, can you share a picture of the “Additional Actions” you configured?

11

That’s weird. If you remove the use of the group variable, Eclipse will either remove the variable or the whole assignment. If the assignment has collateral effects (like forcing Hibernate to hydrate the object) then removing the sentence is dangerous. We should just remove the variable, not the assignment.

12

screen1.png1502×1070 133 KB
screen3.png1776×1254 97.8 KB
screen2.png1774×1254 115 KB

Attached the the windows with selections. The rest of the tabs under Additional Save actions do not have a single selection.

Did you fail to reproduce it in eclipse?

13

I can’t reproduce it, I’m using Eclipse Neon.2 on Windows.

2017-03-23 22_56_23-workspace - Java - xreports-omod_src_main_java_org_openmrs_module_xreports_web_c.png916×142 12.5 KB

Maybe it’s a bug, the group variable should be removed when saving. Ctrl+Shift+O doesn’t work either. :disappointed:

14

What do you have in the else clause?

15

Same as you (I downloaded your repo), I just added line 255.

16

I just found that saving RequiredDataAdviceTest removes getters from inner classes and tests fail. The problem is that these methods are called by reflection, and Eclipse thinks they’re not used. Not sure how to get around that :astonished:

Making the inner classes public perhaps? The inner class is private but its getters are public, it’s just weird.

17

If the “remove unused variables” save action doesnt behave well, then just disable it.

18

Yes, it would be the safest option.

19

i got it disabled on trial when it was adding two empty lines between my class declarations and their first line of class blocks as well as some formats i disliked personally

20

in the latest posts we were not talking about formatting but cleanups like removing unused local variables.

I think what you mention is the formatting we defined in our openmrs formatter.xml. if you disable formatting means that you write code that doesnt comply with the formatter. which when someone else changes files formerly touched by you and uses the formatter will have this in his diff.