Skip to content

Commit

Permalink
Code clean-up, formatting, docs, TEMPORARY supress warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
@benhalasi
benhalasi committed May 18, 2023
1 parent 4a38946 commit 00eebf3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 25 deletions.
26 changes: 16 additions & 10 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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) -> {
Expand All @@ -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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 =
Expand Down
29 changes: 14 additions & 15 deletions error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MemberOrderingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';",
Expand All @@ -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;",
Expand All @@ -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();
Expand All @@ -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';",
Expand All @@ -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 {}",
Expand All @@ -99,7 +102,6 @@ void replacementFirstSuggestedFix() {
"}")
.addOutputLines(
"A.java",
"",
"@SuppressWarnings(\"MemberOrdering\")",
"class A {",
" private static final int X = 1;",
Expand All @@ -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 {}",
Expand All @@ -126,6 +129,7 @@ void replacementFirstSuggestedFix() {
.doTest(TestMode.TEXT_MATCH);
}

@SuppressWarnings("ErrorProneTestHelperSourceFormat")
@Test
void replacementSecondSuggestedFix() {
BugCheckerRefactoringTestHelper.newInstance(MemberOrdering.class, getClass())
Expand All @@ -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\";",
Expand All @@ -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())
Expand All @@ -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.",
Expand Down

0 comments on commit 00eebf3

Please sign in to comment.