Projects:Java Sources Formatting/Specs

Contents 1 Background 2 Problems 3 Goals 3.1 Get rid of Luna formatter 3.2 Review formatting rules 3.2.1 Enable Off/On Tags 3.2.2 Wrap qualified function calls 3.2.3 Add a new line at the end of the file 3.2.4 Indent statements within switch bodies 3.2.5 Enum constant declaration each on a new line 3.2.6 Convert control statement bodies to block 3.2.7 Add missing @Override and @Deprecated annotations

Background

All Openbravo developers must use a common format style for Java sources. To ensure a common baseline, some of those rules are enforced by auto-formatting rules in Eclipse IDE. Those rules were defined in 2009 and never changed afterward.

Starting from Eclipse Mars, released in 2015, formatter code was reimplemented not being possible to keep the previous format using standard formatter. For this reason, instead of using standard formatter, we continue using Luna formatter.

Problems

Old Luna formatter is built on top of an API that is not guaranteed to be kept stable. Therefore the formatter can be broken on any new Eclipse release, which in fact, happened in 2018-12.

Being that old, Luna formatter does not handle properly new Java constructs.

Goals

Get rid of Luna formatter

The main motivation is to start using standard Eclipse formatter to get rid of old Luna formatter due to the problems explained above.

Review formatting rules

As it is not possible to keep exactly the same formatting with the standard formatter, many Java sources will be (slightly) modified. Together with these changes, we are reviewing current rules to check if some of them should be modified or new ones added.

The objective is to reduce as much as possible the number of changes while keeping a useful format.

After some discussion, these are the proposed changes:

Enable Off/On Tags

In this way, we'll be able to have some chunks of manually formatted code. So, for example, we won't need those tailing comments to preserve desired line breaks.

Before:

 
        String hql = "" + //
            "select count(distinct c.id) " + //
            "from BusinessPartnerLocation bpl " + //
            "inner join bpl.locationAddress as ad " + //
            "inner join ad.country as c " + //
            "where bpl.businessPartner.id = :bPartnerId " + //
            "and bpl.taxLocation = true " + //
            "and c.id <> :countryId "; //

After:

 
        // @formatter:off
        String hql =  "select count(distinct c.id) " +
                      "from BusinessPartnerLocation bpl " +
                      "inner join bpl.locationAddress as ad " +
                      "inner join ad.country as c " + 
                      "where bpl.businessPartner.id = :bPartnerId " +
                      "and bpl.taxLocation = true " +
                      "and c.id <> :countryId ";
        // @formatter:on

Wrap qualified function calls

Wrap all elements except the first one. The reason for doing so is to make streams much more readable.

Before:

 
    long total = qry.stream().collect(groupingBy(AlertRule::getFilterClause)).values().stream()
        .mapToLong(rulesByFilterClause -> countActiveAlertsForRules(rulesByFilterClause, vars))
        .sum();

After:

 
    long total = qry.stream()
        .collect(groupingBy(AlertRule::getFilterClause))
        .values()
        .stream()
        .mapToLong(rulesByFilterClause -> countActiveAlertsForRules(rulesByFilterClause, vars))
        .sum();

Note this rule doesn't apply to invocation on objects, so this case ramains unchanged: Selector sel = OBDal.getInstance().get(Selector.class, selectorId);

Add a new line at the end of the file

To follow standards, not very important but it's nice if you want to cat a file.

Indent statements within switch bodies

Before:

 
    switch (sortKey) {
    case "logger":
      return Comparator.comparing(AbstractLogger::getName);
    case "level":
      return Comparator.comparing(Logger::getLevel);
    default:
      throw new IllegalArgumentException("Unkown field to sort " + sortKey);
    }

After:

 
    switch (sortKey) {
      case "logger":
        return Comparator.comparing(AbstractLogger::getName);
      case "level":
        return Comparator.comparing(Logger::getLevel);
      default:
        throw new IllegalArgumentException("Unkown field to sort " + sortKey);
    }

Enum constant declaration each on a new line

Before:

 
public enum SubscriptionStatus {
    COMMUNITY("COM"), ACTIVE("ACT"), CANCEL("CAN"), EXPIRED("EXP"), NO_ACTIVE_YET("NAY"), INVALID("INV");
    ...

After:

 
public enum SubscriptionStatus {
    COMMUNITY("COM"),
    ACTIVE("ACT"),
    CANCEL("CAN"),
    EXPIRED("EXP"),
    NO_ACTIVE_YET("NAY"),
    INVALID("INV");
    ...

Convert control statement bodies to block

From now on control statement bodies will be forced to be a block:

Before this was allowed:

 
  if (condition)
    return;

After, that code will be reformatted to:

 
  if (condition) {
    return;
  }

Add missing @Override and @Deprecated annotations

It is a good practice that now will be enforced.

Before:

 
  public String toString() {
    // ...

Afterwards will be converted to:

 
  @Override
  public String toString() {
    // ...