Skip to content

Commit

Permalink
Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202 @benhalasi
Stephan202 authored and benhalasi committed Oct 1, 2023
1 parent a078ec4 commit 0ee9ee2
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 52 deletions.
5 changes: 5 additions & 0 deletions error-prone-contrib/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@
<artifactId>auto-service-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.auto.value</groupId>
<artifactId>auto-value-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format</artifactId>
Expand Down
97 changes: 45 additions & 52 deletions ...r-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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",
Expand All @@ -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<Tree> CLASS_MEMBER_SORTER =
/** Orders {@link Tree}s to match the standard Java type member declaration order. */
private static final Comparator<Tree> BY_PREFERRED_TYPE_MEMBER_ORDER =
comparing(
(Tree tree) -> {
tree -> {
switch (tree.getKind()) {
case VARIABLE:
return isStatic((VariableTree) tree) ? 1 : 2;
Expand All @@ -71,7 +76,7 @@ public Description matchClass(ClassTree classTree, VisitorState state) {

ImmutableList<ClassMemberWithComments> 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;
Expand Down Expand Up @@ -104,13 +109,12 @@ private static SuggestedFix replaceClassMembers(
ImmutableList<ClassMemberWithComments> classMembers,
ImmutableList<ClassMemberWithComments> 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(
Expand All @@ -119,11 +123,12 @@ private static SuggestedFix replaceClassMember(
if (original.equals(replacement)) {

Check warning on line 123 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 123 without causing a test to fail 

removed conditional - replaced equality check with false (covered by 6 tests RemoveConditionalMutator_EQUAL_ELSE)
return SuggestedFix.emptyFix();

Check warning on line 124 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 124 without causing a test to fail 

replaced return value with null for replaceClassMember (covered by 1 tests NullReturnValsMutator)
}

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);
}
Expand All @@ -133,56 +138,44 @@ private static ImmutableList<ClassMemberWithComments> 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<Tokens.Comment> getClassMemberComments(
private static ImmutableList<String> 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

Check warning on line 153 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ClassMemberOrdering.java

View workflow job for this annotation

GitHub Actions / pitest

3 different changes can be made to line 153 without causing a test to fail 

removed conditional - replaced equality check with true (covered by 6 tests RemoveConditionalMutator_EQUAL_IF)
removed conditional - replaced equality check with true (covered by 6 tests RemoveConditionalMutator_EQUAL_IF)
removed conditional - replaced equality check with true (covered by 6 tests RemoveConditionalMutator_EQUAL_IF)
|| typeEnd == Position.NOPOS
|| memberStart == Position.NOPOS
|| memberEnd == Position.NOPOS) {
/* Source code details appear to be unavailable. */
return ImmutableList.of();
}

ImmutableList<ErrorProneToken> classTokens =
ImmutableList.copyOf(
state.getOffsetTokens(
ASTHelpers.getStartPosition(classTree), state.getEndPosition(classTree)));

int classMemberStartPos = ASTHelpers.getStartPosition(classMember);
Optional<Integer> previousClassTokenEndPos =
classTokens.stream()
state.getOffsetTokens(typeStart, typeEnd).stream()
.map(ErrorProneToken::endPos)
.takeWhile(endPos -> endPos < classMemberStartPos)
.takeWhile(endPos -> endPos < memberStart)
.reduce((earlierPos, laterPos) -> laterPos);

ImmutableList<ErrorProneToken> classMemberTokens =
ImmutableList.copyOf(
state.getOffsetTokens(
previousClassTokenEndPos.orElse(classMemberStartPos),
state.getEndPosition(classMember)));
List<ErrorProneToken> 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<Tokens.Comment> comments;
@AutoValue
abstract static class ClassMemberWithComments {
abstract Tree tree();

ClassMemberWithComments(Tree tree, ImmutableList<Tokens.Comment> comments) {
this.tree = requireNonNull(tree);
this.comments = requireNonNull(comments);
}

public Tree tree() {
return tree;
}

public ImmutableList<Tokens.Comment> comments() {
return comments;
}
abstract ImmutableList<String> comments();
}
}

0 comments on commit 0ee9ee2

Please sign in to comment.