google / google-java-format

Reformats Java source code to comply with Google Java Style.
Other
5.63k stars 855 forks source link

Block-like array initializers are not preserved #572

Open makinako opened 3 years ago

makinako commented 3 years ago

The Google Java Style Guide specifies that arrays may be formatted in a block-like fashion, however google-java-format clobbers this to one line per element.

For example, the following:

protected static final byte[] TEST_ARRAY =
      new byte[] {
        (byte) 0x31, (byte) 0x32, (byte) 0x33, (byte) 0x34, (byte) 0x35, (byte) 0x36, (byte) 0xFF, (byte) 0xFF
      };

becomes

  protected static final byte[] TEST_ARRAY =
      new byte[] {
        (byte) 0x31,
        (byte) 0x32,
        (byte) 0x33,
        (byte) 0x34,
        (byte) 0x35,
        (byte) 0x36,
        (byte) 0xFF,
        (byte) 0xFF
      };

For larger formatted constant definitions, formatting is essential to preserving understanding of the structure. It seems like preserving formatting should either be default behaviour, or an option.

cushon commented 3 years ago

The original formatting exceeds the column limit, and the formatter doesn't introduce block or grid-like layouts, but if you manually reformat that into a grid the formatter will preserve that grid-like layout when it runs.

$ google-java-format T.java
class T {
  protected static final byte[] TEST_ARRAY = {
    (byte) 0x31, (byte) 0x32, (byte) 0x33, (byte) 0x34,
    (byte) 0x35, (byte) 0x36, (byte) 0xFF, (byte) 0xFF
  };
}

(This is one of the few places the formatter preserves existing formatting decisions, instead of ignoring existing whitespace when making formatting decisions.)

makinako commented 3 years ago

Yes in that case that makes total sense! There seems to be a bit more to this though. When I looked at what was happening in my code, I saw that some of my arrays (that were <100 chars) were indented by a mix of tabs and spaces, not just spaces.

See the following:

public class Main {

  protected static final byte[] TEST_ARRAY_TABS =
    new byte[] {
        (byte) 0x31, (byte) 0x32, (byte) 0x33, (byte) 0x34, 
        (byte) 0x35, (byte) 0x36, (byte) 0xFF, (byte) 0xFF
      };

  protected static final byte[] TEST_ARRAY_SPACES =
    new byte[] {
      (byte) 0x31, (byte) 0x32, (byte) 0x33, (byte) 0x34, 
      (byte) 0x35, (byte) 0x36, (byte) 0xFF, (byte) 0xFF
      };

  protected static final byte[] TEST_ARRAY_TABS_AND_SPACES =
    new byte[] {
        (byte) 0x31, (byte) 0x32, (byte) 0x33, (byte) 0x34, 
    (byte) 0x35, (byte) 0x36, (byte) 0xFF, (byte) 0xFF
      };

}

The first two remain intact, the third reformats to one-per-line. This makes it easy to avoid the problem, but I suspect the expected behaviour should be that the tabs are converted to spaces and the block formatting is preserved in the third case.

DidierLoiseau commented 3 months ago

I have noticed similar weird behavior with method arguments as well, for example the following formatting is preserved:

    String generatedOn1 = "test";
    var myString =
        String.valueOf(
            Map.of(
                "id", "123456",
                "title", "Report 1",
                "reportType", REPORT_TYPE,
                "status", "NEW",
                "generatedOn", generatedOn1,
                "generatedBy", "USER X"));

but if you replace "NEW" by a constant (NEW), then it gets reformatted to one-per-line:

    String generatedOn1 = "test";
    var myString =
        String.valueOf(
            Map.of(
                "id",
                "123456",
                "title",
                "Report 1",
                "reportType",
                REPORT_TYPE,
                "status",
                NEW,
                "generatedOn",
                generatedOn1,
                "generatedBy",
                "USER X"));

It will not happen, however, if you inline generatedOn1, or if you add another key/value pair at the end.

It is very annoying because a simple change like removing a Map.of() entry can change the whole formatting, which defeats the purpose of automatic formatting.

I don’t know whether a separate issue should be created for this one since it is not array formatting, but it seems to be the same problem.

Moreover it is unclear for me where this feature is specified for methods arguments, and what rules apply. I only found that it is mentioned in #299.