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/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(); + } +}