formatAnnotations() contradicts Google Java style

[edysli]

I'm using formatAnnotations() together with googleJavaFormat() and introducing the former also brought surprising changes. Type annotations on class and method declarations are affected:

-@Value
-@Jacksonized
+@Value @Jacksonized
 @Builder
 public class Example {

   @Bean
-  @Qualifier("example")
-  RestTemplate example() {
+  @Qualifier("example") RestTemplate example() {

However, Google's Java Style Guide, section "4.8.5 Annotations" says that

Annotations applying to a class appear immediately after the documentation block, and each annotation is listed on a line of its own (that is, one annotation per line).

and the same applies to method and constructor annotations, without mentioning any exception for type annotations.

Moreover, the claim that "Type annotations should be on the same line as the type that they qualify." in Spotless Gradle plugin's README doesn't cite any source.

So which of these formatters is right?

My Spotless configuration for Java:

spotless {
    encoding("UTF-8")
    java {
        target("src/*/java/**/*.java")
        targetExclude("**/build/")
        importOrder()
        removeUnusedImports()
        googleJavaFormat()
        formatAnnotations()
    }
}

Spotless 6.11.0 with Gradle 7.5.1

[nedtwigg]

This step was added in the PR below, might be some answers there. In particular, from the Google Java Style Guide

Type-use annotations appear immediately before the annotated type. An annotation is a type-use annotation if it is meta-annotated with @target(ElementType.TYPE_USE). Example

• formatAnnotations() removes line breaks between type annotations and types #1275

The current behavior of formatAnnotations has been released, so if there's any changes to it they will probably need to be gated behind a flag, e.g. .mode(Google) or something like that.

@mernst I'm happy to give you ownership of the formatAnnotations step if you want it, otherwise I'll likely merge any backward-compatible features that users care to contribute.

[edysli]

The text of the Google Java Style Guide is a bit unclear about whether its type-use annotations rule (4.8.5.1) also applies to classes and methods (4.8.5.2 and 4.8.5.3). There is an example though public @Nullable Person getPersonByName(String name); that shows how method declarations are affected, so my second example in the OP is actually a valid change.

As far as I can see, PR #1275 did not introduce any test case with type-use annotations on a class, so it's hard to guess @mernst's intent here. 😉

[mernst]

Thanks for bringing up this issue!

Type annotations on method declarations are affected:

   @Bean
-  @Qualifier("example")
-  RestTemplate example() {
+  @Qualifier("example") RestTemplate example() {

There is no such thing as a type annotation on a method declaration. In your example, @Qualifier modifies the type RestTemplate (it does not modify the method example), so the formatting is correct.

With regard to class annotations, you are right that style guidelines are not explicit. The preferred style is:

@DeclarationAnnotation1
@DeclarationAnnotation2
public @TypeAnnotation1 @TypeAnnotation2 class MyClass {
  ...
}

Note that this is similar to the case for methods. The preferred style for methods is:

@DeclarationAnnotation1
@DeclarationAnnotation2
public @TypeAnnotation1 @TypeAnnotation2 ReturnType myMethod {
  ...
}

If you write the annotations in the correct places (before/after modifiers such as public), Spotless enforces the correct formatting.

As noted at https://github.com/diffplug/spotless/blob/main/plugin-gradle/README.md#formatAnnotations, Spotless does not currently reorder annotations. It does not move annotations around modifiers. Spotless also does not reorder annotations of a particular variety (say, putting all declaration annotations in a canonical order, such as alphabetical order.
These could be enhancement requests.

As you point out, the Spotless test suite lacks test cases for the dispreferred style, and it entirely lacks test cases for annotations on class declarations. I'll add some at #1368.

Moreover, the claim that "Type annotations should be on the same line as the type that they qualify." in Spotless Gradle plugin's README doesn't cite any source.

The reason is that the qualified type is, in your example, @Qualifier RestTemplate. That is a logical unit. Breaking that logical unit across lines is misleading. For instance, it may give a reader the impression that the annotation qualifies the method.

Here are some code examples from Oracle showing the preferred style:

https://docs.oracle.com/javase/tutorial/java/annotations/basics.html
https://docs.oracle.com/javase/tutorial/java/annotations/type_annotations.html

Here are citations from Google:

https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations
google/google-java-format#5

Note that the GJF manual incorrectly uses the name "type-use annotation" when it should use the standard term "type annotation".

Here is one from the Checker Framework:

https://checkerframework.org/manual/#declaration-annotations-moved

I can also be considered an authority. I designed Java's type annotations feature, and the javac implementation is by a group of people including me.

[edysli]

Thank you very much for the explanation @mernst ! 😄 I was indeed thrown off by this sentence from the Google Java Style Guide:

Type-use annotations appear immediately before the annotated type. An annotation is a type-use annotation if it is meta-annotated with @Target(ElementType.TYPE_USE).

Now, I get it.

I can also be considered an authority. I designed Java's type annotations feature, and the javac implementation is by a group of people including me.

Oops sorry! 🙇‍♂️ I don't know you.

[mernst]

@edysli No problem! Thanks again for bringing up the issue.