From 8e5314cbde55b982a761a00b2e17dfa02bb9cb90 Mon Sep 17 00:00:00 2001 From: Phil Werli Date: Thu, 20 Oct 2022 08:55:49 +0200 Subject: [PATCH] Introduce Guava `Preconditions` Refaster rules (#292) --- .../refasterrules/AssortedRules.java | 22 +++ .../refasterrules/PreconditionsRules.java | 176 ++++++++++++++++++ .../refasterrules/RefasterRulesTest.java | 1 + .../refasterrules/AssortedRulesTestInput.java | 6 + .../AssortedRulesTestOutput.java | 8 +- .../PreconditionsRulesTestInput.java | 59 ++++++ .../PreconditionsRulesTestOutput.java | 47 +++++ 7 files changed, 317 insertions(+), 2 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PreconditionsRules.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestOutput.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssortedRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssortedRules.java index 1d5523736f..e30b2a973e 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssortedRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssortedRules.java @@ -46,11 +46,33 @@ int before(int index, int size) { } @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) int after(int index, int size) { return checkIndex(index, size); } } + /** + * Prefer {@link Objects#checkIndex(int, int)} over less descriptive or more verbose alternatives. + * + *

If a custom error message is desired, consider using Guava's {@link + * com.google.common.base.Preconditions#checkElementIndex(int, int, String)}. + */ + static final class CheckIndexConditional { + @BeforeTemplate + void before(int index, int size) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException(); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int index, int size) { + checkIndex(index, size); + } + } + // XXX: We could add a rule for `new EnumMap(Map m)`, but that constructor does // not allow an empty non-EnumMap to be provided. static final class CreateEnumMap, V> { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PreconditionsRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PreconditionsRules.java new file mode 100644 index 0000000000..a117aea41c --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PreconditionsRules.java @@ -0,0 +1,176 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkElementIndex; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkPositionIndex; +import static com.google.common.base.Preconditions.checkState; +import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; + +import com.google.common.base.Preconditions; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import com.google.errorprone.refaster.annotation.UseImportPolicy; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +/** Refaster templates related to statements dealing with {@link Preconditions}. */ +@OnlineDocumentation +final class PreconditionsRules { + private PreconditionsRules() {} + + /** Prefer {@link Preconditions#checkArgument(boolean)} over more verbose alternatives. */ + static final class CheckArgument { + @BeforeTemplate + void before(boolean condition) { + if (condition) { + throw new IllegalArgumentException(); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean condition) { + checkArgument(!condition); + } + } + + /** Prefer {@link Preconditions#checkArgument(boolean, Object)} over more verbose alternatives. */ + static final class CheckArgumentWithMessage { + @BeforeTemplate + void before(boolean condition, String message) { + if (condition) { + throw new IllegalArgumentException(message); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean condition, String message) { + checkArgument(!condition, message); + } + } + + /** + * Prefer {@link Preconditions#checkElementIndex(int, int, String)} over less descriptive or more + * verbose alternatives. + * + *

Note that the two-argument {@link Preconditions#checkElementIndex(int, int)} is better + * replaced with {@link java.util.Objects#checkIndex(int, int)}. + */ + static final class CheckElementIndexWithMessage { + @BeforeTemplate + void before(int index, int size, String message) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException(message); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int index, int size, String message) { + checkElementIndex(index, size, message); + } + } + + /** Prefer {@link Preconditions#checkNotNull(Object)} over more verbose alternatives. */ + static final class CheckNotNull { + @BeforeTemplate + void before(T object) { + if (object == null) { + throw new NullPointerException(); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(T object) { + checkNotNull(object); + } + } + + /** Prefer {@link Preconditions#checkNotNull(Object, Object)} over more verbose alternatives. */ + static final class CheckNotNullWithMessage { + @BeforeTemplate + void before(T object, String message) { + if (object == null) { + throw new NullPointerException(message); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(T object, String message) { + checkNotNull(object, message); + } + } + + /** + * Prefer {@link Preconditions#checkPositionIndex(int, int)} over less descriptive or more verbose + * alternatives. + */ + static final class CheckPositionIndex { + @BeforeTemplate + void before(int index, int size) { + if (index < 0 || index > size) { + throw new IndexOutOfBoundsException(); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int index, int size) { + checkPositionIndex(index, size); + } + } + + /** + * Prefer {@link Preconditions#checkPositionIndex(int, int, String)} over less descriptive or more + * verbose alternatives. + */ + static final class CheckPositionIndexWithMessage { + @BeforeTemplate + void before(int index, int size, String message) { + if (index < 0 || index > size) { + throw new IndexOutOfBoundsException(message); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(int index, int size, String message) { + checkPositionIndex(index, size, message); + } + } + + /** Prefer {@link Preconditions#checkState(boolean)} over more verbose alternatives. */ + static final class CheckState { + @BeforeTemplate + void before(boolean condition) { + if (condition) { + throw new IllegalStateException(); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean condition) { + checkState(!condition); + } + } + + /** Prefer {@link Preconditions#checkState(boolean, Object)} over more verbose alternatives. */ + static final class CheckStateWithMessage { + @BeforeTemplate + void before(boolean condition, String message) { + if (condition) { + throw new IllegalStateException(message); + } + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(boolean condition, String message) { + checkState(!condition, message); + } + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java index be11145750..eeb1afdc5f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java @@ -56,6 +56,7 @@ final class RefasterRulesTest { MultimapRules.class, NullRules.class, OptionalRules.class, + PreconditionsRules.class, PrimitiveRules.class, ReactorRules.class, RxJava2AdapterRules.class, diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestInput.java index 6c1398a39c..6c9e16358d 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestInput.java @@ -37,6 +37,12 @@ int testCheckIndex() { return Preconditions.checkElementIndex(0, 1); } + void testCheckIndexConditional() { + if (1 < 0 || 1 >= 2) { + throw new IndexOutOfBoundsException(); + } + } + Map testCreateEnumMap() { return new HashMap<>(); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestOutput.java index 7a9a91cb73..117cf1e6a0 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssortedRulesTestOutput.java @@ -2,6 +2,7 @@ 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; @@ -19,7 +20,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Objects; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -38,7 +38,11 @@ public ImmutableSet elidedTypesAndStaticImports() { } int testCheckIndex() { - return Objects.checkIndex(0, 1); + return checkIndex(0, 1); + } + + void testCheckIndexConditional() { + checkIndex(1, 2); } Map testCreateEnumMap() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestInput.java new file mode 100644 index 0000000000..4979486d02 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestInput.java @@ -0,0 +1,59 @@ +package tech.picnic.errorprone.refasterrules; + +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class PreconditionsRulesTest implements RefasterRuleCollectionTestCase { + void testCheckArgument() { + if ("foo".isEmpty()) { + throw new IllegalArgumentException(); + } + } + + void testCheckArgumentWithMessage() { + if ("foo".isEmpty()) { + throw new IllegalArgumentException("The string is empty"); + } + } + + void testCheckElementIndexWithMessage() { + if (1 < 0 || 1 >= 2) { + throw new IndexOutOfBoundsException("My index"); + } + } + + void testCheckNotNull() { + if ("foo" == null) { + throw new NullPointerException(); + } + } + + void testCheckNotNullWithMessage() { + if ("foo" == null) { + throw new NullPointerException("The string is null"); + } + } + + void testCheckPositionIndex() { + if (1 < 0 || 1 > 2) { + throw new IndexOutOfBoundsException(); + } + } + + void testCheckPositionIndexWithMessage() { + if (1 < 0 || 1 > 2) { + throw new IndexOutOfBoundsException("My position"); + } + } + + void testCheckState() { + if ("foo".isEmpty()) { + throw new IllegalStateException(); + } + } + + void testCheckStateWithMessage() { + if ("foo".isEmpty()) { + throw new IllegalStateException("The string is empty"); + } + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestOutput.java new file mode 100644 index 0000000000..965dab5989 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/PreconditionsRulesTestOutput.java @@ -0,0 +1,47 @@ +package tech.picnic.errorprone.refasterrules; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkElementIndex; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkPositionIndex; +import static com.google.common.base.Preconditions.checkState; + +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class PreconditionsRulesTest implements RefasterRuleCollectionTestCase { + void testCheckArgument() { + checkArgument(!"foo".isEmpty()); + } + + void testCheckArgumentWithMessage() { + checkArgument(!"foo".isEmpty(), "The string is empty"); + } + + void testCheckElementIndexWithMessage() { + checkElementIndex(1, 2, "My index"); + } + + void testCheckNotNull() { + checkNotNull("foo"); + } + + void testCheckNotNullWithMessage() { + checkNotNull("foo", "The string is null"); + } + + void testCheckPositionIndex() { + checkPositionIndex(1, 2); + } + + void testCheckPositionIndexWithMessage() { + checkPositionIndex(1, 2, "My position"); + } + + void testCheckState() { + checkState(!"foo".isEmpty()); + } + + void testCheckStateWithMessage() { + checkState(!"foo".isEmpty(), "The string is empty"); + } +}