From 361c1f34fb4d92cf774ae1d1c0b4f1c501d736de Mon Sep 17 00:00:00 2001 From: Benedek Halasi Date: Wed, 4 Jan 2023 01:33:36 +0100 Subject: [PATCH 1/3] Introduce `Map#getOrDefault` Refaster rules adds rules to fix #431. --- .../errorprone/refasterrules/MapRules.java | 19 +++++++++++++++++++ .../refasterrules/MapRulesTestInput.java | 8 +++++++- .../refasterrules/MapRulesTestOutput.java | 8 +++++++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java index cdf99f5e60..e2147cacd4 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java @@ -1,5 +1,7 @@ package tech.picnic.errorprone.refasterrules; +import static java.util.Objects.requireNonNullElse; + import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; @@ -43,6 +45,23 @@ V after(Map map, T key) { } } + /** + * Prefer {@link Map#getOrDefault(Object, Object)} over more contrived alternatives. Note that + * this method may return null instead of throwing a NPE. + */ + static final class MapGetOrDefault { + @BeforeTemplate + V before(Map map, T key, V defaultValue) { + return requireNonNullElse(map.get(key), defaultValue); + } + + @AfterTemplate + @Nullable + V after(Map map, T key, V defaultValue) { + return map.getOrDefault(key, defaultValue); + } + } + /** Prefer {@link Map#isEmpty()} over more contrived alternatives. */ static final class MapIsEmpty { @BeforeTemplate diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java index a8dc6e7fa4..a539799cdd 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java @@ -1,5 +1,7 @@ package tech.picnic.errorprone.refasterrules; +import static java.util.Objects.requireNonNullElse; + import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.math.RoundingMode; @@ -11,7 +13,7 @@ final class MapRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(HashMap.class); + return ImmutableSet.of(HashMap.class, requireNonNullElse("foo", "bar")); } Map testCreateEnumMap() { @@ -22,6 +24,10 @@ String testMapGetOrNull() { return ImmutableMap.of(1, "foo").getOrDefault("bar", null); } + String testMapGetOrDefault() { + return requireNonNullElse(ImmutableMap.of(1, "foo").get("bar"), "baz"); + } + ImmutableSet testMapIsEmpty() { return ImmutableSet.of( ImmutableMap.of("foo", 1).keySet().isEmpty(), diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java index 650cd4094f..22d3fd6065 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java @@ -1,5 +1,7 @@ package tech.picnic.errorprone.refasterrules; +import static java.util.Objects.requireNonNullElse; + import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.math.RoundingMode; @@ -12,7 +14,7 @@ final class MapRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(HashMap.class); + return ImmutableSet.of(HashMap.class, requireNonNullElse("foo", "bar")); } Map testCreateEnumMap() { @@ -23,6 +25,10 @@ String testMapGetOrNull() { return ImmutableMap.of(1, "foo").get("bar"); } + String testMapGetOrDefault() { + return ImmutableMap.of(1, "foo").getOrDefault("bar", "baz"); + } + ImmutableSet testMapIsEmpty() { return ImmutableSet.of( ImmutableMap.of("foo", 1).isEmpty(), From 857973d2ce029b4b508ee9bb4172d654027599fb Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 4 Jan 2023 08:45:39 +0100 Subject: [PATCH 2/3] Suggestions --- .../tech/picnic/errorprone/refasterrules/MapRules.java | 9 ++++----- .../errorprone/refasterrules/MapRulesTestInput.java | 2 +- .../errorprone/refasterrules/MapRulesTestOutput.java | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java index e2147cacd4..835b9d1e83 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java @@ -45,10 +45,10 @@ V after(Map map, T key) { } } - /** - * Prefer {@link Map#getOrDefault(Object, Object)} over more contrived alternatives. Note that - * this method may return null instead of throwing a NPE. - */ + /** Prefer {@link Map#getOrDefault(Object, Object)} over more contrived alternatives. */ + // Note that + // XXX: `requireNonNullElse` throws an NPE if the second argument is `null`, while the alternative + // does not. static final class MapGetOrDefault { @BeforeTemplate V before(Map map, T key, V defaultValue) { @@ -56,7 +56,6 @@ V before(Map map, T key, V defaultValue) { } @AfterTemplate - @Nullable V after(Map map, T key, V defaultValue) { return map.getOrDefault(key, defaultValue); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java index a539799cdd..707243ba4d 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java @@ -13,7 +13,7 @@ final class MapRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(HashMap.class, requireNonNullElse("foo", "bar")); + return ImmutableSet.of(HashMap.class, requireNonNullElse(null, null)); } Map testCreateEnumMap() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java index 22d3fd6065..9214fa6f79 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java @@ -14,7 +14,7 @@ final class MapRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(HashMap.class, requireNonNullElse("foo", "bar")); + return ImmutableSet.of(HashMap.class, requireNonNullElse(null, null)); } Map testCreateEnumMap() { From 6852705d91519a87197107a8fa292e94275ce0d4 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Thu, 5 Jan 2023 17:46:22 +0100 Subject: [PATCH 3/3] Tweak --- .../java/tech/picnic/errorprone/refasterrules/MapRules.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java index 835b9d1e83..b9610c255c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java @@ -46,9 +46,8 @@ V after(Map map, T key) { } /** Prefer {@link Map#getOrDefault(Object, Object)} over more contrived alternatives. */ - // Note that - // XXX: `requireNonNullElse` throws an NPE if the second argument is `null`, while the alternative - // does not. + // XXX: Note that `requireNonNullElse` throws an NPE if the second argument is `null`, while the + // alternative does not. static final class MapGetOrDefault { @BeforeTemplate V before(Map map, T key, V defaultValue) {