Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single argumentless annotation exception from rule 4.8.5 not honored #342

Closed
JakeWharton opened this issue Feb 6, 2019 · 4 comments
Closed

Comments

@JakeWharton
Copy link
Contributor

https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations states:

Exception: A single parameterless annotation may instead appear together with the first line of the signature, for example:

@Override public int hashCode() { ... }

Given code like

@Test public void stuff() {
  // ...
}

and

@Override public void run() {
  // ...
}

GJF formats them like

+@Test
+public void stuff() {
-@Test public void stuff() {
   // ...
 }

and

+@Override
+public void run() {
-@Override public void run() {
   // ...
 }

in contradiction to this exception.

While I understand why you might not want to format the own-line style into the single-line style, changing the single-line style to the own-line style when it's already in compliance with the rules seems erroneous.

I'm observing this with version 1.7.

$ cat Test.before | java -jar google-java-format-1.7-all-deps.jar - > Test.after && diff -u Test.before Test.after
--- Test.before	2019-02-06 13:19:29.000000000 -0500
+++ Test.after	2019-02-06 13:21:33.000000000 -0500
@@ -3,7 +3,8 @@
 import org.junit.Test;

 class Test {
-  @Test public void stuff() {
+  @Test
+  public void stuff() {
     System.out.println("hi");
   }
 }
@JakeWharton
Copy link
Contributor Author

Also the latest SNAPSHOT build.

$ cat Test.before | java -jar google-java-format-1.8-20190109.235157-1-all-deps.jar - > Test.after && diff -u Test.before Test.after
--- Test.before	2019-02-06 13:19:29.000000000 -0500
+++ Test.after	2019-02-06 13:23:30.000000000 -0500
@@ -3,7 +3,8 @@
 import org.junit.Test;

 class Test {
-  @Test public void stuff() {
+  @Test
+  public void stuff() {
     System.out.println("hi");
   }
 }

@cushon
Copy link
Collaborator

cushon commented Feb 6, 2019

While I understand why you might not want to format the own-line style into the single-line style, changing the single-line style to the own-line style when it's already in compliance with the rules seems erroneous.

This comment is related: #318 (comment). The formatter generally doesn't factor the existing formatting in to its decisions.

I think the key word in 4.8.5 is may. Both approaches are allowed by the style guide, and the formatter has to make a decision, and in general it prioritizes making a single consistent decision over preserving existing formatting choices.

@JakeWharton
Copy link
Contributor Author

Yeah that's fair. And I guess detection of this case just isn't worth it?

For me, the pervasiveness of @Test and @Override as the sole annotation on a methods makes me want this optimization because of the vertical space it otherwise steals.

@cushon
Copy link
Collaborator

cushon commented Feb 6, 2019

(I just cc'd you on a related internal bug.)

I think it's still up for debate whether we'd prefer to always leave annotations like @Test or @Override on the same line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants