Code clean-up, formatting, docs, TEMPORARY supress warnings

Author: benhalasi

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java

@@ -38,14 +38,18 @@
 import java.util.stream.Stream;
 import javax.lang.model.element.Modifier;
 
+/** A {@link BugChecker} that flags classes with non-standard member ordering. */
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "Members should be ordered in a standard way.",
+    explanation =
+        "Members should be ordered in a standard way, which is: "
+            + "static member variables, non-static member variables, constructors, methods.",
     link = BUG_PATTERNS_BASE_URL + "MemberOrdering",
     linkType = CUSTOM,
     severity = WARNING,
     tags = STYLE)
-public class MemberOrdering extends BugChecker implements BugChecker.ClassTreeMatcher {
+public final class MemberOrdering extends BugChecker implements BugChecker.ClassTreeMatcher {
   private static final long serialVersionUID = 1L;
   /** A comparator that sorts members, constructors and methods in a standard order. */
   private static final Comparator<Tree> COMPARATOR =
@@ -56,8 +60,9 @@ public class MemberOrdering extends BugChecker implements BugChecker.ClassTreeMa
                     return 0;
                   case METHOD:
                     return 1;
+                  default:
+                    throw new IllegalStateException("Unexpected kind: " + memberTree.getKind());
                 }
-                throw new IllegalStateException("Unexpected kind: " + memberTree.getKind());
               })
           .thenComparing(
               (Tree memberTree) -> {
@@ -66,13 +71,16 @@ public class MemberOrdering extends BugChecker implements BugChecker.ClassTreeMa
                     return isStatic((JCVariableDecl) memberTree) ? 0 : 1;
                   case METHOD:
                     return isConstructor((JCMethodDecl) memberTree) ? 0 : 1;
+                  default:
+                    throw new IllegalStateException("Unexpected kind: " + memberTree.getKind());
                 }
-                throw new IllegalStateException("Unexpected kind: " + memberTree.getKind());
               });
-  // XXX: Evaluate alternative implementation.
+
+  // todo: Evaluate alternative implementation.
   /** A comparator that sorts members, constructors and methods in a standard order. */
+  @SuppressWarnings("unused")
   private static final Comparator<Tree> SQUASHED_COMPARATOR =
-      Comparator.comparing(
+      comparing(
           (Tree memberTree) -> {
             if (memberTree.getKind() == VARIABLE) {
               if (isStatic((JCVariableDecl) memberTree)) {
@@ -96,9 +104,7 @@ public class MemberOrdering extends BugChecker implements BugChecker.ClassTreeMa
           });
 
   /** Instantiates a new {@link MemberOrdering} instance. */
-  public MemberOrdering() {
-    super();
-  }
+  public MemberOrdering() {}
 
   @Override
   public Description matchClass(ClassTree tree, VisitorState state) {
@@ -134,7 +140,7 @@ private static SuggestedFix swapMembersIncludingComments(
     for (int i = 0; i < members.size(); i++) {
       Tree original = members.get(i);
       Tree correct = sortedMembers.get(i);
-      // XXX: Technically not necessary, but avoids redundant replacements.
+      // xxx: Technically not necessary, but avoids redundant replacements.
       if (!original.equals(correct)) {
         var replacement =
             Stream.concat(
@@ -167,7 +173,7 @@ private static boolean isConstructor(JCMethodDecl methodDecl) {
     return getSymbol(methodDecl).isConstructor();
   }
 
-  public static SuggestedFix replaceIncludingComments(
+  private static SuggestedFix replaceIncludingComments(
       ClassTree classTree, Tree member, String replacement, VisitorState state) {
     Optional<Tree> previousMember = getPreviousMember(member, classTree);
     ImmutableList<ErrorProneToken> tokens =

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MemberOrderingTest.java

@@ -21,7 +21,6 @@ void identification() {
                     "Members, constructors and methods should follow standard ordering.")))
         .addSourceLines(
             "A.java",
-            "",
             "// BUG: Diagnostic matches: MemberOrdering",
             "class A {",
             "  char a = 'a';",
@@ -30,22 +29,23 @@ void identification() {
             "",
             "  void m2() {}",
             "",
-            "  public A () {}",
+            "  public A() {}",
             "",
             "  private static String BAR = \"bar\";",
             "  char b = 'b';",
             "",
             "  void m1() {",
             "    System.out.println(\"foo\");",
             "  }",
+            "",
             "  static int TWO = 2;",
             "",
             "  class Inner {}",
+            "",
             "  static class StaticInner {}",
             "}")
         .addSourceLines(
             "B.java",
-            "",
             "class B {",
             "  private static String FOO = \"foo\";",
             "  static int ONE = 1;",
@@ -56,14 +56,17 @@ void identification() {
             "  char a = 'a';",
             "",
             "  char b = 'b';",
-            "  public B () {}",
+            "",
+            "  public B() {}",
             "",
             "  void m1() {",
             "    System.out.println(\"foo\");",
             "  }",
+            "",
             "  void m2() {}",
             "",
             "  class Inner {}",
+            "",
             "  static class StaticInner {}",
             "}")
         .doTest();
@@ -74,7 +77,6 @@ void replacementFirstSuggestedFix() {
     BugCheckerRefactoringTestHelper.newInstance(MemberOrdering.class, getClass())
         .addInputLines(
             "A.java",
-            "",
             "class A {",
             "  private static final int X = 1;",
             "  char a = 'a';",
@@ -83,14 +85,15 @@ void replacementFirstSuggestedFix() {
             "",
             "  void m2() {}",
             "",
-            "  public A () {}",
+            "  public A() {}",
             "",
             "  private static String BAR = \"bar\";",
             "  char b = 'b';",
             "",
             "  void m1() {",
             "    System.out.println(\"foo\");",
             "  }",
+            "",
             "  static int TWO = 2;",
             "",
             "  class Inner {}",
@@ -99,7 +102,6 @@ void replacementFirstSuggestedFix() {
             "}")
         .addOutputLines(
             "A.java",
-            "",
             "@SuppressWarnings(\"MemberOrdering\")",
             "class A {",
             "  private static final int X = 1;",
@@ -109,14 +111,15 @@ void replacementFirstSuggestedFix() {
             "",
             "  void m2() {}",
             "",
-            "  public A () {}",
+            "  public A() {}",
             "",
             "  private static String BAR = \"bar\";",
             "  char b = 'b';",
             "",
             "  void m1() {",
             "    System.out.println(\"foo\");",
             "  }",
+            "",
             "  static int TWO = 2;",
             "",
             "  class Inner {}",
@@ -126,6 +129,7 @@ void replacementFirstSuggestedFix() {
         .doTest(TestMode.TEXT_MATCH);
   }
 
+  @SuppressWarnings("ErrorProneTestHelperSourceFormat")
   @Test
   void replacementSecondSuggestedFix() {
     BugCheckerRefactoringTestHelper.newInstance(MemberOrdering.class, getClass())
@@ -152,12 +156,10 @@ void replacementSecondSuggestedFix() {
             "  static int TWO = 2;",
             "",
             "  class Inner {}",
-            "",
             "  static class StaticInner {}",
             "}")
         .addOutputLines(
             "A.java",
-            "",
             "class A {",
             "  private static final int X = 1;",
             "  private static String FOO = \"foo\";",
@@ -177,12 +179,12 @@ void replacementSecondSuggestedFix() {
             "  }",
             "",
             "  class Inner {}",
-            "",
             "  static class StaticInner {}",
             "}")
         .doTest(TestMode.TEXT_MATCH);
   }
 
+  @SuppressWarnings("ErrorProneTestHelperSourceFormat")
   @Test
   void replacementSecondSuggestedFixWithDefaultConstructor() {
     BugCheckerRefactoringTestHelper.newInstance(MemberOrdering.class, getClass())
@@ -208,32 +210,29 @@ void replacementSecondSuggestedFixWithDefaultConstructor() {
         .doTest(TestMode.TEXT_MATCH);
   }
 
+  @SuppressWarnings("ErrorProneTestHelperSourceFormat")
   @Test
   void replacementSecondSuggestedFixWithComments() {
     BugCheckerRefactoringTestHelper.newInstance(MemberOrdering.class, getClass())
         .setFixChooser(SECOND)
         .addInputLines(
             "A.java",
-            "",
             "class A {",
             "  // `m1()` comment.",
             "  void m1() {",
             "    // Print line 'foo' to stdout.",
             "    System.out.println(\"foo\");",
             "  }",
-            "",
             "  // foo",
             "  /** Instantiates a new {@link A} instance. */",
             "  public A () {}",
             "}")
         .addOutputLines(
             "A.java",
-            "",
             "class A {",
             "  // foo",
             "  /** Instantiates a new {@link A} instance. */",
             "  public A () {}",
-            "",
             "  // `m1()` comment.",
             "  void m1() {",
             "    // Print line 'foo' to stdout.",