Support self-application of the checks

This commit is contained in:
Stephan Schroevers
2021-07-17 15:58:10 +02:00
parent d86611e66a
commit affd5c7b93
36 changed files with 203 additions and 141 deletions

View File

@@ -11,10 +11,11 @@ install:
script:
# We run the build twice: once against the original Error Prone release,
# using only Error Prone checks available on Maven Central, and once against
# the Picnic Error Prone fork, additionally enabling Error Prone checks
# available from other artifact repositories.
# the Picnic Error Prone fork, additionally enabling all checks defined in
# this project and any Error Prone checks available only from other artifact
# repositories.
- ./mvnw clean install
- ./mvnw clean install -Perror-prone-fork -Pnon-maven-central -s settings.xml
- ./mvnw clean install -Perror-prone-fork -Pnon-maven-central -Pself-check -s settings.xml
# XXX: Enable SonarCloud once we "go public".
# ./mvnw jacoco:prepare-agent surefire:test jacoco:report sonar:sonar
- ./mvnw jacoco:prepare-agent surefire:test jacoco:report

View File

@@ -1,5 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static java.util.Comparator.comparing;
import static java.util.Comparator.naturalOrder;
import static java.util.stream.Collectors.joining;
@@ -31,7 +32,6 @@ import com.sun.source.tree.Tree.Kind;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
@@ -153,7 +153,7 @@ public final class LexicographicalAnnotationAttributeListingCheck extends BugChe
// XXX: Perhaps we should use `Collator` with `.setStrength(Collator.PRIMARY)` and
// `getCollationKey`. Not clear whether that's worth the hassle at this point.
return ImmutableList.sortedCopyOf(
Comparator.comparing(
comparing(
e -> getStructure(e, state),
Comparators.lexicographical(
Comparators.lexicographical(

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Comparator.comparing;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
@@ -17,7 +18,6 @@ import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodTree;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
@@ -60,7 +60,7 @@ public final class LexicographicalAnnotationListingCheck extends BugChecker
private static ImmutableList<? extends AnnotationTree> sort(
List<? extends AnnotationTree> annotations, VisitorState state) {
return annotations.stream()
.sorted(Comparator.comparing(annotation -> Util.treeToString(annotation, state)))
.sorted(comparing(annotation -> Util.treeToString(annotation, state)))
.collect(toImmutableList());
}

View File

@@ -47,6 +47,8 @@ import javax.lang.model.element.Name;
// black-and-white. Maybe we can more closely approximate it?
// XXX: With Java 9's introduction of `Predicate.not`, we could write many lambda expressions to
// `not(some::reference)`.
// XXX: This check is extremely inefficient due to its reliance on `SuggestedFixes.compilesWithFix`.
// Palantir's `LambdaMethodReference` check seems to suffer a similar issue at this time.
@AutoService(BugChecker.class)
@BugPattern(
name = "MethodReferenceUsage",

View File

@@ -2,6 +2,7 @@ package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableRangeSet.toImmutableRangeSet;
import static java.util.Objects.requireNonNullElseGet;
import static java.util.function.Predicate.not;
import com.google.auto.service.AutoService;
@@ -42,7 +43,6 @@ import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.regex.Pattern;
@@ -92,7 +92,14 @@ public final class RefasterCheck extends BugChecker implements CompilationUnitTr
public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
/* First, collect all matches. */
List<Description> matches = new ArrayList<>();
codeTransformer.apply(state.getPath(), new SubContext(state.context), matches::add);
try {
codeTransformer.apply(state.getPath(), new SubContext(state.context), matches::add);
} catch (LinkageError e) {
// XXX: This `try/catch` block handles the issue described and resolved in
// https://github.com/google/error-prone/pull/2456. Drop this block once that change is
// released.
return Description.NO_MATCH;
}
/* Then apply them. */
applyMatches(matches, ((JCCompilationUnit) tree).endPositions, state);
@@ -188,8 +195,8 @@ public final class RefasterCheck extends BugChecker implements CompilationUnitTr
private static ImmutableSet<ResourceInfo> getClassPathResources() {
try {
return ClassPath.from(
Objects.requireNonNullElseGet(
RefasterCheck.class.getClassLoader(), () -> ClassLoader.getSystemClassLoader()))
requireNonNullElseGet(
RefasterCheck.class.getClassLoader(), ClassLoader::getSystemClassLoader))
.getResources();
} catch (IOException e) {
throw new UncheckedIOException("Failed to scan classpath for resources", e);

View File

@@ -1,5 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import static java.util.Objects.requireNonNull;
import com.google.auto.service.AutoService;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
@@ -20,7 +22,6 @@ import com.google.errorprone.matchers.Description;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import java.util.Objects;
import java.util.Optional;
/** A {@link BugChecker} which flags methods that can and should be statically imported. */
@@ -140,8 +141,7 @@ public final class StaticImportCheck extends BugChecker implements MemberSelectT
private static boolean isCandidate(VisitorState state) {
Tree parentTree =
Objects.requireNonNull(
state.getPath().getParentPath(), "MemberSelectTree lacks enclosing node")
requireNonNull(state.getPath().getParentPath(), "MemberSelectTree lacks enclosing node")
.getLeaf();
switch (parentTree.getKind()) {
case IMPORT:

View File

@@ -1,9 +1,10 @@
package tech.picnic.errorprone.refastertemplates;
import static com.google.common.base.Preconditions.checkElementIndex;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Sets.toImmutableEnumSet;
import static java.util.Objects.checkIndex;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -38,12 +39,12 @@ final class AssortedTemplates {
static final class CheckIndex {
@BeforeTemplate
int before(int index, int size) {
return Preconditions.checkElementIndex(index, size);
return checkElementIndex(index, size);
}
@AfterTemplate
int after(int index, int size) {
return Objects.checkIndex(index, size);
return checkIndex(index, size);
}
}
@@ -62,14 +63,14 @@ final class AssortedTemplates {
}
static final class MapGetOrNull<K, V, L> {
@Nullable
@BeforeTemplate
@Nullable
V before(Map<K, V> map, L key) {
return map.getOrDefault(key, null);
}
@Nullable
@AfterTemplate
@Nullable
V after(Map<K, V> map, L key) {
return map.get(key);
}
@@ -107,8 +108,8 @@ final class AssortedTemplates {
Streams.stream(iterator).findAny().orElse(defaultValue));
}
@Nullable
@AfterTemplate
@Nullable
T after(Iterator<T> iterator, T defaultValue) {
return Iterators.getNext(iterator, defaultValue);
}

View File

@@ -1,5 +1,10 @@
package tech.picnic.errorprone.refastertemplates;
import static java.util.Comparator.comparing;
import static java.util.Comparator.comparingDouble;
import static java.util.Comparator.comparingInt;
import static java.util.Comparator.comparingLong;
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.reverseOrder;
import static java.util.function.Function.identity;
@@ -29,14 +34,13 @@ final class ComparatorTemplates {
@BeforeTemplate
Comparator<T> before() {
return Refaster.anyOf(
Comparator.comparing(Refaster.anyOf(identity(), v -> v)),
Comparator.<T>reverseOrder().reversed());
comparing(Refaster.anyOf(identity(), v -> v)), Comparator.<T>reverseOrder().reversed());
}
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
Comparator<T> after() {
return Comparator.naturalOrder();
return naturalOrder();
}
}
@@ -58,7 +62,7 @@ final class ComparatorTemplates {
// XXX: Drop the `Refaster.anyOf` if/when we decide to rewrite one to the other.
@BeforeTemplate
Comparator<T> before(Comparator<T> cmp) {
return Comparator.comparing(Refaster.anyOf(identity(), v -> v), cmp);
return comparing(Refaster.anyOf(identity(), v -> v), cmp);
}
@AfterTemplate
@@ -72,7 +76,7 @@ final class ComparatorTemplates {
static final class ThenComparing<S, T extends Comparable<? super T>> {
@BeforeTemplate
Comparator<S> before(Comparator<S> cmp, Function<? super S, ? extends T> function) {
return cmp.thenComparing(Comparator.comparing(function));
return cmp.thenComparing(comparing(function));
}
@AfterTemplate
@@ -85,7 +89,7 @@ final class ComparatorTemplates {
static final class ThenComparingReversed<S, T extends Comparable<? super T>> {
@BeforeTemplate
Comparator<S> before(Comparator<S> cmp, Function<? super S, ? extends T> function) {
return cmp.thenComparing(Comparator.comparing(function).reversed());
return cmp.thenComparing(comparing(function).reversed());
}
@AfterTemplate
@@ -100,7 +104,7 @@ final class ComparatorTemplates {
@BeforeTemplate
Comparator<S> before(
Comparator<S> cmp, Function<? super S, ? extends T> function, Comparator<? super T> cmp2) {
return cmp.thenComparing(Comparator.comparing(function, cmp2));
return cmp.thenComparing(comparing(function, cmp2));
}
@AfterTemplate
@@ -115,7 +119,7 @@ final class ComparatorTemplates {
@BeforeTemplate
Comparator<S> before(
Comparator<S> cmp, Function<? super S, ? extends T> function, Comparator<? super T> cmp2) {
return cmp.thenComparing(Comparator.comparing(function, cmp2).reversed());
return cmp.thenComparing(comparing(function, cmp2).reversed());
}
@AfterTemplate
@@ -129,7 +133,7 @@ final class ComparatorTemplates {
static final class ThenComparingDouble<T> {
@BeforeTemplate
Comparator<T> before(Comparator<T> cmp, ToDoubleFunction<? super T> function) {
return cmp.thenComparing(Comparator.comparingDouble(function));
return cmp.thenComparing(comparingDouble(function));
}
@AfterTemplate
@@ -142,7 +146,7 @@ final class ComparatorTemplates {
static final class ThenComparingInt<T> {
@BeforeTemplate
Comparator<T> before(Comparator<T> cmp, ToIntFunction<? super T> function) {
return cmp.thenComparing(Comparator.comparingInt(function));
return cmp.thenComparing(comparingInt(function));
}
@AfterTemplate
@@ -155,7 +159,7 @@ final class ComparatorTemplates {
static final class ThenComparingLong<T> {
@BeforeTemplate
Comparator<T> before(Comparator<T> cmp, ToLongFunction<? super T> function) {
return cmp.thenComparing(Comparator.comparingLong(function));
return cmp.thenComparing(comparingLong(function));
}
@AfterTemplate
@@ -178,7 +182,7 @@ final class ComparatorTemplates {
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
Comparator<T> after(Comparator<T> cmp) {
return cmp.thenComparing(Comparator.naturalOrder());
return cmp.thenComparing(naturalOrder());
}
}

View File

@@ -26,8 +26,8 @@ final class EqualityTemplates {
return Refaster.anyOf(a.equals(b), Objects.equals(a, b));
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(T a, T b) {
return a == b;
}

View File

@@ -1,5 +1,10 @@
package tech.picnic.errorprone.refastertemplates;
import static java.util.Comparator.comparing;
import static java.util.Comparator.naturalOrder;
import static java.util.Map.Entry.comparingByKey;
import static java.util.Map.Entry.comparingByValue;
import com.google.common.collect.Maps;
import com.google.errorprone.refaster.ImportPolicy;
import com.google.errorprone.refaster.Refaster;
@@ -42,15 +47,13 @@ final class MapEntryTemplates {
static final class MapEntryComparingByKey<K extends Comparable<? super K>, V> {
@BeforeTemplate
Comparator<Map.Entry<K, V>> before() {
return Refaster.anyOf(
Comparator.comparing(Map.Entry::getKey),
Map.Entry.comparingByKey(Comparator.naturalOrder()));
return Refaster.anyOf(comparing(Map.Entry::getKey), comparingByKey(naturalOrder()));
}
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
Comparator<Map.Entry<K, V>> after() {
return Map.Entry.comparingByKey();
return comparingByKey();
}
}
@@ -58,13 +61,13 @@ final class MapEntryTemplates {
static final class MapEntryComparingByKeyWithCustomComparator<K, V> {
@BeforeTemplate
Comparator<Map.Entry<K, V>> before(Comparator<? super K> cmp) {
return Comparator.comparing(Map.Entry::getKey, cmp);
return comparing(Map.Entry::getKey, cmp);
}
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
Comparator<Map.Entry<K, V>> after(Comparator<? super K> cmp) {
return Map.Entry.comparingByKey(cmp);
return comparingByKey(cmp);
}
}
@@ -73,15 +76,13 @@ final class MapEntryTemplates {
static final class MapEntryComparingByValue<K, V extends Comparable<? super V>> {
@BeforeTemplate
Comparator<Map.Entry<K, V>> before() {
return Refaster.anyOf(
Comparator.comparing(Map.Entry::getValue),
Map.Entry.comparingByValue(Comparator.naturalOrder()));
return Refaster.anyOf(comparing(Map.Entry::getValue), comparingByValue(naturalOrder()));
}
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
Comparator<Map.Entry<K, V>> after() {
return Map.Entry.comparingByValue();
return comparingByValue();
}
}
@@ -89,13 +90,13 @@ final class MapEntryTemplates {
static final class MapEntryComparingByValueWithCustomComparator<K, V> {
@BeforeTemplate
Comparator<Map.Entry<K, V>> before(Comparator<? super V> cmp) {
return Comparator.comparing(Map.Entry::getValue, cmp);
return comparing(Map.Entry::getValue, cmp);
}
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
Comparator<Map.Entry<K, V>> after(Comparator<? super V> cmp) {
return Map.Entry.comparingByValue(cmp);
return comparingByValue(cmp);
}
}
}

View File

@@ -1,5 +1,7 @@
package tech.picnic.errorprone.refastertemplates;
import static java.util.Objects.requireNonNullElse;
import com.google.common.base.MoreObjects;
import com.google.errorprone.refaster.ImportPolicy;
import com.google.errorprone.refaster.annotation.AfterTemplate;
@@ -24,7 +26,7 @@ final class NullTemplates {
@AfterTemplate
@UseImportPolicy(ImportPolicy.STATIC_IMPORT_ALWAYS)
T after(T first, T second) {
return Objects.requireNonNullElse(first, second);
return requireNonNullElse(first, second);
}
}

View File

@@ -1,5 +1,6 @@
package tech.picnic.errorprone.refastertemplates;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Joiner;
@@ -10,7 +11,6 @@ import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.AlsoNegation;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Optional;
@@ -122,7 +122,7 @@ final class StringTemplates {
static final class Utf8EncodedLength {
@BeforeTemplate
int before(String str) {
return str.getBytes(StandardCharsets.UTF_8).length;
return str.getBytes(UTF_8).length;
}
@AfterTemplate

View File

@@ -81,8 +81,8 @@ final class TestNGToAssertJTemplates {
Assert.fail();
}
@DoNotCall
@AfterTemplate
@DoNotCall
void after() {
throw new AssertionError();
}

View File

@@ -62,11 +62,13 @@ final class TimeTemplates {
/** Use {@link Clock#systemUTC()} when possible. */
static final class UtcClock {
@BeforeTemplate
@SuppressWarnings("TimeZoneUsage")
Clock before() {
return Clock.system(ZoneOffset.UTC);
}
@AfterTemplate
@SuppressWarnings("TimeZoneUsage")
Clock after() {
return Clock.systemUTC();
}
@@ -96,8 +98,8 @@ final class TimeTemplates {
return a.compareTo(b) < 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(Instant a, Instant b) {
return a.isBefore(b);
}
@@ -113,8 +115,8 @@ final class TimeTemplates {
return a.compareTo(b) > 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(Instant a, Instant b) {
return a.isAfter(b);
}
@@ -162,8 +164,8 @@ final class TimeTemplates {
return a.compareTo(b) < 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(ChronoLocalDate a, ChronoLocalDate b) {
return a.isBefore(b);
}
@@ -179,8 +181,8 @@ final class TimeTemplates {
return a.compareTo(b) > 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(ChronoLocalDate a, ChronoLocalDate b) {
return a.isAfter(b);
}
@@ -196,8 +198,8 @@ final class TimeTemplates {
return a.compareTo(b) < 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(ChronoLocalDateTime<?> a, ChronoLocalDateTime<?> b) {
return a.isBefore(b);
}
@@ -213,8 +215,8 @@ final class TimeTemplates {
return a.compareTo(b) > 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(ChronoLocalDateTime<?> a, ChronoLocalDateTime<?> b) {
return a.isAfter(b);
}
@@ -230,8 +232,8 @@ final class TimeTemplates {
return a.compareTo(b) < 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(ChronoZonedDateTime<?> a, ChronoZonedDateTime<?> b) {
return a.isBefore(b);
}
@@ -247,8 +249,8 @@ final class TimeTemplates {
return a.compareTo(b) > 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(ChronoZonedDateTime<?> a, ChronoZonedDateTime<?> b) {
return a.isAfter(b);
}
@@ -264,8 +266,8 @@ final class TimeTemplates {
return a.compareTo(b) < 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(OffsetDateTime a, OffsetDateTime b) {
return a.isBefore(b);
}
@@ -281,8 +283,8 @@ final class TimeTemplates {
return a.compareTo(b) > 0;
}
@AlsoNegation
@AfterTemplate
@AlsoNegation
boolean after(OffsetDateTime a, OffsetDateTime b) {
return a.isAfter(b);
}

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.common.base.Predicates;
import static com.google.common.base.Predicates.containsPattern;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
@@ -10,13 +11,12 @@ final class AmbiguousJsonCreatorCheckTest {
CompilationTestHelper.newInstance(AmbiguousJsonCreatorCheck.class, getClass())
.expectErrorMessage(
"X",
Predicates.containsPattern(
"`JsonCreator.Mode` should be set for single-argument creators"));
containsPattern("`JsonCreator.Mode` should be set for single-argument creators"));
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(AmbiguousJsonCreatorCheck.class, getClass());
@Test
void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"Container.java",
@@ -118,7 +118,7 @@ final class AmbiguousJsonCreatorCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -8,14 +8,14 @@ import org.junit.jupiter.api.Test;
public final class AnnotationAttributeMatcherTest {
@Test
public void testWithoutListings() {
void withoutListings() {
AnnotationAttributeMatcher matcher =
AnnotationAttributeMatcher.create(Optional.empty(), ImmutableList.of());
assertThat(matcher.matches("foo", "bar")).isTrue();
}
@Test
public void testWithSingleFullAnnotationWhitelist() {
void withSingleFullAnnotationWhitelist() {
AnnotationAttributeMatcher matcher =
AnnotationAttributeMatcher.create(Optional.of(ImmutableList.of("foo")), ImmutableList.of());
assertThat(matcher.matches("foo", "bar")).isTrue();
@@ -24,7 +24,7 @@ public final class AnnotationAttributeMatcherTest {
}
@Test
public void testWithSingleAnnotationAttributeWhitelist() {
void withSingleAnnotationAttributeWhitelist() {
AnnotationAttributeMatcher matcher =
AnnotationAttributeMatcher.create(
Optional.of(ImmutableList.of("foo#bar")), ImmutableList.of());
@@ -34,7 +34,7 @@ public final class AnnotationAttributeMatcherTest {
}
@Test
public void testWithSingleFullAnnotationBlacklist() {
void withSingleFullAnnotationBlacklist() {
AnnotationAttributeMatcher matcher =
AnnotationAttributeMatcher.create(Optional.empty(), ImmutableList.of("foo"));
assertThat(matcher.matches("foo", "bar")).isFalse();
@@ -43,7 +43,7 @@ public final class AnnotationAttributeMatcherTest {
}
@Test
public void testWithSingleAnnotationAttributeBlacklist() {
void withSingleAnnotationAttributeBlacklist() {
AnnotationAttributeMatcher matcher =
AnnotationAttributeMatcher.create(Optional.empty(), ImmutableList.of("foo#bar"));
assertThat(matcher.matches("foo", "bar")).isFalse();
@@ -52,7 +52,7 @@ public final class AnnotationAttributeMatcherTest {
}
@Test
public void testWithComplicatedConfiguration() {
void withComplicatedConfiguration() {
AnnotationAttributeMatcher matcher =
AnnotationAttributeMatcher.create(
Optional.of(ImmutableList.of("foo", "bar", "baz", "baz#1", "baz#2", "quux#1")),

View File

@@ -12,7 +12,7 @@ public final class AutowiredConstructorCheckTest {
BugCheckerRefactoringTestHelper.newInstance(AutowiredConstructorCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"Container.java",
@@ -62,7 +62,7 @@ public final class AutowiredConstructorCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/Container.java",

View File

@@ -12,7 +12,7 @@ public final class CanonicalAnnotationSyntaxCheckTest {
BugCheckerRefactoringTestHelper.newInstance(CanonicalAnnotationSyntaxCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"pkg/A.java",
@@ -85,7 +85,7 @@ public final class CanonicalAnnotationSyntaxCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/pkg/A.java",

View File

@@ -12,7 +12,7 @@ public final class EmptyMethodCheckTest {
BugCheckerRefactoringTestHelper.newInstance(EmptyMethodCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -64,7 +64,7 @@ public final class EmptyMethodCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -12,7 +12,7 @@ public final class JUnitMethodDeclarationCheckTest {
BugCheckerRefactoringTestHelper.newInstance(JUnitMethodDeclarationCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -127,7 +127,7 @@ public final class JUnitMethodDeclarationCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -22,7 +22,7 @@ public final class LexicographicalAnnotationAttributeListingCheckTest {
LexicographicalAnnotationAttributeListingCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -105,7 +105,7 @@ public final class LexicographicalAnnotationAttributeListingCheckTest {
// introduced. Avoiding that might make the code too complex. Instead, users can have the
// `CanonicalAnnotationSyntaxCheck` correct the situation in a subsequent run.
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",
@@ -161,7 +161,7 @@ public final class LexicographicalAnnotationAttributeListingCheckTest {
}
@Test
public void testFiltering() {
void filtering() {
/* Some violations are not flagged because they are not in- or excluded. */
restrictedCompilationTestHelper
.addSourceLines(

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.common.base.Predicates;
import static com.google.common.base.Predicates.containsPattern;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
@@ -10,13 +11,13 @@ public final class LexicographicalAnnotationListingCheckTest {
private final CompilationTestHelper compilationTestHelper =
CompilationTestHelper.newInstance(LexicographicalAnnotationListingCheck.class, getClass())
.expectErrorMessage(
"X", Predicates.containsPattern("Sort annotations lexicographically where possible"));
"X", containsPattern("Sort annotations lexicographically where possible"));
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(
LexicographicalAnnotationListingCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -71,7 +72,7 @@ public final class LexicographicalAnnotationListingCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -47,7 +47,7 @@ public final class MethodMatcherFactoryTest {
CompilationTestHelper.newInstance(MatchedMethodsFlagger.class, getClass());
@Test
public void testCreateWithMalformedSignatures() {
void createWithMalformedSignatures() {
MethodMatcherFactory factory = new MethodMatcherFactory();
assertThatThrownBy(() -> factory.create(ImmutableList.of("foo.bar")))
.isInstanceOf(IllegalArgumentException.class);
@@ -60,7 +60,7 @@ public final class MethodMatcherFactoryTest {
}
@Test
public void testMatcher() {
void matcher() {
compilationTestHelper
.addSourceLines(
"com/example/A.java",

View File

@@ -12,7 +12,7 @@ public final class MethodReferenceUsageCheckTest {
BugCheckerRefactoringTestHelper.newInstance(MethodReferenceUsageCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -201,7 +201,7 @@ public final class MethodReferenceUsageCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.common.base.Predicates;
import static com.google.common.base.Predicates.containsPattern;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
@@ -9,11 +10,11 @@ public final class MissingRefasterAnnotationCheckTest {
CompilationTestHelper.newInstance(MissingRefasterAnnotationCheck.class, getClass())
.expectErrorMessage(
"X",
Predicates.containsPattern(
containsPattern(
"The Refaster template contains a method without any Refaster annotations"));
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",

View File

@@ -19,7 +19,7 @@ public final class PrimitiveComparisonCheckTest {
// The logic for `char` and `short` is exactly analogous to the `byte` case.
@Test
public void testByteComparison() {
void byteComparison() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -108,7 +108,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testIntComparison() {
void intComparison() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -201,7 +201,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testLongComparison() {
void longComparison() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -278,7 +278,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testFloatComparison() {
void floatComparison() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -335,7 +335,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testDoubleComparison() {
void doubleComparison() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -396,7 +396,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testStringComparison() {
void stringComparison() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -429,7 +429,7 @@ public final class PrimitiveComparisonCheckTest {
// XXX: If the explicit `<A, BoxedPrimitive>` generic type information was necessary, then this
// replacement drops too much information.
@Test
public void testReplacementWithPrimitiveVariants() {
void replacementWithPrimitiveVariants() {
refactoringTestHelper
.addInputLines(
"in/A.java",
@@ -483,7 +483,7 @@ public final class PrimitiveComparisonCheckTest {
// XXX: If the explicit `<A>` generic type information was necessary, then this replacement drops
// too much information.
@Test
public void testReplacementWithBoxedVariants() {
void replacementWithBoxedVariants() {
refactoringTestHelper
.addInputLines(
"in/A.java",
@@ -535,7 +535,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testReplacementWithPrimitiveVariantsUsingStaticImports() {
void replacementWithPrimitiveVariantsUsingStaticImports() {
refactoringTestHelper
.addInputLines(
"in/A.java",
@@ -574,7 +574,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testReplacementWithBoxedVariantsUsingStaticImports() {
void replacementWithBoxedVariantsUsingStaticImports() {
refactoringTestHelper
.addInputLines(
"in/A.java",
@@ -615,7 +615,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testReplacementWithPrimitiveVariantsInComplexSyntacticalContext() {
void replacementWithPrimitiveVariantsInComplexSyntacticalContext() {
refactoringTestHelper
.addInputLines(
"in/A.java",
@@ -647,7 +647,7 @@ public final class PrimitiveComparisonCheckTest {
}
@Test
public void testReplacementWithBoxedVariantsInComplexSyntacticalContext() {
void replacementWithBoxedVariantsInComplexSyntacticalContext() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -21,7 +21,7 @@ public final class RedundantStringConversionCheckTest {
BugCheckerRefactoringTestHelper.newInstance(RedundantStringConversionCheck.class, getClass());
@Test
public void testIdentificationOfIdentityTransformation() {
void identificationOfIdentityTransformation() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -44,7 +44,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testIdentificationWithinMutatingAssignment() {
void identificationWithinMutatingAssignment() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -100,7 +100,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testIdentificationWithinBinaryOperation() {
void identificationWithinBinaryOperation() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -186,7 +186,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testIdentificationWithinStringBuilderMethod() {
void identificationWithinStringBuilderMethod() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -235,7 +235,7 @@ public final class RedundantStringConversionCheckTest {
// XXX: Also test the other formatter methods.
@Test
public void testIdentificationWithinFormatterMethod() {
void identificationWithinFormatterMethod() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -280,7 +280,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testIdentificationWithinGuavaGuardMethod() {
void identificationWithinGuavaGuardMethod() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -340,7 +340,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testIdentificationWithinSlf4jLoggerMethod() {
void identificationWithinSlf4jLoggerMethod() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -395,7 +395,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testIdentificationOfCustomConversionMethod() {
void identificationOfCustomConversionMethod() {
customizedCompilationTestHelper
.addSourceLines(
"A.java",
@@ -486,7 +486,7 @@ public final class RedundantStringConversionCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -5,6 +5,7 @@ import static java.util.function.Function.identity;
import static java.util.function.Predicate.not;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
@@ -97,7 +98,7 @@ public final class RefasterCheckTest {
// XXX: Drop the filter once we have added tests for AssertJ!
return TEMPLATES_BY_GROUP.entries().stream()
.filter(e -> !"AssertJ".equals(e.getKey()))
.map(e -> Arguments.of(e.getKey(), e.getValue()));
.map(e -> arguments(e.getKey(), e.getValue()));
}
/**
@@ -105,10 +106,10 @@ public final class RefasterCheckTest {
* for all of the {@link #TEMPLATE_GROUPS}.
*
* <p>This test is just as much about ensuring that {@link #TEMPLATE_GROUPS} is exhaustive, so
* that in turn {@link #testReplacement}'s coverage is exhaustive.
* that in turn {@link #replacement}'s coverage is exhaustive.
*/
@Test
public void testLoadAllCodeTransformers() {
void loadAllCodeTransformers() {
assertThat(TEMPLATES_BY_GROUP.keySet()).hasSameElementsAs(TEMPLATE_GROUPS);
}
@@ -116,9 +117,9 @@ public final class RefasterCheckTest {
* Verifies for each of the {@link #TEMPLATE_GROUPS} that the associated code transformers have
* the desired effect.
*/
@ParameterizedTest
@MethodSource("templateGroupsUnderTest")
public void testReplacement(String group) {
@ParameterizedTest
void replacement(String group) {
verifyRefactoring(group, namePattern(group));
}
@@ -129,9 +130,9 @@ public final class RefasterCheckTest {
* com.google.errorprone.refaster.Refaster#anyOf} branches are tested. Idem for {@link
* com.google.errorprone.refaster.annotation.BeforeTemplate} methods in case there are multiple .
*/
@ParameterizedTest
@MethodSource("templatesUnderTest")
public void testCoverage(String group, String template) {
@ParameterizedTest
void coverage(String group, String template) {
assertThatCode(() -> verifyRefactoring(group, namePattern(group, template)))
.withFailMessage(
"Template %s does not affect the tests for group %s; is it tested?", template, group)

View File

@@ -8,7 +8,7 @@ public final class RequestMappingAnnotationCheckTest {
CompilationTestHelper.newInstance(RequestMappingAnnotationCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",

View File

@@ -12,7 +12,7 @@ public final class Slf4jLogStatementCheckTest {
BugCheckerRefactoringTestHelper.newInstance(Slf4jLogStatementCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -92,7 +92,7 @@ public final class Slf4jLogStatementCheckTest {
// XXX: Drop what's unused.
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -12,7 +12,7 @@ public final class SpringMvcAnnotationCheckTest {
BugCheckerRefactoringTestHelper.newInstance(SpringMvcAnnotationCheck.class, getClass());
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -59,7 +59,7 @@ public final class SpringMvcAnnotationCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -13,13 +13,13 @@ public final class StaticImportCheckTest {
BugCheckerRefactoringTestHelper.newInstance(StaticImportCheck.class, getClass());
@Test
public void testCandidateMethodsAreNotRedundant() {
void candidateMethodsAreNotRedundant() {
assertThat(StaticImportCheck.STATIC_IMPORT_CANDIDATE_METHODS.keySet())
.doesNotContainAnyElementsOf(StaticImportCheck.STATIC_IMPORT_CANDIDATE_CLASSES);
}
@Test
public void testIdentification() {
void identification() {
compilationTestHelper
.addSourceLines(
"A.java",
@@ -77,7 +77,7 @@ public final class StaticImportCheckTest {
}
@Test
public void testReplacement() {
void replacement() {
refactoringTestHelper
.addInputLines(
"in/A.java",

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.common.base.Predicates;
import static com.google.common.base.Predicates.containsPattern;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
@@ -9,11 +10,11 @@ public final class TimeZoneUsageCheckTest {
CompilationTestHelper.newInstance(TimeZoneUsageCheck.class, getClass())
.expectErrorMessage(
"X",
Predicates.containsPattern(
containsPattern(
"Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone"));
@Test
public void testIdentification() {
void identification() {
compilationHelper
.addSourceLines(
"A.java",

View File

@@ -1,5 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Joiner;
@@ -8,7 +9,6 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Optional;
import java.util.stream.Stream;
@@ -17,7 +17,7 @@ final class StringTemplatesTest implements RefasterTemplateTestCase {
@Override
public ImmutableSet<?> elidedTypesAndStaticImports() {
return ImmutableSet.of(
Arrays.class, Joiner.class, StandardCharsets.class, Stream.class, Streams.class, joining());
Arrays.class, Joiner.class, Stream.class, Streams.class, joining(), UTF_8);
}
ImmutableSet<Boolean> testStringIsEmpty() {
@@ -63,6 +63,6 @@ final class StringTemplatesTest implements RefasterTemplateTestCase {
}
int testUtf8EncodedLength() {
return "foo".getBytes(StandardCharsets.UTF_8).length;
return "foo".getBytes(UTF_8).length;
}
}

View File

@@ -1,5 +1,6 @@
package tech.picnic.errorprone.bugpatterns;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Joiner;
@@ -9,7 +10,6 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Optional;
import java.util.stream.Stream;
@@ -18,7 +18,7 @@ final class StringTemplatesTest implements RefasterTemplateTestCase {
@Override
public ImmutableSet<?> elidedTypesAndStaticImports() {
return ImmutableSet.of(
Arrays.class, Joiner.class, StandardCharsets.class, Stream.class, Streams.class, joining());
Arrays.class, Joiner.class, Stream.class, Streams.class, joining(), UTF_8);
}
ImmutableSet<Boolean> testStringIsEmpty() {

48
pom.xml
View File

@@ -96,9 +96,10 @@
build number. When building locally, this number is obviously absent.
So we provide a default value. -->
<build.number>LOCAL</build.number>
<!-- Property using which additional Error Prone flags can be
specified. Used by the `patch` profile to enable patching. -->
<error-prone.args />
<!-- Properties using which additional Error Prone flags can be
specified. Used by the `patch` and `self-check` profiles. -->
<error-prone.patch-args />
<error-prone.self-check-args />
<!-- The Maven `groupId` under which Error Prone dependencies are
published. By default we use an official Error Prone release. This
property allows the `error-prone-fork` profile below to build the
@@ -1179,6 +1180,42 @@
</plugins>
</build>
</profile>
<profile>
<!-- Applies the Error Prone checks defined by this project to the
code base itself. Assumes that a prior build has already installed
the project in the local Maven repository. -->
<id>self-check</id>
<properties>
<!-- XXX: `MethodReferenceUsage` is an extremely expensive
check due to its use of `SuggestedFixes.compilesWithFix`. Maybe
we should drop it altogether? -->
<!-- XXX: We shouldn't disable the `Refaster` check here.
Rather, we should fix whatever bug causes templates to flag
their own code. -->
<!-- XXX: Find a way to assert that test code (both inline
`BugChecker` test code and the Refaster test files) does not
exhibit anti-patterns other than those associated with the
check/template under test. Ideally all test cases are realistic. -->
<error-prone.self-check-args>-Xep:MethodReferenceUsage:OFF -Xep:Refaster:OFF</error-prone.self-check-args>
</properties>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths combine.children="append">
<path>
<groupId>${project.groupId}</groupId>
<artifactId>error-prone-contrib</artifactId>
<version>${project.version}</version>
</path>
</annotationProcessorPaths>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<profile>
<!-- By default we verify various aspects of a module and the
artifact(s) it produces. We define these checks in a profile so
@@ -1399,7 +1436,8 @@
-XepOpt:NullAway:AssertsEnabled=true
-XepOpt:NullAway:CheckOptionalEmptiness=true
<!-- Append additional custom arguments. -->
${error-prone.args}
${error-prone.patch-args}
${error-prone.self-check-args}
</arg>
<!-- The Error Prone plugin makes certain
assumptions about the state of the AST at the
@@ -1425,7 +1463,7 @@
</property>
</activation>
<properties>
<error-prone.args>-XepPatchChecks:${error-prone.patch-checks} -XepPatchLocation:IN_PLACE</error-prone.args>
<error-prone.patch-args>-XepPatchChecks:${error-prone.patch-checks} -XepPatchLocation:IN_PLACE</error-prone.patch-args>
</properties>
</profile>
<profile>