diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml
index cd4016dfe8..1df8d22618 100644
--- a/error-prone-contrib/pom.xml
+++ b/error-prone-contrib/pom.xml
@@ -72,6 +72,11 @@
auto-service-annotations
provided
+
+ com.google.auto.value
+ auto-value-annotations
+ provided
+
com.google.googlejavaformat
google-java-format
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java
index f8a59474c9..cde2a737e0 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java
@@ -5,12 +5,13 @@
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
import static java.util.Comparator.comparing;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
@@ -25,13 +26,18 @@
import com.sun.source.tree.VariableTree;
import com.sun.source.util.TreePath;
import com.sun.tools.javac.parser.Tokens;
+import com.sun.tools.javac.util.Position;
import java.util.Comparator;
+import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.lang.model.element.Modifier;
+import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/** A {@link BugChecker} that flags classes with non-standard member ordering. */
+// XXX: Reference
+// https://checkstyle.sourceforge.io/apidocs/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.html
@AutoService(BugChecker.class)
@BugPattern(
summary = "Class members should be ordered in a standard way",
@@ -44,11 +50,10 @@
tags = STYLE)
public final class ClassMemberOrdering extends BugChecker implements BugChecker.ClassTreeMatcher {
private static final long serialVersionUID = 1L;
-
- /** A comparator that sorts class members (including constructors) in a standard order. */
- private static final Comparator CLASS_MEMBER_SORTER =
+ /** Orders {@link Tree}s to match the standard Java type member declaration order. */
+ private static final Comparator BY_PREFERRED_TYPE_MEMBER_ORDER =
comparing(
- (Tree tree) -> {
+ tree -> {
switch (tree.getKind()) {
case VARIABLE:
return isStatic((VariableTree) tree) ? 1 : 2;
@@ -71,7 +76,7 @@ public Description matchClass(ClassTree classTree, VisitorState state) {
ImmutableList sortedClassMembers =
ImmutableList.sortedCopyOf(
- (a, b) -> CLASS_MEMBER_SORTER.compare(a.tree(), b.tree()), classMembers);
+ (a, b) -> BY_PREFERRED_TYPE_MEMBER_ORDER.compare(a.tree(), b.tree()), classMembers);
if (classMembers.equals(sortedClassMembers)) {
return Description.NO_MATCH;
@@ -104,13 +109,12 @@ private static SuggestedFix replaceClassMembers(
ImmutableList classMembers,
ImmutableList replacementClassMembers,
VisitorState state) {
- SuggestedFix.Builder fix = SuggestedFix.builder();
- for (int i = 0; i < classMembers.size(); i++) {
- ClassMemberWithComments original = classMembers.get(i);
- ClassMemberWithComments replacement = replacementClassMembers.get(i);
- fix.merge(replaceClassMember(state, original, replacement));
- }
- return fix.build();
+ return Streams.zip(
+ classMembers.stream(),
+ replacementClassMembers.stream(),
+ (original, replacement) -> replaceClassMember(state, original, replacement))
+ .reduce(SuggestedFix.builder(), SuggestedFix.Builder::merge, SuggestedFix.Builder::merge)
+ .build();
}
private static SuggestedFix replaceClassMember(
@@ -119,11 +123,12 @@ private static SuggestedFix replaceClassMember(
if (original.equals(replacement)) {
return SuggestedFix.emptyFix();
}
+
String replacementSource =
Stream.concat(
- replacement.comments().stream().map(Tokens.Comment::getText),
- Stream.of(state.getSourceForNode(replacement.tree())))
- .collect(joining("\n"));
+ replacement.comments().stream(),
+ Stream.of(SourceCode.treeToString(replacement.tree(), state)))
+ .collect(joining(System.lineSeparator()));
return SuggestedFixes.replaceIncludingComments(
TreePath.getPath(state.getPath(), original.tree()), replacementSource, state);
}
@@ -133,56 +138,44 @@ private static ImmutableList getClassMembersWithComment
ClassTree classTree, VisitorState state) {
return classTree.getMembers().stream()
.map(
- classMember ->
- new ClassMemberWithComments(
- classMember, getClassMemberComments(state, classTree, classMember)))
+ member ->
+ new AutoValue_ClassMemberOrdering_ClassMemberWithComments(
+ member, getClassMemberComments(state, classTree, member)))
.collect(toImmutableList());
}
- private static ImmutableList getClassMemberComments(
+ private static ImmutableList getClassMemberComments(
VisitorState state, ClassTree classTree, Tree classMember) {
- if (state.getEndPosition(classMember) == -1) {
- // Member is probably generated, according to `VisitorState` its end position is "not
- // available".
+ int typeStart = ASTHelpers.getStartPosition(classTree);
+ int typeEnd = state.getEndPosition(classTree);
+ int memberStart = ASTHelpers.getStartPosition(classMember);
+ int memberEnd = state.getEndPosition(classMember);
+ if (typeStart == Position.NOPOS
+ || typeEnd == Position.NOPOS
+ || memberStart == Position.NOPOS
+ || memberEnd == Position.NOPOS) {
+ /* Source code details appear to be unavailable. */
return ImmutableList.of();
}
- ImmutableList classTokens =
- ImmutableList.copyOf(
- state.getOffsetTokens(
- ASTHelpers.getStartPosition(classTree), state.getEndPosition(classTree)));
-
- int classMemberStartPos = ASTHelpers.getStartPosition(classMember);
Optional previousClassTokenEndPos =
- classTokens.stream()
+ state.getOffsetTokens(typeStart, typeEnd).stream()
.map(ErrorProneToken::endPos)
- .takeWhile(endPos -> endPos < classMemberStartPos)
+ .takeWhile(endPos -> endPos < memberStart)
.reduce((earlierPos, laterPos) -> laterPos);
- ImmutableList classMemberTokens =
- ImmutableList.copyOf(
- state.getOffsetTokens(
- previousClassTokenEndPos.orElse(classMemberStartPos),
- state.getEndPosition(classMember)));
+ List classMemberTokens =
+ state.getOffsetTokens(previousClassTokenEndPos.orElse(memberStart), memberEnd);
- return ImmutableList.copyOf(classMemberTokens.get(0).comments());
+ return classMemberTokens.get(0).comments().stream()
+ .map(Tokens.Comment::getText)
+ .collect(toImmutableList());
}
- private static final class ClassMemberWithComments {
- private final Tree tree;
- private final ImmutableList comments;
+ @AutoValue
+ abstract static class ClassMemberWithComments {
+ abstract Tree tree();
- ClassMemberWithComments(Tree tree, ImmutableList comments) {
- this.tree = requireNonNull(tree);
- this.comments = requireNonNull(comments);
- }
-
- public Tree tree() {
- return tree;
- }
-
- public ImmutableList comments() {
- return comments;
- }
+ abstract ImmutableList comments();
}
}