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 0a77f8c5e6d..9d50eb96109 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 @@ -2,13 +2,17 @@ import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; import static java.util.Objects.requireNonNullElse; +import static java.util.Objects.requireNonNullElseGet; import com.google.common.base.MoreObjects; 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; import java.util.function.Predicate; +import java.util.function.Supplier; import org.jspecify.annotations.Nullable; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; @@ -43,15 +47,24 @@ boolean after(@Nullable Object object) { } } - /** Prefer {@link Objects#requireNonNullElse(Object, Object)} over the Guava alternative. */ - // XXX: This rule is not valid in case `second` is `@Nullable`: in that case the Guava variant - // will return `null`, while the JDK variant will throw an NPE. + /** + * Prefer {@link Objects#requireNonNullElse(Object, Object)} over the Guava and Optional + * 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. 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); + } + @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) T after(T first, T second) { @@ -59,6 +72,25 @@ T after(T first, T second) { } } + /** + * Prefer {@link Objects#requireNonNullElseGet(Object, Supplier)} over {@link Optional}{@code + * .ofNullable(Object).orElseGet(Supplier)} + */ + // 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> { + @BeforeTemplate + T before(T object, S defaultSupplier) { + return Optional.ofNullable(object).orElseGet(defaultSupplier); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + T after(T object, S defaultSupplier) { + return requireNonNullElseGet(object, defaultSupplier); + } + } + /** Prefer {@link Objects#isNull(Object)} over the equivalent lambda function. */ static final class IsNullFunction { @BeforeTemplate 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 9163673a5f6..2e1dcd70c6b 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 @@ -20,8 +20,13 @@ boolean testIsNotNull() { return Objects.nonNull("foo"); } - String testRequireNonNullElse() { - return MoreObjects.firstNonNull("foo", "bar"); + ImmutableSet testRequireNonNullElse() { + return ImmutableSet.of( + MoreObjects.firstNonNull("foo", "bar"), java.util.Optional.ofNullable("foo").orElse("bar")); + } + + String testRequireNonNullElseGet() { + return java.util.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 4867d0089d3..a494ef2a06a 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 @@ -1,6 +1,7 @@ package tech.picnic.errorprone.refasterrules; import static java.util.Objects.requireNonNullElse; +import static java.util.Objects.requireNonNullElseGet; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; @@ -22,8 +23,12 @@ boolean testIsNotNull() { return "foo" != null; } - String testRequireNonNullElse() { - return requireNonNullElse("foo", "bar"); + ImmutableSet testRequireNonNullElse() { + return ImmutableSet.of(requireNonNullElse("foo", "bar"), requireNonNullElse("foo", "bar")); + } + + String testRequireNonNullElseGet() { + return requireNonNullElseGet("foo", () -> "bar"); } long testIsNullFunction() {