From 00eebf3b615c067e6f75bdba6301440dfea527f9 Mon Sep 17 00:00:00 2001 From: Benedek Halasi Date: Thu, 18 May 2023 09:24:26 +0200 Subject: [PATCH] Code clean-up, formatting, docs, TEMPORARY supress warnings --- .../bugpatterns/MemberOrdering.java | 26 ++++++++++------- .../bugpatterns/MemberOrderingTest.java | 29 +++++++++---------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java index e7182227746..5793589d32d 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java +++ b/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 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 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 previousMember = getPreviousMember(member, classTree); ImmutableList tokens = diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MemberOrderingTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MemberOrderingTest.java index 9d7599406d9..a45e3ec74e8 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MemberOrderingTest.java +++ b/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,7 +29,7 @@ void identification() { "", " void m2() {}", "", - " public A () {}", + " public A() {}", "", " private static String BAR = \"bar\";", " char b = 'b';", @@ -38,14 +37,15 @@ void identification() { " 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,7 +85,7 @@ void replacementFirstSuggestedFix() { "", " void m2() {}", "", - " public A () {}", + " public A() {}", "", " private static String BAR = \"bar\";", " char b = 'b';", @@ -91,6 +93,7 @@ void replacementFirstSuggestedFix() { " 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,7 +111,7 @@ void replacementFirstSuggestedFix() { "", " void m2() {}", "", - " public A () {}", + " public A() {}", "", " private static String BAR = \"bar\";", " char b = 'b';", @@ -117,6 +119,7 @@ void replacementFirstSuggestedFix() { " 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.",