jlengrand/error-prone-support
1
0
Fork 0
mirror of https://github.com/jlengrand/error-prone-support.git synced 2026-03-10 08:11:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Suggestions

Browse Source
This commit is contained in:
Rick Ossendrijver
2024-05-27 08:49:00 +02:00
committed by Benedek Halasi
parent 8c4b8c27d2
commit a2c92b54dd
5 changed files with 88 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TypeMemberOrder.java
View File

@@ -82,28 +82,6 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
return Description.NO_MATCH;
}
/* All members that can be moved or may lay between movable ones. */
ImmutableList<TypeMember> members =
tree.getMembers().stream()
.filter(
member -> !MoreASTHelpers.isGeneratedConstructor(member) && !isEnumerator(member))
.map(m -> new AutoValue_TypeMemberOrder_TypeMember(m, getPreferredOrdinal(m, state)))
.collect(toImmutableList());
/*
List of the sortable members' preferred ordinals, ordered by the member's position in the
original source.
*/
ImmutableList<Integer> preferredOrdinals =
members.stream()
.filter(m -> m.preferredOrdinal().isPresent())
.map(m -> m.preferredOrdinal().orElseThrow(/* Unreachable due to preceding check. */ ))
.collect(toImmutableList());
if (Comparators.isInOrder(preferredOrdinals, naturalOrder())) {
return Description.NO_MATCH;
}
int bodyStartPos = getBodyStartPos(tree, state);
if (bodyStartPos == Position.NOPOS) {
/*
@@ -114,17 +92,22 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
return Description.NO_MATCH;
}
ImmutableList<TypeMember> members = getAllTypeMembers(tree, state);
ImmutableList<Integer> preferredOrdinals = extractPreferredOrdinals(members);
if (Comparators.isInOrder(preferredOrdinals, naturalOrder())) {
return Description.NO_MATCH;
}
return describeMatch(tree, sortTypeMembers(bodyStartPos, members, state));
}
/**
* Returns true if {@link Tree} is an enumerator of an enumerated type, false otherwise.
*
* @see com.sun.tools.javac.tree.Pretty#isEnumerator(JCTree)
* @see com.sun.tools.javac.code.Flags#ENUM
*/
private static boolean isEnumerator(Tree tree) {
return tree instanceof JCVariableDecl variableDecl && (variableDecl.mods.flags & ENUM) != 0;
/** Returns all members that can be moved or may lay between movable ones. */
private ImmutableList<TypeMember> getAllTypeMembers(ClassTree tree, VisitorState state) {
return tree.getMembers().stream()
.filter(member -> !MoreASTHelpers.isGeneratedConstructor(member) && !isEnumerator(member))
.map(m -> new AutoValue_TypeMemberOrder_TypeMember(m, getPreferredOrdinal(m, state)))
.collect(toImmutableList());
}
/**
@@ -140,7 +123,6 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
case BLOCK -> Optional.of(isStatic((BlockTree) tree) ? 3 : 4);
case METHOD -> Optional.of(isConstructor((MethodTree) tree) ? 5 : 6);
case CLASS, INTERFACE, ENUM -> Optional.of(7);
// TODO: Should we log unhandled kinds?
default -> Optional.empty();
};
}
@@ -158,14 +140,14 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
return Position.NOPOS;
}
/* We return the source code position of the first token that follows the first left brace. */
/* Returns the source code position of the first token that comes after the first curly left bracket. */
return ErrorProneTokens.getTokens(
sourceCode.subSequence(typeStart, typeEnd).toString(), typeStart, state.context)
.stream()
.dropWhile(token -> token.kind() != TokenKind.LBRACE)
/*
* To accommodate enums, skip processing their enumerators.
* This is needed as ErrorProne has access to the enumerations individually, but not to the
* This is needed as Error Prone has access to the enumerations individually, but not to the
* whole expression that declares them, leaving the semicolon trailing the declarations
* unaccounted for. The current logic would move this trailing semicolon with the first
* member after the enumerations instead of leaving it to close the enumerations'
@@ -177,6 +159,28 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
.orElse(Position.NOPOS);
}
/**
* Returns a list of the sortable members' preferred ordinals, ordered by the member's position in
* the original source.
*/
private static ImmutableList<Integer> extractPreferredOrdinals(
ImmutableList<TypeMember> members) {
return members.stream()
.filter(m -> m.preferredOrdinal().isPresent())
.map(m -> m.preferredOrdinal().orElseThrow(/* Unreachable due to preceding check. */ ))
.collect(toImmutableList());
}
/**
* Returns true if {@link Tree} is an enumerator of an enumerated type, false otherwise.
*
* @see com.sun.tools.javac.tree.Pretty#isEnumerator(JCTree)
* @see com.sun.tools.javac.code.Flags#ENUM
*/
private static boolean isEnumerator(Tree tree) {
return tree instanceof JCVariableDecl variableDecl && (variableDecl.mods.flags & ENUM) != 0;
}
/**
* Suggests a different way of ordering the given type members.
*
@@ -196,8 +200,9 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
int end = state.getEndPosition(member.tree());
verify(
end != Position.NOPOS && start < end,
"Unexpected member end position, member: %s",
member);
"Unexpected member end position, member: %s, end position: %s",
member,
end);
if (member.preferredOrdinal().isPresent()) {
membersWithSource.add(
new AutoValue_TypeMemberOrder_MovableTypeMember(
@@ -232,6 +237,7 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
return ASTHelpers.getSymbol(methodTree).isConstructor();
}
/** XXX: Write this. Every member that is in a ClassTree? */
@AutoValue
abstract static class TypeMember {
abstract Tree tree();
@@ -239,6 +245,7 @@ public final class TypeMemberOrder extends BugChecker implements ClassTreeMatche
abstract Optional<Integer> preferredOrdinal();
}
/** Type members that have a sourcecode and are not generated, are considered to be movable. */
@AutoValue
abstract static class MovableTypeMember {
abstract Tree tree();

48
error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TypeMemberOrderClassTest.java
View File

@@ -78,7 +78,15 @@ final class TypeMemberOrderClassTest {
"",
" D() {}",
"}")
.addSourceLines("E.java", "class E {}")
.addSourceLines(
"E.java",
"@SuppressWarnings(\"TypeMemberOrder\")",
"class E {",
" void unorderedMethod() {}",
"",
" E() {}",
"}")
.addSourceLines("F.java", "class F {}")
.doTest();
}
@@ -215,7 +223,7 @@ final class TypeMemberOrderClassTest {
}
@Test
void replacementDefaultConstructor() {
void replacementHandlesGeneratedDefaultConstructor() {
BugCheckerRefactoringTestHelper.newInstance(TypeMemberOrder.class, getClass())
.addInputLines(
"A.java",
@@ -263,9 +271,13 @@ final class TypeMemberOrderClassTest {
.addInputLines(
"A.java",
"class A {",
" /* `foo` method's dangling comment. */",
" /* empty statement's dangling comment */",
" ;",
" // `foo` method's comment",
" /**",
" * Multiline JavaDoc",
" *",
" * <p>`foo` method's comment",
" */",
" int foo() {",
" return foo;",
" }",
@@ -279,7 +291,7 @@ final class TypeMemberOrderClassTest {
" static {",
" System.out.println(\"foo\");",
" }",
" /* `bar` field's dangling comment. */ ;",
" /* `bar` field's dangling comment */",
"",
" private final int bar = 2;",
" // `foo` field's comment",
@@ -291,7 +303,7 @@ final class TypeMemberOrderClassTest {
"class A {",
" // `foo` field's comment",
" private static final int foo = 1;",
" /* `bar` field's dangling comment. */ ;",
" /* `bar` field's dangling comment */",
"",
" private final int bar = 2;",
"",
@@ -304,10 +316,14 @@ final class TypeMemberOrderClassTest {
" {",
" System.out.println(\"bar\");",
" }",
" /* `foo` method's dangling comment. */",
" /* empty statement's dangling comment */",
" ;",
"",
" // `foo` method's comment",
" /**",
" * Multiline JavaDoc",
" *",
" * <p>`foo` method's comment",
" */",
" int foo() {",
" return foo;",
" }",
@@ -328,7 +344,13 @@ final class TypeMemberOrderClassTest {
" }",
"",
" @AnnotationWithClassReferences({Object.class})",
" class InnerClass {",
" class InnerClassOneValue {",
" String bar;",
" private static final int foo = 1;",
" }",
"",
" @AnnotationWithClassReferences(value = {Integer.class, String.class})",
" class InnerClassTwoValues {",
" String bar;",
" private static final int foo = 1;",
" }",
@@ -342,7 +364,13 @@ final class TypeMemberOrderClassTest {
" }",
"",
" @AnnotationWithClassReferences({Object.class})",
" class InnerClass {",
" class InnerClassOneValue {",
" private static final int foo = 1;",
" String bar;",
" }",
"",
" @AnnotationWithClassReferences(value = {Integer.class, String.class})",
" class InnerClassTwoValues {",
" private static final int foo = 1;",
" String bar;",
" }",

2
error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TypeMemberOrderEnumTest.java
View File

@@ -208,7 +208,7 @@ final class TypeMemberOrderEnumTest {
}
@Test
void replacementDefaultConstructor() {
void replacementHandlesGeneratedDefaultConstructor() {
BugCheckerRefactoringTestHelper.newInstance(TypeMemberOrder.class, getClass())
.addInputLines(
"A.java",

12
error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TypeMemberOrderInterfaceTest.java
View File

@@ -168,22 +168,22 @@ final class TypeMemberOrderInterfaceTest {
.addInputLines(
"A.java",
"interface A {",
" // `bar` method's dangling comment.",
" // empty statement's dangling comment",
" ;",
" // `bar` method's comment",
" void bar();",
"",
" // `foo` method's comment",
" static final int foo = 1;",
" // `foo` field's comment",
" int foo = 1;",
" // trailing comment",
"}")
.addOutputLines(
"A.java",
"interface A {",
"",
" // `foo` method's comment",
" static final int foo = 1;",
" // `bar` method's dangling comment.",
" // `foo` field's comment",
" int foo = 1;",
" // empty statement's dangling comment",
" ;",
"",
" // `bar` method's comment",

2
error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TypeMemberOrderUnhandledKindsTest.java → error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TypeMemberOrderUnhandledCasesTest.java
View File

@@ -3,7 +3,7 @@ package tech.picnic.errorprone.bugpatterns;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class TypeMemberOrderUnhandledKindsTest {
final class TypeMemberOrderUnhandledCasesTest {
@Test
void identification() {
CompilationTestHelper.newInstance(TypeMemberOrder.class, getClass())