google / google-java-format

Reformats Java source code to comply with Google Java Style.
Other
5.49k stars 848 forks source link

Inconsistent formatting of text blocks in 1.21.0 #1087

Open bmeier-pros opened 3 months ago

bmeier-pros commented 3 months ago

I have a file that consists mostly of constant strings written as text blocks. When I apply the formatter to the file, the text blocks are inconsistently indented, with some being completely left justified, and others being shifted underneath the opening """. There doesn't seem to be any particular rationale for which blocks get justified, but it is consistent. I'm trying to generate a sanitized reproduction currently.

Stephan202 commented 3 months ago

Hey @bmeier-pros! Does your issue match what's described in #1081?

bmeier-pros commented 3 months ago

@Stephan202 - not completely, I saw that issue as well, but these are just bare static final string constants.

For example:

    public static final String JSON_1 =
        """
        [
          "FUNCTION1",
          "FUNCTION2",
          "// ... 98 other records ..."
          "FUNCTION101"
        ]""";

    public static final String JSON_2 =
        """
{
  "errorID": "error id",
  "errorMessage": "message",
  "resourceType": "metadata",
  "requestID": "request"
}""";

I'd say it was something about the JSON curly brackets, but it's not consistent. I'm trying to find a truly minimal reproduction.

cushon commented 3 months ago

I suspect the behaviour you're seeing is that there's some logic to start the text block contents at the left margin if indenting the lines would cause the column limit to be exceeded, and to preserve the existing formatting choice if a text block is aligned with the left margin.

In that example, adding a least one space of indent to the second text block will cause the formatter to indent it the same way as the first text block.

The formatting of text blocks hasn't been finalized yet, we might revisit some of those details, in particular whether or not aligning with the left margin is a choice that should be preserved.

https://github.com/google/google-java-format/blob/71a755b4daf0846c867834b5a0d86297ac7d0667/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java#L204-L207

Example:

$ google-java-format T.java
class T {
  String S =
      """
      hello
      world
      """;
}
$ google-java-format T.java
class T {
  String S =
      """
hello
world
""";
}
bmeier-pros commented 3 months ago

Hi @cushon,

That makes some sense, however the original formatting of the two text blocks before applying google-java-format was consistent at 8 spaces indentation for both blocks. This also happened in several other places throughout the file where some blocks were indented and others were justified. It's also consistent - if I modify the indentation of the second block, it gets revised back to left justified. I will try again to make a minimal case :-) I've had a busy couple of days.

bmeier-pros commented 3 months ago

Ok, looked at it a bit more.

I suspect the behaviour you're seeing is that there's some logic to start the text block contents at the left margin if indenting the lines would cause the column limit to be exceeded...

This is the issue, generally. The text blocks in question all have lines exceeding 100 characters when the file is consistently indented. There's one block that seems to be an outlier, but this behavior explains the move of every other block. The gutter in the editor is configured at 120 characters, so that threw me for a bit.

IMHO, the left justification of long lines is unexpected, and I would not do it for text blocks. It makes the left margin of files look very strange if they have text blocks with long lines, and there's lots of situations where long lines are somewhat required, like embedded scripts. In the case of these files, about a third of the blocks are left justified, and the rest are justified with the code. Something to discuss, for sure.

bmeier-pros commented 3 months ago

My preferred formatting for text blocks would be something like:

public static final String STRING = """
    ... Embedded code and scripts possibly with long lines...
    """; // the position of this element defines the rest of the block's indentation
zaneduffield commented 2 months ago

The formatting of text blocks hasn't been finalized yet, we might revisit some of those details, in particular whether or not aligning with the left margin is a choice that should be preserved.

I'm also not a huge fan of pushing the text blocks all the way to the left margin. I would prefer if the line length limit was violated. GJF can't promise that its output will never violate the line length limit anyway (all it takes is an identifier of length 101).

Another option would be to wrap the inner contexts of the text block, like

foo(
  """
  aaaaaaaaaaaaaaaaaaaaaaa\
  a
  """
)