From 67c1519f769d6d77392531db7843b929a6cb1e5f Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 28 Dec 2022 07:55:03 +0100 Subject: [PATCH] Suggestions --- .../errorprone/refasterrules/NullRules.java | 35 +++++++++---------- .../refasterrules/NullRulesTestInput.java | 7 ++-- .../refasterrules/NullRulesTestOutput.java | 5 +-- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/NullRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/NullRules.java index 9d50eb96109..578ef91c590 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/NullRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/NullRules.java @@ -5,9 +5,9 @@ import static java.util.Objects.requireNonNullElseGet; import com.google.common.base.MoreObjects; +import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; -import com.google.errorprone.refaster.annotation.Matches; import com.google.errorprone.refaster.annotation.UseImportPolicy; import java.util.Objects; import java.util.Optional; @@ -48,21 +48,17 @@ boolean after(@Nullable Object object) { } /** - * Prefer {@link Objects#requireNonNullElse(Object, Object)} over the Guava and Optional + * Prefer {@link Objects#requireNonNullElse(Object, Object)} over non-JDK or more contrived * alternatives. */ // XXX: This rule is not valid in case `second` is `@Nullable`: in that case the Guava and - // Optional variants - // will return `null`, while the Objects.requireNonNullElse variant will throw an NPE. + // `Optional` variants will return `null`, where the `requireNonNullElse` alternative will throw + // an NPE. static final class RequireNonNullElse { @BeforeTemplate T before(T first, T second) { - return MoreObjects.firstNonNull(first, second); - } - - @BeforeTemplate - T beforeOptional(T first, T second) { - return Optional.ofNullable(first).orElse(second); + return Refaster.anyOf( + MoreObjects.firstNonNull(first, second), Optional.ofNullable(first).orElse(second)); } @AfterTemplate @@ -73,21 +69,22 @@ T after(T first, T second) { } /** - * Prefer {@link Objects#requireNonNullElseGet(Object, Supplier)} over {@link Optional}{@code - * .ofNullable(Object).orElseGet(Supplier)} + * Prefer {@link Objects#requireNonNullElseGet(Object, Supplier)} over more contrived + * alternatives. */ - // XXX: This rule is not valid in case `supplier` may return `null`: Optional will return `null`, - // while the `requireNonNullElseGet` replacement will throw an NPE. - static final class RequireNonNullElseGet> { + // XXX: This rule is not valid in case `supplier` yields `@Nullable` values: in that case the + // `Optional` variant will return `null`, where the `requireNonNullElseGet` alternative will throw + // an NPE. + static final class RequireNonNullElseGet { @BeforeTemplate - T before(T object, S defaultSupplier) { - return Optional.ofNullable(object).orElseGet(defaultSupplier); + T before(T object, Supplier supplier) { + return Optional.ofNullable(object).orElseGet(supplier); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - T after(T object, S defaultSupplier) { - return requireNonNullElseGet(object, defaultSupplier); + T after(T object, Supplier supplier) { + return requireNonNullElseGet(object, supplier); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java index 2e1dcd70c6b..a84a12a27e6 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestInput.java @@ -3,13 +3,14 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; import java.util.Objects; +import java.util.Optional; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class NullRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(MoreObjects.class); + return ImmutableSet.of(MoreObjects.class, Optional.class); } boolean testIsNull() { @@ -22,11 +23,11 @@ boolean testIsNotNull() { ImmutableSet testRequireNonNullElse() { return ImmutableSet.of( - MoreObjects.firstNonNull("foo", "bar"), java.util.Optional.ofNullable("foo").orElse("bar")); + MoreObjects.firstNonNull("foo", "bar"), Optional.ofNullable("baz").orElse("qux")); } String testRequireNonNullElseGet() { - return java.util.Optional.ofNullable("foo").orElseGet(() -> "bar"); + return Optional.ofNullable("foo").orElseGet(() -> "bar"); } long testIsNullFunction() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java index a494ef2a06a..6fa2228d914 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/NullRulesTestOutput.java @@ -6,13 +6,14 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; import java.util.Objects; +import java.util.Optional; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class NullRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(MoreObjects.class); + return ImmutableSet.of(MoreObjects.class, Optional.class); } boolean testIsNull() { @@ -24,7 +25,7 @@ boolean testIsNotNull() { } ImmutableSet testRequireNonNullElse() { - return ImmutableSet.of(requireNonNullElse("foo", "bar"), requireNonNullElse("foo", "bar")); + return ImmutableSet.of(requireNonNullElse("foo", "bar"), requireNonNullElse("baz", "qux")); } String testRequireNonNullElseGet() {