[FOLIO-1667] Agree on Java Import Ordering (and expansion) Policy Created: 20/Dec/18  Updated: 25/Jan/23  Resolved: 25/Jan/23

Status: Closed
Project: FOLIO
Components: None
Affects versions: None
Fix versions: None

Type: Task Priority: P3
Reporter: Marc Johnson Assignee: Unassigned
Resolution: Won't Do Votes: 0
Labels: devdoc, potential-decision
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified

Attachments: XML File Eclipse style imports.xml     PNG File ecf.png     File folio.importorder    
Issue links:
Relates
relates to FOLIO-2127 Decision: Switch to Google Java style... Closed
Sprint:
Development Team: Core: Platform

 Description   

A significant amount of the changes that I review (and make, often by accident) are due to the import statements in Java code being reordered or expanded / contracted due to different settings in various IDEs used.

A while ago, I think Kurt Nordstrom Jeremy Huff and I informally agreed to use Eclipse style (I use IntelliJ so needed to change my configuration) in order to reduce this.

As more people work on these modules, I think we need to formally agree on which style to use, in order to avoid these redundant changes.

Proposed Standard (based upon Eclipse standard)

  • No use of wildcard import statements
  • Static imports in alphabetic order first
  • Then imports in defined then alphabetic order (java, javax, org, com, everything else) with a line break between defined groups


 Comments   
Comment by Khalilah Gambrell [ 20/Dec/18 ]

Marc Johnson - should this feature be moved to another project? Maybe Folio?

Comment by Marc Johnson [ 20/Dec/18 ]

Khalilah Gambrell Sorry, my bad, switching between too many tasks :-/

Comment by Julian Ladisch [ 21/Dec/18 ]

This is Eclipse's default import order (configurable in Java/Code Style/Organize Import):

java
javax
org
com

Eclipse has Java/Editor/Save Actions/Organize Imports disabled by default.

Comment by Marc Johnson [ 28/Dec/18 ]

Julian Ladisch Thanks, that matches what I got from my research.

I believe IntelliJ and maybe NetBeans do re-order them automatically. I didn't realise that Eclipse did not do this by default.

Given this is turned off by default, what ordering scheme do developers using Eclipse use? Adam Dickmeiss Jakub Skoczen do you use an IDE?

I changed my IntelliJ config according to https://stackoverflow.com/questions/14716283/is-it-possible-for-intellij-to-organize-imports-the-same-way-as-in-eclipse which seemed to reduce the conflicts for a time.

As more people contribute changes to modules I've noticed this (and other formatting / style differences) take up significant review time.

I don't know whether there is a recommended standard, so I'm basing my proposal on that question answer.

  • No use of wildcard import statements
  • Static imports in alphabetic order first
  • Then imports in defined then alphabetic order (java, javax, org, com, everything else) with a line break between defined groups

Thoughts?

(I have a ~4000 line pull request to review soon, and at least some of that is ordering of imports, indentation and spacing of statements).

Comment by Julian Ladisch [ 08/Jan/19 ]

I use Eclipse.

When adding a new import line it looks for the longest matching existing import and adds it there.

If the import group (java, javax, org, com) does not exist yet it tries to insert the group at a position that results in that order.

Comment by Julian Ladisch [ 08/Jan/19 ]

What you propose is the ordering that Eclipse since Mars release does. I support your proposal.
A checkstyle configuration for that is on https://checkstyle.org/config_imports.html#ImportOrder
Screenshots how to configure IntelliJ IDEA and NetBeans to use that is on https://github.com/checkstyle/checkstyle/issues/1448

Comment by Marc Johnson [ 17/Jan/19 ]

Thanks Julian Ladisch

I'd like to get this agreed (mostly so I can hopefully stop reviewing pull requests with lots of import ordering changes), so I'm going to ask for lots more peoples thoughts (partly based upon the tech leads defined on the wiki)

Adam Dickmeiss Craig McNally Matt Reno Sobha Duvvuri Dima Tkachenko Pavel Korolenok Taras Spashchenko Kostyantyn Khodarev What do you think to this proposal?

Comment by Matt Reno [ 17/Jan/19 ]

Marc Johnson I use Eclipse and didn't realize other IDEs reorder imports on load or save or whatever. That seems wrong to me and something to disable, but having best practices and checkstyle monitoring for this is a good solution.

Comment by Adam Dickmeiss [ 17/Jan/19 ]

I'm using Netbeans which leaves those imports alone. So I never had this issue.. Pretty lame to reorder them by an IDE IMHO.

Comment by Dima Tkachenko [ 17/Jan/19 ]

Marc Johnson
I've prepared import ordering setting in Eclipse and shared with my team. We use both Eclipse and IntelliJ. But IntelliJ understands Eclipse format settings, so no problem.
Here is out settings:
folio.importorder

This file should be imported into Eclipse Code Formatter (a plugin in IntelliJ):

Comment by Craig McNally [ 18/Jan/19 ]

agreeing upon an order makes sense to me, though I never really paid much attention to the order of imports when reviewing PRs... I figured the build would fail if something was missing, and if sonarqube would complain if there were unused imports.

Comment by Marc Johnson [ 20/Jan/19 ]

Craig McNally I don't pay attention to the order itself. I only care about the order because different ordering creates noise (which slows down an already slow task for me) in pull requests (and redundant changes history), especially in files that are otherwise unchanged.

I'm noticing similar issues for code formatting (brace alignment, long line separation, spaces in expressions etc).

I believe there are warnings from SonarQube for unused imports.

Comment by Marc Johnson [ 20/Jan/19 ]

Dima Tkachenko Thank you for providing these settings (I use IntelliJ and have configured it separately, without the plugin).

Comment by Marc Johnson [ 20/Jan/19 ]

Matt Reno An optional post-commit hook that checked aspects like this, is something David Crossley and I have discussed in the past.

A while ago, I tried using checkstyle, I couldn't get a local run of it to work with RAML Module Builder (which I think is a module with it setup). Are other modules using it? Is it run in the CI?

Comment by Dima Tkachenko [ 21/Jan/19 ]

Thank you for providing these settings (I use IntelliJ and have configured it separately, without the plugin).

Plugin is required if some of your teammates use Eclipse. Thus you can have a single file with the settings and make it shareable or even put it under VCS.
If it's not the case then, yes, settings can be done by IntelliJ standard tools.

Comment by Marc Johnson [ 19/Feb/19 ]

Given that there hasn't been any active objections to this, how do we make this decision and document this policy?

Comment by Adam Dickmeiss [ 19/Feb/19 ]

I tried to object. I think the best approach is tell your IDE not to reorder things.. vi can do that.

Comment by Marc Johnson [ 22/Feb/19 ]

Adam Dickmeiss If we ignore the IDE configuration aspect of this.

Do you object to having a policy, or the policy proposed?

Comment by Matt Reno [ 02/Dec/19 ]

Note: if we use the Google Java style, they have a prescribed import order/format:
https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing

Comment by Marc Johnson [ 02/Dec/19 ]

Matt Reno

if we use the Google Java style, they have a prescribed import order/format

I'm ok with that, as long as we can agree on the standard and the common IDEs can be made to support it

Generated at Thu Feb 08 23:15:01 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.