diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheck.java new file mode 100644 index 0000000000..b6dcb71b92 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheck.java @@ -0,0 +1,91 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; +import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; +import static java.util.stream.Collectors.collectingAndThen; + +import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSetMultimap; +import com.google.common.collect.Multimaps; +import com.google.common.collect.Ordering; +import com.google.common.collect.Sets; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.LinkType; +import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.BugPattern.StandardTags; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Type; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; + +/** + * A {@link BugChecker} which flags {@link Ordering#explicit(Object, Object[])}} invocations listing + * a subset of an enum type's values. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "ExplicitEnumOrdering", + summary = "Make sure `Ordering#explicit` lists all of an enum's values", + linkType = LinkType.NONE, + severity = SeverityLevel.WARNING, + tags = StandardTags.FRAGILE_CODE) +public final class ExplicitEnumOrderingCheck extends BugChecker + implements MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Matcher EXPLICIT_ORDERING = + staticMethod().onClass(Ordering.class.getName()).named("explicit"); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!EXPLICIT_ORDERING.matches(tree, state)) { + return Description.NO_MATCH; + } + + ImmutableSet missingEnumValues = getMissingEnumValues(tree.getArguments()); + if (missingEnumValues.isEmpty()) { + return Description.NO_MATCH; + } + + return buildDescription(tree) + .setMessage( + String.format( + "Explicit ordering lacks some enum values: %s", + String.join(", ", missingEnumValues))) + .build(); + } + + private static ImmutableSet getMissingEnumValues( + List expressions) { + return expressions.stream() + .map(ASTHelpers::getSymbol) + .filter(Symbol::isEnum) + .collect( + collectingAndThen( + toImmutableSetMultimap(Symbol::asType, Symbol::toString), + ExplicitEnumOrderingCheck::getMissingEnumValues)); + } + + private static ImmutableSet getMissingEnumValues( + ImmutableSetMultimap valuesByType) { + return Multimaps.asMap(valuesByType).entrySet().stream() + .flatMap(e -> getMissingEnumValues(e.getKey(), e.getValue())) + .collect(toImmutableSet()); + } + + private static Stream getMissingEnumValues(Type enumType, Set values) { + Symbol.TypeSymbol typeSymbol = enumType.asElement(); + return Sets.difference(ASTHelpers.enumValues(typeSymbol), values).stream() + .map(v -> String.format("%s.%s", typeSymbol.getSimpleName(), v)); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/OrderingExplicitMethodCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/OrderingExplicitMethodCheck.java deleted file mode 100644 index 7913c6a321..0000000000 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/OrderingExplicitMethodCheck.java +++ /dev/null @@ -1,94 +0,0 @@ -package tech.picnic.errorprone.bugpatterns; - -import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static com.google.errorprone.matchers.Matchers.not; -import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; - -import com.google.auto.service.AutoService; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Ordering; -import com.google.errorprone.BugPattern; -import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker; -import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; -import com.google.errorprone.matchers.Description; -import com.google.errorprone.util.ASTHelpers; -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.MethodInvocationTree; -import com.sun.tools.javac.code.Symbol; -import com.sun.tools.javac.tree.JCTree; -import com.sun.tools.javac.util.Name; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Objects; -import java.util.function.Predicate; -import java.util.stream.Stream; - -/** - * A {@link BugChecker} which flags that {@link com.google.common.collect.Ordering#explicit(Object, - * Object[])}} for enums does not contain all it's values. - */ -@AutoService(BugChecker.class) -@BugPattern( - name = "OrderingExplicitMethod", - summary = - "Make sure Ordering.explicit() comparator with enum types contains all values from it", - linkType = BugPattern.LinkType.NONE, - severity = BugPattern.SeverityLevel.WARNING, - tags = BugPattern.StandardTags.FRAGILE_CODE) -public final class OrderingExplicitMethodCheck extends BugChecker - implements MethodInvocationTreeMatcher { - private static final long serialVersionUID = 1L; - - @Override - public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (not(staticMethod().onClass(Ordering.class.getName()).named("explicit")) - .matches(tree, state)) { - return Description.NO_MATCH; - } - List arguments = tree.getArguments(); - if (isNotEnumType(arguments)) { - return Description.NO_MATCH; - } - - ImmutableSet actualValues = - arguments.stream() - .map(arg -> (JCTree.JCFieldAccess) arg) - .map(arg -> arg.name) - .map(Name::toString) - .collect(toImmutableSet()); - - LinkedHashSet enumValues = - getTypeSymbolStream(arguments).map(ASTHelpers::enumValues).findFirst().orElseThrow(); - - if (actualValues.containsAll(enumValues)) { - return Description.NO_MATCH; - } - - return buildDescription(tree) - .setMessage( - String.format( - "Method should include all values from %s enum", getClassSimpleName(arguments))) - .build(); - } - - private static boolean isNotEnumType(List arguments) { - return arguments.stream().anyMatch(Predicate.not(arg -> ASTHelpers.getSymbol(arg).isEnum())); - } - - private static String getClassSimpleName(List arguments) { - return getTypeSymbolStream(arguments) - .map(Symbol::getSimpleName) - .map(Name::toString) - .findFirst() - .orElseThrow(); - } - - private static Stream getTypeSymbolStream( - List arguments) { - return arguments.stream() - .map(ASTHelpers::getType) - .filter(Objects::nonNull) - .map(type -> type.tsym); - } -} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java new file mode 100644 index 0000000000..4e91ab1eb1 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java @@ -0,0 +1,85 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class ExplicitEnumOrderingCheckTest { + private final CompilationTestHelper compilationTestHelper = + CompilationTestHelper.newInstance(ExplicitEnumOrderingCheck.class, getClass()); + + @Test + void Identification() { + compilationTestHelper + .addSourceLines( + "A.java", + "import static java.lang.annotation.RetentionPolicy.SOURCE;", + "import static java.lang.annotation.RetentionPolicy.CLASS;", + "import static java.lang.annotation.RetentionPolicy.RUNTIME;", + "import static java.time.chrono.IsoEra.BCE;", + "import static java.time.chrono.IsoEra.CE;", + "", + "import com.google.common.collect.Ordering;", + "import com.google.common.collect.ImmutableList;", + "import java.lang.annotation.RetentionPolicy;", + "import java.time.chrono.IsoEra;", + "", + "class A {", + " {", + " // The `List`-accepting overload is currently ignored.", + " Ordering.explicit(ImmutableList.of(RetentionPolicy.SOURCE, RetentionPolicy.CLASS));", + "", + " Ordering.explicit(IsoEra.BCE, IsoEra.CE);", + " // BUG: Diagnostic contains: IsoEra.CE", + " Ordering.explicit(IsoEra.BCE);", + " // BUG: Diagnostic contains: IsoEra.BCE", + " Ordering.explicit(IsoEra.CE);", + "", + " Ordering.explicit(RetentionPolicy.SOURCE, RetentionPolicy.CLASS, RetentionPolicy.RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.CLASS, RetentionPolicy.RUNTIME", + " Ordering.explicit(RetentionPolicy.SOURCE);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE, RetentionPolicy.RUNTIME", + " Ordering.explicit(RetentionPolicy.CLASS);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE, RetentionPolicy.CLASS", + " Ordering.explicit(RetentionPolicy.RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.RUNTIME", + " Ordering.explicit(RetentionPolicy.SOURCE, RetentionPolicy.CLASS);", + " // BUG: Diagnostic contains: RetentionPolicy.CLASS", + " Ordering.explicit(RetentionPolicy.SOURCE, RetentionPolicy.RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE", + " Ordering.explicit(RetentionPolicy.CLASS, RetentionPolicy.RUNTIME);", + "", + " Ordering.explicit(BCE, CE);", + " // BUG: Diagnostic contains: IsoEra.CE", + " Ordering.explicit(BCE);", + " // BUG: Diagnostic contains: IsoEra.BCE", + " Ordering.explicit(CE);", + "", + " Ordering.explicit(SOURCE, CLASS, RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.CLASS, RetentionPolicy.RUNTIME", + " Ordering.explicit(SOURCE);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE, RetentionPolicy.RUNTIME", + " Ordering.explicit(CLASS);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE, RetentionPolicy.CLASS", + " Ordering.explicit(RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.RUNTIME", + " Ordering.explicit(SOURCE, CLASS);", + " // BUG: Diagnostic contains: RetentionPolicy.CLASS", + " Ordering.explicit(SOURCE, RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE", + " Ordering.explicit(CLASS, RUNTIME);", + "", + " Ordering.explicit(RetentionPolicy.SOURCE, BCE, RetentionPolicy.CLASS, CE, RUNTIME);", + " Ordering.explicit(SOURCE, IsoEra.BCE, CLASS, IsoEra.CE, RetentionPolicy.RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.CLASS", + " Ordering.explicit(RetentionPolicy.SOURCE, BCE, CE, RUNTIME);", + " // BUG: Diagnostic contains: RetentionPolicy.CLASS", + " Ordering.explicit(IsoEra.BCE, SOURCE, IsoEra.CE, RetentionPolicy.RUNTIME);", + " // BUG: Diagnostic contains: IsoEra.CE, RetentionPolicy.RUNTIME", + " Ordering.explicit(IsoEra.BCE, SOURCE, RetentionPolicy.CLASS);", + " // BUG: Diagnostic contains: RetentionPolicy.SOURCE, IsoEra.BCE", + " Ordering.explicit(CLASS, RUNTIME, CE);", + " }", + "}") + .doTest(); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/OrderingExplicitMethodCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/OrderingExplicitMethodCheckTest.java deleted file mode 100644 index da731282f3..0000000000 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/OrderingExplicitMethodCheckTest.java +++ /dev/null @@ -1,28 +0,0 @@ -package tech.picnic.errorprone.bugpatterns; - -import com.google.errorprone.CompilationTestHelper; -import org.junit.jupiter.api.Test; - -final class OrderingExplicitMethodCheckTest { - private final CompilationTestHelper compilationTestHelper = - CompilationTestHelper.newInstance(OrderingExplicitMethodCheck.class, getClass()); - - @Test - void testHandleExplicitForEnums() { - compilationTestHelper - .addSourceLines( - "in/A.java", - "import com.google.common.collect.Ordering;", - "import java.util.Comparator;", - "", - "enum A {", - " ONE, TWO, THREE", - "}", - "", - "class B {", - " // BUG: Diagnostic contains: Method should include all values from A enum", - " Comparator explicit = Ordering.explicit(A.TWO, A.ONE);", - "}") - .doTest(); - } -}