From fcaa1f7068c6066856205d67fc8739e5fee5fad2 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Wed, 9 Nov 2022 08:54:43 +0100 Subject: [PATCH] Improve and extend Refaster `Map` rules (#337) Summary of changes: - Move relevant rules from `AssertJRules` to `AssertJMapRules` and introduce associated tests. - Extract relevant rules from `AssortedRules` to the new `MapRules` Refaster rule collection. - Add a few new rules to both `AssertJMapRules` and `MapRules`. --- .../refasterrules/AssertJMapRules.java | 211 ++++++++++++++++++ .../refasterrules/AssertJRules.java | 177 +-------------- .../refasterrules/AssortedRules.java | 55 ----- .../errorprone/refasterrules/MapRules.java | 108 +++++++++ .../refasterrules/RefasterRulesTest.java | 1 + .../AssertJMapRulesTestInput.java | 133 ++++++++++- .../AssertJMapRulesTestOutput.java | 133 ++++++++++- .../refasterrules/AssortedRulesTestInput.java | 21 -- .../AssortedRulesTestOutput.java | 22 -- .../refasterrules/MapRulesTestInput.java | 47 ++++ .../refasterrules/MapRulesTestOutput.java | 48 ++++ 11 files changed, 675 insertions(+), 281 deletions(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJMapRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJMapRules.java index 59f99e9504..4f3d782a7f 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJMapRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJMapRules.java @@ -1,16 +1,148 @@ package tech.picnic.errorprone.refasterrules; +import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.ImmutableBiMap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultiset; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableSortedMultiset; +import com.google.common.collect.ImmutableSortedSet; +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.UseImportPolicy; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.Map; +import java.util.TreeMap; +import java.util.TreeSet; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractBooleanAssert; import org.assertj.core.api.AbstractMapAssert; +import org.assertj.core.api.MapAssert; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; @OnlineDocumentation final class AssertJMapRules { private AssertJMapRules() {} + // XXX: Reduce boilerplate using a `Matcher` that identifies "empty" instances. + static final class AbstractMapAssertIsEmpty { + @BeforeTemplate + @SuppressWarnings("unchecked") + void before(AbstractMapAssert mapAssert) { + Refaster.anyOf( + mapAssert.containsExactlyEntriesOf( + Refaster.anyOf( + ImmutableMap.of(), + ImmutableBiMap.of(), + ImmutableSortedMap.of(), + new HashMap<>(), + new LinkedHashMap<>(), + new TreeMap<>())), + mapAssert.hasSameSizeAs( + Refaster.anyOf( + ImmutableMap.of(), + ImmutableBiMap.of(), + ImmutableSortedMap.of(), + new HashMap<>(), + new LinkedHashMap<>(), + new TreeMap<>())), + mapAssert.isEqualTo( + Refaster.anyOf( + ImmutableMap.of(), + ImmutableBiMap.of(), + ImmutableSortedMap.of(), + new HashMap<>(), + new LinkedHashMap<>(), + new TreeMap<>())), + mapAssert.containsOnlyKeys( + Refaster.anyOf( + ImmutableList.of(), + new ArrayList<>(), + ImmutableSet.of(), + new HashSet<>(), + new LinkedHashSet<>(), + ImmutableSortedSet.of(), + new TreeSet<>(), + ImmutableMultiset.of(), + ImmutableSortedMultiset.of())), + mapAssert.containsExactly(), + mapAssert.containsOnly(), + mapAssert.containsOnlyKeys()); + } + + @AfterTemplate + void after(AbstractMapAssert mapAssert) { + mapAssert.isEmpty(); + } + } + + static final class AssertThatMapIsEmpty { + @BeforeTemplate + void before(Map map) { + Refaster.anyOf( + assertThat(map).hasSize(0), + assertThat(map.isEmpty()).isTrue(), + assertThat(map.size()).isEqualTo(0L), + assertThat(map.size()).isNotPositive()); + } + + @BeforeTemplate + void before2(Map map) { + assertThat(Refaster.anyOf(map.keySet(), map.values(), map.entrySet())).isEmpty(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + void after(Map map) { + assertThat(map).isEmpty(); + } + } + + static final class AbstractMapAssertIsNotEmpty { + @BeforeTemplate + AbstractMapAssert before(AbstractMapAssert mapAssert) { + return mapAssert.isNotEqualTo( + Refaster.anyOf( + ImmutableMap.of(), + ImmutableBiMap.of(), + ImmutableSortedMap.of(), + new HashMap<>(), + new LinkedHashMap<>(), + new TreeMap<>())); + } + + @AfterTemplate + AbstractMapAssert after(AbstractMapAssert mapAssert) { + return mapAssert.isNotEmpty(); + } + } + + static final class AssertThatMapIsNotEmpty { + @BeforeTemplate + AbstractAssert before(Map map) { + return Refaster.anyOf( + assertThat(map.isEmpty()).isFalse(), + assertThat(map.size()).isNotEqualTo(0), + assertThat(map.size()).isPositive(), + assertThat(Refaster.anyOf(map.keySet(), map.values(), map.entrySet())).isNotEmpty()); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + MapAssert after(Map map) { + return assertThat(map).isNotEmpty(); + } + } + static final class AbstractMapAssertContainsExactlyInAnyOrderEntriesOf { @BeforeTemplate AbstractMapAssert before(AbstractMapAssert mapAssert, Map map) { @@ -34,4 +166,83 @@ static final class AbstractMapAssertContainsExactlyEntriesOf { return mapAssert.containsExactlyEntriesOf(ImmutableMap.of(key, value)); } } + + static final class AssertThatMapHasSize { + @BeforeTemplate + AbstractAssert before(Map map, int length) { + return Refaster.anyOf( + assertThat(map.size()).isEqualTo(length), + assertThat(Refaster.anyOf(map.keySet(), map.values(), map.entrySet())).hasSize(length)); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + MapAssert after(Map map, int length) { + return assertThat(map).hasSize(length); + } + } + + static final class AbstractMapAssertHasSameSizeAs { + @BeforeTemplate + AbstractMapAssert before(AbstractMapAssert mapAssert, Map map) { + return mapAssert.hasSize(map.size()); + } + + @AfterTemplate + AbstractMapAssert after(AbstractMapAssert mapAssert, Map map) { + return mapAssert.hasSameSizeAs(map); + } + } + + static final class AssertThatMapContainsKey { + @BeforeTemplate + AbstractBooleanAssert before(Map map, K key) { + return assertThat(map.containsKey(key)).isTrue(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + MapAssert after(Map map, K key) { + return assertThat(map).containsKey(key); + } + } + + static final class AssertThatMapDoesNotContainKey { + @BeforeTemplate + AbstractBooleanAssert before(Map map, K key) { + return assertThat(map.containsKey(key)).isFalse(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + MapAssert after(Map map, K key) { + return assertThat(map).doesNotContainKey(key); + } + } + + static final class AssertThatMapContainsValue { + @BeforeTemplate + AbstractBooleanAssert before(Map map, V value) { + return assertThat(map.containsValue(value)).isTrue(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + MapAssert after(Map map, V value) { + return assertThat(map).containsValue(value); + } + } + + static final class AssertThatMapDoesNotContainValue { + @BeforeTemplate + AbstractBooleanAssert before(Map map, V value) { + return assertThat(map.containsValue(value)).isFalse(); + } + + @AfterTemplate + @UseImportPolicy(STATIC_IMPORT_ALWAYS) + MapAssert after(Map map, V value) { + return assertThat(map).doesNotContainValue(value); + } + } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java index 8ee7396d9a..f07102e2d3 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java @@ -3,12 +3,9 @@ import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS; import static org.assertj.core.api.Assertions.assertThat; -import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultiset; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.ImmutableSortedMultiset; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; @@ -22,9 +19,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -32,19 +27,16 @@ import java.util.OptionalInt; import java.util.OptionalLong; import java.util.Set; -import java.util.TreeMap; import java.util.TreeSet; import java.util.function.Predicate; import java.util.stream.Collector; import java.util.stream.Stream; import org.assertj.core.api.AbstractAssert; -import org.assertj.core.api.AbstractBooleanAssert; import org.assertj.core.api.AbstractCollectionAssert; import org.assertj.core.api.AbstractComparableAssert; import org.assertj.core.api.AbstractDoubleAssert; import org.assertj.core.api.AbstractIntegerAssert; import org.assertj.core.api.AbstractLongAssert; -import org.assertj.core.api.AbstractMapAssert; import org.assertj.core.api.IterableAssert; import org.assertj.core.api.ListAssert; import org.assertj.core.api.MapAssert; @@ -566,173 +558,8 @@ static final class AssertThatMultisetsAreEqual { // Map // - static final class AssertThatMapIsEmpty { - @BeforeTemplate - @SuppressWarnings("unchecked") - void before(AbstractMapAssert mapAssert) { - Refaster.anyOf( - mapAssert.containsExactlyEntriesOf( - Refaster.anyOf( - ImmutableMap.of(), - ImmutableBiMap.of(), - ImmutableSortedMap.of(), - new HashMap<>(), - new LinkedHashMap<>(), - new TreeMap<>())), - mapAssert.hasSameSizeAs( - Refaster.anyOf( - ImmutableMap.of(), - ImmutableBiMap.of(), - ImmutableSortedMap.of(), - new HashMap<>(), - new LinkedHashMap<>(), - new TreeMap<>())), - mapAssert.isEqualTo( - Refaster.anyOf( - ImmutableMap.of(), - ImmutableBiMap.of(), - ImmutableSortedMap.of(), - new HashMap<>(), - new LinkedHashMap<>(), - new TreeMap<>())), - mapAssert.containsOnlyKeys( - Refaster.anyOf( - ImmutableList.of(), - new ArrayList<>(), - ImmutableSet.of(), - new HashSet<>(), - new LinkedHashSet<>(), - ImmutableSortedSet.of(), - new TreeSet<>(), - ImmutableMultiset.of(), - ImmutableSortedMultiset.of())), - mapAssert.containsExactly(), - mapAssert.containsOnly(), - mapAssert.containsOnlyKeys()); - } - - @AfterTemplate - void after(AbstractMapAssert mapAssert) { - mapAssert.isEmpty(); - } - } - - // XXX: Find a better name. - static final class AssertThatMapIsEmpty2 { - @BeforeTemplate - void before(Map map) { - Refaster.anyOf( - assertThat(map).hasSize(0), - assertThat(map.isEmpty()).isTrue(), - assertThat(map.size()).isEqualTo(0L), - assertThat(map.size()).isNotPositive()); - } - - @BeforeTemplate - void before2(Map map) { - assertThat(Refaster.anyOf(map.keySet(), map.values())).isEmpty(); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - void after(Map map) { - assertThat(map).isEmpty(); - } - } - - static final class AssertThatMapIsNotEmpty { - @BeforeTemplate - AbstractMapAssert before(AbstractMapAssert mapAssert) { - return mapAssert.isNotEqualTo( - Refaster.anyOf( - ImmutableMap.of(), - ImmutableBiMap.of(), - ImmutableSortedMap.of(), - new HashMap<>(), - new LinkedHashMap<>(), - new TreeMap<>())); - } - - @AfterTemplate - AbstractMapAssert after(AbstractMapAssert mapAssert) { - return mapAssert.isNotEmpty(); - } - } - - // XXX: Find a better name. - static final class AssertThatMapIsNotEmpty2 { - @BeforeTemplate - AbstractAssert before(Map map) { - return Refaster.anyOf( - assertThat(map.isEmpty()).isFalse(), - assertThat(map.size()).isNotEqualTo(0), - assertThat(map.size()).isPositive(), - assertThat(Refaster.anyOf(map.keySet(), map.values())).isNotEmpty()); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - MapAssert after(Map map) { - return assertThat(map).isNotEmpty(); - } - } - - static final class AssertThatMapHasSize { - @BeforeTemplate - AbstractAssert before(Map map, int length) { - return Refaster.anyOf( - assertThat(map.size()).isEqualTo(length), - assertThat(Refaster.anyOf(map.keySet(), map.values())).hasSize(length)); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - MapAssert after(Map map, int length) { - return assertThat(map).hasSize(length); - } - } - - static final class AssertThatMapsHaveSameSize { - @BeforeTemplate - AbstractAssert before(Map map1, Map map2) { - return assertThat(map1) - .hasSize(Refaster.anyOf(map2.size(), map2.keySet().size(), map2.values().size())); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - MapAssert after(Map map1, Map map2) { - return assertThat(map1).hasSameSizeAs(map2); - } - } - - // XXX: Should also add a rule (elsewhere) to simplify `map.keySet().contains(key)`. - static final class AssertThatMapContainsKey { - @BeforeTemplate - AbstractBooleanAssert before(Map map, K key) { - return assertThat(map.containsKey(key)).isTrue(); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - MapAssert after(Map map, K key) { - return assertThat(map).containsKey(key); - } - } - - static final class AssertThatMapDoesNotContainKey { - @BeforeTemplate - AbstractBooleanAssert before(Map map, K key) { - return assertThat(map.containsKey(key)).isFalse(); - } - - @AfterTemplate - @UseImportPolicy(STATIC_IMPORT_ALWAYS) - MapAssert after(Map map, K key) { - return assertThat(map).doesNotContainKey(key); - } - } - + // XXX: To match in all cases there'll need to be a `@BeforeTemplate` variant for each + // `assertThat` overload. Consider defining a `BugChecker` instead. static final class AssertThatMapContainsEntry { @BeforeTemplate ObjectAssert before(Map map, K key, V value) { 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 c822da93d9..80d97c7b25 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 @@ -19,11 +19,8 @@ import com.google.errorprone.refaster.annotation.UseImportPolicy; import java.util.Collection; import java.util.Collections; -import java.util.EnumMap; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; -import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Stream; @@ -73,32 +70,6 @@ void after(int index, int 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> { - @BeforeTemplate - Map before() { - return new HashMap<>(); - } - - @AfterTemplate - Map after() { - return new EnumMap<>(Refaster.clazz()); - } - } - - static final class MapGetOrNull { - @BeforeTemplate - @Nullable V before(Map map, L key) { - return map.getOrDefault(key, null); - } - - @AfterTemplate - @Nullable V after(Map map, L key) { - return map.get(key); - } - } - /** * Use {@link Sets#toImmutableEnumSet()} when possible, as it is more efficient than {@link * ImmutableSet#toImmutableSet()} and produces a more compact object. @@ -224,32 +195,6 @@ boolean after(Iterable iterable) { } } - /** Don't unnecessarily use {@link Map#entrySet()}. */ - static final class MapKeyStream { - @BeforeTemplate - Stream before(Map map) { - return map.entrySet().stream().map(Map.Entry::getKey); - } - - @AfterTemplate - Stream after(Map map) { - return map.keySet().stream(); - } - } - - /** Don't unnecessarily use {@link Map#entrySet()}. */ - static final class MapValueStream { - @BeforeTemplate - Stream before(Map map) { - return map.entrySet().stream().map(Map.Entry::getValue); - } - - @AfterTemplate - Stream after(Map map) { - return map.values().stream(); - } - } - /** Prefer {@link Splitter#splitToStream(CharSequence)} over less efficient alternatives. */ static final class SplitToStream { @BeforeTemplate 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 new file mode 100644 index 0000000000..cb4fd9bf76 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/MapRules.java @@ -0,0 +1,108 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.errorprone.refaster.Refaster; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import java.util.EnumMap; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; +import org.jspecify.nullness.Nullable; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +/** Refaster rules related to expressions dealing with {@link Map} instances. */ +@OnlineDocumentation +final class MapRules { + private MapRules() {} + + // 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> { + @BeforeTemplate + Map before() { + return new HashMap<>(); + } + + @AfterTemplate + Map after() { + return new EnumMap<>(Refaster.clazz()); + } + } + + static final class MapGetOrNull { + @BeforeTemplate + @Nullable V before(Map map, T key) { + return map.getOrDefault(key, null); + } + + @AfterTemplate + @Nullable V after(Map map, T key) { + return map.get(key); + } + } + + /** Prefer {@link Map#size()} over more contrived alternatives. */ + static final class MapSize { + @BeforeTemplate + int before(Map map) { + return Refaster.anyOf(map.keySet(), map.values(), map.entrySet()).size(); + } + + @AfterTemplate + int after(Map map) { + return map.size(); + } + } + + /** Prefer {@link Map#containsKey(Object)} over more contrived alternatives. */ + static final class MapContainsKey { + @BeforeTemplate + boolean before(Map map, T key) { + return map.keySet().contains(key); + } + + @AfterTemplate + boolean after(Map map, T key) { + return map.containsKey(key); + } + } + + /** Prefer {@link Map#containsValue(Object)} over more contrived alternatives. */ + static final class MapContainsValue { + @BeforeTemplate + boolean before(Map map, T value) { + return map.values().contains(value); + } + + @AfterTemplate + boolean after(Map map, T value) { + return map.containsValue(value); + } + } + + /** Don't unnecessarily use {@link Map#entrySet()}. */ + static final class MapKeyStream { + @BeforeTemplate + Stream before(Map map) { + return map.entrySet().stream().map(Map.Entry::getKey); + } + + @AfterTemplate + Stream after(Map map) { + return map.keySet().stream(); + } + } + + /** Don't unnecessarily use {@link Map#entrySet()}. */ + static final class MapValueStream { + @BeforeTemplate + Stream before(Map map) { + return map.entrySet().stream().map(Map.Entry::getValue); + } + + @AfterTemplate + Stream after(Map map) { + return map.values().stream(); + } + } +} 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 eeb1afdc5f..1129ca80f6 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 @@ -52,6 +52,7 @@ final class RefasterRulesTest { JUnitRules.class, LongStreamRules.class, MapEntryRules.class, + MapRules.class, MockitoRules.class, MultimapRules.class, NullRules.class, diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestInput.java index 064430f62a..82436281ab 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestInput.java @@ -2,18 +2,143 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.google.common.collect.ImmutableBiMap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import org.assertj.core.api.AbstractMapAssert; +import com.google.common.collect.ImmutableMultiset; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableSortedMultiset; +import com.google.common.collect.ImmutableSortedSet; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.TreeMap; +import java.util.TreeSet; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractObjectAssert; +import org.assertj.core.api.MapAssert; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class AssertJMapRulesTest implements RefasterRuleCollectionTestCase { - AbstractMapAssert - testAbstractMapAssertContainsExactlyInAnyOrderEntriesOf() { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of( + ArrayList.class, + HashMap.class, + HashSet.class, + ImmutableBiMap.class, + ImmutableList.class, + ImmutableMultiset.class, + ImmutableSortedMap.class, + ImmutableSortedMultiset.class, + ImmutableSortedSet.class, + LinkedHashMap.class, + TreeMap.class, + TreeSet.class); + } + + void testAbstractMapAssertIsEmpty() { + assertThat(ImmutableMap.of(1, 0)).containsExactlyEntriesOf(ImmutableMap.of()); + assertThat(ImmutableMap.of(2, 0)).containsExactlyEntriesOf(ImmutableBiMap.of()); + assertThat(ImmutableMap.of(3, 0)).containsExactlyEntriesOf(ImmutableSortedMap.of()); + assertThat(ImmutableMap.of(4, 0)).containsExactlyEntriesOf(new HashMap<>()); + assertThat(ImmutableMap.of(5, 0)).containsExactlyEntriesOf(new LinkedHashMap<>()); + assertThat(ImmutableMap.of(6, 0)).containsExactlyEntriesOf(new TreeMap<>()); + assertThat(ImmutableMap.of(7, 0)).hasSameSizeAs(ImmutableMap.of()); + assertThat(ImmutableMap.of(8, 0)).hasSameSizeAs(ImmutableBiMap.of()); + assertThat(ImmutableMap.of(9, 0)).hasSameSizeAs(ImmutableSortedMap.of()); + assertThat(ImmutableMap.of(10, 0)).hasSameSizeAs(new HashMap<>()); + assertThat(ImmutableMap.of(11, 0)).hasSameSizeAs(new LinkedHashMap<>()); + assertThat(ImmutableMap.of(12, 0)).hasSameSizeAs(new TreeMap<>()); + assertThat(ImmutableMap.of(13, 0)).isEqualTo(ImmutableMap.of()); + assertThat(ImmutableMap.of(14, 0)).isEqualTo(ImmutableBiMap.of()); + assertThat(ImmutableMap.of(15, 0)).isEqualTo(ImmutableSortedMap.of()); + assertThat(ImmutableMap.of(16, 0)).isEqualTo(new HashMap<>()); + assertThat(ImmutableMap.of(17, 0)).isEqualTo(new LinkedHashMap<>()); + assertThat(ImmutableMap.of(18, 0)).isEqualTo(new TreeMap<>()); + assertThat(ImmutableMap.of(19, 0)).containsOnlyKeys(ImmutableList.of()); + assertThat(ImmutableMap.of(20, 0)).containsOnlyKeys(new ArrayList<>()); + assertThat(ImmutableMap.of(21, 0)).containsOnlyKeys(ImmutableSet.of()); + assertThat(ImmutableMap.of(22, 0)).containsOnlyKeys(new HashSet<>()); + assertThat(ImmutableMap.of(23, 0)).containsOnlyKeys(ImmutableSortedSet.of()); + assertThat(ImmutableMap.of(24, 0)).containsOnlyKeys(new TreeSet<>()); + assertThat(ImmutableMap.of(25, 0)).containsOnlyKeys(ImmutableMultiset.of()); + assertThat(ImmutableMap.of(26, 0)).containsOnlyKeys(ImmutableSortedMultiset.of()); + assertThat(ImmutableMap.of(27, 0)).containsExactly(); + assertThat(ImmutableMap.of(28, 0)).containsOnly(); + assertThat(ImmutableMap.of(29, 0)).containsOnlyKeys(); + } + + void testAssertThatMapIsEmpty() { + assertThat(ImmutableMap.of(1, 0)).hasSize(0); + assertThat(ImmutableMap.of(2, 0).isEmpty()).isTrue(); + assertThat(ImmutableMap.of(3, 0).size()).isEqualTo(0L); + assertThat(ImmutableMap.of(4, 0).size()).isNotPositive(); + assertThat(ImmutableMap.of(5, 0).keySet()).isEmpty(); + assertThat(ImmutableMap.of(6, 0).values()).isEmpty(); + assertThat(ImmutableMap.of(7, 0).entrySet()).isEmpty(); + } + + ImmutableSet> testAbstractMapAssertIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableMap.of(1, 0)).isNotEqualTo(ImmutableMap.of()), + assertThat(ImmutableMap.of(2, 0)).isNotEqualTo(ImmutableBiMap.of()), + assertThat(ImmutableMap.of(3, 0)).isNotEqualTo(ImmutableSortedMap.of()), + assertThat(ImmutableMap.of(4, 0)).isNotEqualTo(new HashMap<>()), + assertThat(ImmutableMap.of(5, 0)).isNotEqualTo(new LinkedHashMap<>()), + assertThat(ImmutableMap.of(6, 0)).isNotEqualTo(new TreeMap<>())); + } + + ImmutableSet> testAssertThatMapIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableMap.of(1, 0).isEmpty()).isFalse(), + assertThat(ImmutableMap.of(2, 0).size()).isNotEqualTo(0), + assertThat(ImmutableMap.of(3, 0).size()).isPositive(), + assertThat(ImmutableMap.of(4, 0).keySet()).isNotEmpty(), + assertThat(ImmutableMap.of(5, 0).values()).isNotEmpty(), + assertThat(ImmutableMap.of(6, 0).entrySet()).isNotEmpty()); + } + + MapAssert testAbstractMapAssertContainsExactlyInAnyOrderEntriesOf() { return assertThat(ImmutableMap.of(1, 2, 3, 4)).isEqualTo(ImmutableMap.of(1, 2, 3, 4)); } - AbstractMapAssert testAbstractMapAssertContainsExactlyEntriesOf() { + MapAssert testAbstractMapAssertContainsExactlyEntriesOf() { return assertThat(ImmutableMap.of(1, 2)) .containsExactlyInAnyOrderEntriesOf(ImmutableMap.of(1, 2)); } + + ImmutableSet> testAssertThatMapHasSize() { + return ImmutableSet.of( + assertThat(ImmutableMap.of(1, 2).size()).isEqualTo(1), + assertThat(ImmutableMap.of(3, 4).keySet()).hasSize(1), + assertThat(ImmutableMap.of(5, 6).values()).hasSize(1), + assertThat(ImmutableMap.of(7, 8).entrySet()).hasSize(1)); + } + + MapAssert testAbstractMapAssertHasSameSizeAs() { + return assertThat(ImmutableMap.of(1, 2)).hasSize(ImmutableMap.of(3, 4).size()); + } + + AbstractAssert testAssertThatMapContainsKey() { + return assertThat(ImmutableMap.of(1, 2).containsKey(3)).isTrue(); + } + + AbstractAssert testAssertThatMapDoesNotContainKey() { + return assertThat(ImmutableMap.of(1, 2).containsKey(3)).isFalse(); + } + + AbstractAssert testAssertThatMapContainsValue() { + return assertThat(ImmutableMap.of(1, 2).containsValue(3)).isTrue(); + } + + AbstractAssert testAssertThatMapDoesNotContainValue() { + return assertThat(ImmutableMap.of(1, 2).containsValue(3)).isFalse(); + } + + AbstractObjectAssert testAssertThatMapContainsEntry() { + return assertThat(ImmutableMap.of(1, 2).get(1)).isEqualTo(2); + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestOutput.java index a5e58f23b0..e6087c8267 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJMapRulesTestOutput.java @@ -2,18 +2,143 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.google.common.collect.ImmutableBiMap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import org.assertj.core.api.AbstractMapAssert; +import com.google.common.collect.ImmutableMultiset; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableSortedMultiset; +import com.google.common.collect.ImmutableSortedSet; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.TreeMap; +import java.util.TreeSet; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.api.AbstractObjectAssert; +import org.assertj.core.api.MapAssert; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class AssertJMapRulesTest implements RefasterRuleCollectionTestCase { - AbstractMapAssert - testAbstractMapAssertContainsExactlyInAnyOrderEntriesOf() { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of( + ArrayList.class, + HashMap.class, + HashSet.class, + ImmutableBiMap.class, + ImmutableList.class, + ImmutableMultiset.class, + ImmutableSortedMap.class, + ImmutableSortedMultiset.class, + ImmutableSortedSet.class, + LinkedHashMap.class, + TreeMap.class, + TreeSet.class); + } + + void testAbstractMapAssertIsEmpty() { + assertThat(ImmutableMap.of(1, 0)).isEmpty(); + assertThat(ImmutableMap.of(2, 0)).isEmpty(); + assertThat(ImmutableMap.of(3, 0)).isEmpty(); + assertThat(ImmutableMap.of(4, 0)).isEmpty(); + assertThat(ImmutableMap.of(5, 0)).isEmpty(); + assertThat(ImmutableMap.of(6, 0)).isEmpty(); + assertThat(ImmutableMap.of(7, 0)).isEmpty(); + assertThat(ImmutableMap.of(8, 0)).isEmpty(); + assertThat(ImmutableMap.of(9, 0)).isEmpty(); + assertThat(ImmutableMap.of(10, 0)).isEmpty(); + assertThat(ImmutableMap.of(11, 0)).isEmpty(); + assertThat(ImmutableMap.of(12, 0)).isEmpty(); + assertThat(ImmutableMap.of(13, 0)).isEmpty(); + assertThat(ImmutableMap.of(14, 0)).isEmpty(); + assertThat(ImmutableMap.of(15, 0)).isEmpty(); + assertThat(ImmutableMap.of(16, 0)).isEmpty(); + assertThat(ImmutableMap.of(17, 0)).isEmpty(); + assertThat(ImmutableMap.of(18, 0)).isEmpty(); + assertThat(ImmutableMap.of(19, 0)).isEmpty(); + assertThat(ImmutableMap.of(20, 0)).isEmpty(); + assertThat(ImmutableMap.of(21, 0)).isEmpty(); + assertThat(ImmutableMap.of(22, 0)).isEmpty(); + assertThat(ImmutableMap.of(23, 0)).isEmpty(); + assertThat(ImmutableMap.of(24, 0)).isEmpty(); + assertThat(ImmutableMap.of(25, 0)).isEmpty(); + assertThat(ImmutableMap.of(26, 0)).isEmpty(); + assertThat(ImmutableMap.of(27, 0)).isEmpty(); + assertThat(ImmutableMap.of(28, 0)).isEmpty(); + assertThat(ImmutableMap.of(29, 0)).isEmpty(); + } + + void testAssertThatMapIsEmpty() { + assertThat(ImmutableMap.of(1, 0)).isEmpty(); + assertThat(ImmutableMap.of(2, 0)).isEmpty(); + assertThat(ImmutableMap.of(3, 0)).isEmpty(); + assertThat(ImmutableMap.of(4, 0)).isEmpty(); + assertThat(ImmutableMap.of(5, 0)).isEmpty(); + assertThat(ImmutableMap.of(6, 0)).isEmpty(); + assertThat(ImmutableMap.of(7, 0)).isEmpty(); + } + + ImmutableSet> testAbstractMapAssertIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableMap.of(1, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(2, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(3, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(4, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(5, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(6, 0)).isNotEmpty()); + } + + ImmutableSet> testAssertThatMapIsNotEmpty() { + return ImmutableSet.of( + assertThat(ImmutableMap.of(1, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(2, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(3, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(4, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(5, 0)).isNotEmpty(), + assertThat(ImmutableMap.of(6, 0)).isNotEmpty()); + } + + MapAssert testAbstractMapAssertContainsExactlyInAnyOrderEntriesOf() { return assertThat(ImmutableMap.of(1, 2, 3, 4)) .containsExactlyInAnyOrderEntriesOf(ImmutableMap.of(1, 2, 3, 4)); } - AbstractMapAssert testAbstractMapAssertContainsExactlyEntriesOf() { + MapAssert testAbstractMapAssertContainsExactlyEntriesOf() { return assertThat(ImmutableMap.of(1, 2)).containsExactlyEntriesOf(ImmutableMap.of(1, 2)); } + + ImmutableSet> testAssertThatMapHasSize() { + return ImmutableSet.of( + assertThat(ImmutableMap.of(1, 2)).hasSize(1), + assertThat(ImmutableMap.of(3, 4)).hasSize(1), + assertThat(ImmutableMap.of(5, 6)).hasSize(1), + assertThat(ImmutableMap.of(7, 8)).hasSize(1)); + } + + MapAssert testAbstractMapAssertHasSameSizeAs() { + return assertThat(ImmutableMap.of(1, 2)).hasSameSizeAs(ImmutableMap.of(3, 4)); + } + + AbstractAssert testAssertThatMapContainsKey() { + return assertThat(ImmutableMap.of(1, 2)).containsKey(3); + } + + AbstractAssert testAssertThatMapDoesNotContainKey() { + return assertThat(ImmutableMap.of(1, 2)).doesNotContainKey(3); + } + + AbstractAssert testAssertThatMapContainsValue() { + return assertThat(ImmutableMap.of(1, 2)).containsValue(3); + } + + AbstractAssert testAssertThatMapDoesNotContainValue() { + return assertThat(ImmutableMap.of(1, 2)).doesNotContainValue(3); + } + + AbstractObjectAssert testAssertThatMapContainsEntry() { + return assertThat(ImmutableMap.of(1, 2).get(1)).isEqualTo(2); + } } 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 6c9e16358d..7aa5b7690f 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 @@ -6,16 +6,12 @@ import com.google.common.base.Splitter; import com.google.common.collect.BoundType; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.common.collect.Streams; -import java.math.RoundingMode; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -23,7 +19,6 @@ final class AssortedRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { return ImmutableSet.of( - HashMap.class, HashSet.class, Iterables.class, Preconditions.class, @@ -43,14 +38,6 @@ void testCheckIndexConditional() { } } - Map testCreateEnumMap() { - return new HashMap<>(); - } - - String testMapGetOrNull() { - return ImmutableMap.of(1, "foo").getOrDefault("bar", null); - } - ImmutableSet testStreamToImmutableEnumSet() { return Stream.of(BoundType.OPEN).collect(toImmutableSet()); } @@ -95,14 +82,6 @@ boolean testIterableIsEmpty() { return !ImmutableList.of().iterator().hasNext(); } - Stream testMapKeyStream() { - return ImmutableMap.of("foo", 1).entrySet().stream().map(Map.Entry::getKey); - } - - Stream testMapValueStream() { - return ImmutableMap.of("foo", 1).entrySet().stream().map(Map.Entry::getValue); - } - ImmutableSet> testSplitToStream() { return ImmutableSet.of( Streams.stream(Splitter.on(':').split("foo")), 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 117cf1e6a0..6a76afe1bf 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 @@ -8,18 +8,13 @@ import com.google.common.base.Splitter; import com.google.common.collect.BoundType; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; import com.google.common.collect.Sets; import com.google.common.collect.Streams; -import java.math.RoundingMode; import java.util.Collections; -import java.util.EnumMap; -import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.stream.Stream; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -27,7 +22,6 @@ final class AssortedRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { return ImmutableSet.of( - HashMap.class, HashSet.class, Iterables.class, Preconditions.class, @@ -45,14 +39,6 @@ void testCheckIndexConditional() { checkIndex(1, 2); } - Map testCreateEnumMap() { - return new EnumMap<>(RoundingMode.class); - } - - String testMapGetOrNull() { - return ImmutableMap.of(1, "foo").get("bar"); - } - ImmutableSet testStreamToImmutableEnumSet() { return Stream.of(BoundType.OPEN).collect(toImmutableEnumSet()); } @@ -95,14 +81,6 @@ boolean testIterableIsEmpty() { return Iterables.isEmpty(ImmutableList.of()); } - Stream testMapKeyStream() { - return ImmutableMap.of("foo", 1).keySet().stream(); - } - - Stream testMapValueStream() { - return ImmutableMap.of("foo", 1).values().stream(); - } - ImmutableSet> testSplitToStream() { return ImmutableSet.of( Splitter.on(':').splitToStream("foo"), 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 new file mode 100644 index 0000000000..1f303a4f84 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestInput.java @@ -0,0 +1,47 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import java.math.RoundingMode; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class MapRulesTest implements RefasterRuleCollectionTestCase { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(HashMap.class); + } + + Map testCreateEnumMap() { + return new HashMap<>(); + } + + String testMapGetOrNull() { + return ImmutableMap.of(1, "foo").getOrDefault("bar", null); + } + + ImmutableSet testMapSize() { + return ImmutableSet.of( + ImmutableMap.of("foo", 1).keySet().size(), + ImmutableMap.of("bar", 2).values().size(), + ImmutableMap.of("baz", 3).entrySet().size()); + } + + boolean testMapContainsKey() { + return ImmutableMap.of("foo", 1).keySet().contains("bar"); + } + + boolean testMapContainsValue() { + return ImmutableMap.of("foo", 1).values().contains(2); + } + + Stream testMapKeyStream() { + return ImmutableMap.of("foo", 1).entrySet().stream().map(Map.Entry::getKey); + } + + Stream testMapValueStream() { + return ImmutableMap.of("foo", 1).entrySet().stream().map(Map.Entry::getValue); + } +} 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 new file mode 100644 index 0000000000..35f183ee18 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/MapRulesTestOutput.java @@ -0,0 +1,48 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import java.math.RoundingMode; +import java.util.EnumMap; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class MapRulesTest implements RefasterRuleCollectionTestCase { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(HashMap.class); + } + + Map testCreateEnumMap() { + return new EnumMap<>(RoundingMode.class); + } + + String testMapGetOrNull() { + return ImmutableMap.of(1, "foo").get("bar"); + } + + ImmutableSet testMapSize() { + return ImmutableSet.of( + ImmutableMap.of("foo", 1).size(), + ImmutableMap.of("bar", 2).size(), + ImmutableMap.of("baz", 3).size()); + } + + boolean testMapContainsKey() { + return ImmutableMap.of("foo", 1).containsKey("bar"); + } + + boolean testMapContainsValue() { + return ImmutableMap.of("foo", 1).containsValue(2); + } + + Stream testMapKeyStream() { + return ImmutableMap.of("foo", 1).keySet().stream(); + } + + Stream testMapValueStream() { + return ImmutableMap.of("foo", 1).values().stream(); + } +}