Method parameter with many annotations gets unintuitive indentation

[anthonyvdotbe]

The first annotation is indented as expected, but any additional annotations and the method parameter itself get an additional indent. I'd expect all lines to have the same indentation.

To reproduce, format:

import io.swagger.v3.oas.annotations.Parameter;
import org.springframework.web.bind.annotation.RequestParam;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;
import java.util.Locale;

class Test {

    void foo(@NotNull @Parameter(name = "Indicates the language of the label to search", required = true) @Valid @RequestParam(value = "language", required = true) Locale language) {}
}

Actual (using the native executable built from 61eabd62d6a461c27621ebc77240345e40fb60b4):

import io.swagger.v3.oas.annotations.Parameter;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;
import java.util.Locale;
import org.springframework.web.bind.annotation.RequestParam;

class Test {

  void foo(
      @NotNull
          @Parameter(name = "Indicates the language of the label to search", required = true)
          @Valid
          @RequestParam(value = "language", required = true)
          Locale language) {}
}

Expected:

import io.swagger.v3.oas.annotations.Parameter;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;
import java.util.Locale;
import org.springframework.web.bind.annotation.RequestParam;

class Test {

  void foo(
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale language) {}
}

[cushon]

Thanks, I agree the example with one parameter can be surprising. The rationale for the indentation is to make it easier to scan signatures with multiple parameters, e.g.:

  void foo(
      @NotNull
          @Parameter(name = "Indicates the language of the label to search", required = true)
          @Valid
          @RequestParam(value = "language", required = true)
          Locale language,
      @NotNull
          @Parameter(name = "Indicates the language of the label to search", required = true)
          @Valid
          @RequestParam(value = "language", required = true)
          Locale bar) {}

vs.

  void foo(
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale language,
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale bar) {}

[anthonyvdotbe]

Thanks for explaining the rationale. I think what I find unintuitive about it, is the fact that not all annotations have the same indentation; it seems like @NotNull is special in some way. So I would prefer (in order of preference):

to format without any extra indentation:

  void foo(
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale language,
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale bar) {}

to insert a blank line:

  void foo(
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale language,

      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale bar) {}

to only indent the parameter:

  void foo(
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
          Locale language,
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
          Locale bar) {}

Inserting a blank line seems like a good compromise between both points of view.

What do you think?

[cushon]

One drawback of only indenting the type is that if the annotations are all at the same indent, it's hard to scan to see how many parameters there are, especially if some of the parameters are short enough to fit on a single line.

I think the style guide allows the blank line approach (https://google.github.io/styleguide/javaguide.html#s4.6.1-vertical-whitespace says a blank line may appear 'anywhere it improves readability'). It seems a little surprising to me, most of the other blank lines that are required/permitted by the formatter are between statements.

[cushon]

[For posterity I dug up some discussion in b/257119244 which discussed some of the same options, and was leaning towards leaving the current formatting as-is]

[anthonyvdotbe]

A final option with equally-indented annotations is the following. This allows quick scanning and applies the idea of "the less important it is, the more we indent it":

  void foo(
        @NotNull
        @Parameter(name = "Indicates the language of the label to search", required = true)
        @Valid
        @RequestParam(value = "language", required = true)
    Locale language,
        @NotNull
        @Parameter(name = "Indicates the language of the label to search", required = true)
        @Valid
        @RequestParam(value = "language", required = true)
    Locale bar) {}

Or maybe gjf shouldn't do anything special here, and have authors add Javadoc and/or a comment like the following if they want to allow readers to quickly see the parameter list? The advantage of (Javadoc) comments is that all parameters will "always" fit on a single screen, whereas with the actual parameter list it's quite possible that you'd have to scroll to see all parameters.

  void foo(// Locale language, Locale bar
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale language,
      @NotNull
      @Parameter(name = "Indicates the language of the label to search", required = true)
      @Valid
      @RequestParam(value = "language", required = true)
      Locale bar) {}