Generalize assorted Refaster rules

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJCharSequenceRules.java

@@ -18,6 +18,7 @@ static final class AssertThatCharSequenceIsEmpty {
     @BeforeTemplate
     void before(CharSequence charSequence) {
       Refaster.anyOf(
+          assertThat(charSequence.isEmpty()).isTrue(),
           assertThat(charSequence.length()).isEqualTo(0L),
           assertThat(charSequence.length()).isNotPositive());
     }
@@ -33,6 +34,7 @@ static final class AssertThatCharSequenceIsNotEmpty {
     @BeforeTemplate
     AbstractAssert<?, ?> before(CharSequence charSequence) {
       return Refaster.anyOf(
+          assertThat(charSequence.isEmpty()).isFalse(),
           assertThat(charSequence.length()).isNotEqualTo(0),
           assertThat(charSequence.length()).isPositive());
     }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJMapRules.java

@@ -11,7 +11,6 @@
 import com.google.errorprone.refaster.annotation.UseImportPolicy;
 import java.util.Collection;
 import java.util.Map;
-import java.util.Set;
 import org.assertj.core.api.AbstractAssert;
 import org.assertj.core.api.AbstractBooleanAssert;
 import org.assertj.core.api.AbstractCollectionAssert;
@@ -182,13 +181,13 @@ MapAssert<K, V> after(Map<K, V> map, K key) {
   static final class AssertThatMapContainsOnlyKeys<K, V> {
     @BeforeTemplate
     AbstractCollectionAssert<?, Collection<? extends K>, K, ?> before(
-        Map<K, V> map, Set<? extends K> keys) {
+        Map<K, V> map, Iterable<? extends K> keys) {
       return assertThat(map.keySet()).hasSameElementsAs(keys);
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    MapAssert<K, V> after(Map<K, V> map, Set<? extends K> keys) {
+    MapAssert<K, V> after(Map<K, V> map, Iterable<? extends K> keys) {
       return assertThat(map).containsOnlyKeys(keys);
     }
   }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJRules.java

@@ -362,13 +362,13 @@ IterableAssert<S> after(Iterable<S> iterable, T element) {
 
   static final class AssertThatListsAreEqual<S, T extends S> {
     @BeforeTemplate
-    ListAssert<S> before(List<S> list1, List<T> list2) {
+    ListAssert<S> before(List<S> list1, Iterable<T> list2) {
       return assertThat(list1).isEqualTo(list2);
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    ListAssert<S> after(List<S> list1, List<T> list2) {
+    ListAssert<S> after(List<S> list1, Iterable<T> list2) {
       return assertThat(list1).containsExactlyElementsOf(list2);
     }
   }
@@ -379,15 +379,15 @@ ListAssert<S> after(List<S> list1, List<T> list2) {
 
   static final class AssertThatSetsAreEqual<S, T extends S> {
     @BeforeTemplate
-    AbstractCollectionAssert<?, ?, S, ?> before(Set<S> set1, Set<T> set2) {
+    AbstractCollectionAssert<?, ?, S, ?> before(Set<S> set1, Iterable<T> set2) {
       return Refaster.anyOf(
           assertThat(set1).isEqualTo(set2),
           assertThat(set1).containsExactlyInAnyOrderElementsOf(set2));
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    AbstractCollectionAssert<?, ?, S, ?> after(Set<S> set1, Set<T> set2) {
+    AbstractCollectionAssert<?, ?, S, ?> after(Set<S> set1, Iterable<T> set2) {
       return assertThat(set1).hasSameElementsAs(set2);
     }
   }
@@ -398,13 +398,13 @@ static final class AssertThatSetsAreEqual<S, T extends S> {
 
   static final class AssertThatMultisetsAreEqual<S, T extends S> {
     @BeforeTemplate
-    AbstractCollectionAssert<?, ?, S, ?> before(Multiset<S> multiset1, Multiset<T> multiset2) {
+    AbstractCollectionAssert<?, ?, S, ?> before(Multiset<S> multiset1, Iterable<T> multiset2) {
       return assertThat(multiset1).isEqualTo(multiset2);
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    AbstractCollectionAssert<?, ?, S, ?> after(Multiset<S> multiset1, Multiset<T> multiset2) {
+    AbstractCollectionAssert<?, ?, S, ?> after(Multiset<S> multiset1, Iterable<T> multiset2) {
       return assertThat(multiset1).containsExactlyInAnyOrderElementsOf(multiset2);
     }
   }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java

@@ -32,19 +32,6 @@ void after(AbstractStringAssert<?> stringAssert) {
     }
   }
 
-  static final class AssertThatStringIsEmpty {
-    @BeforeTemplate
-    void before(String string) {
-      assertThat(string.isEmpty()).isTrue();
-    }
-
-    @AfterTemplate
-    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    void after(String string) {
-      assertThat(string).isEmpty();
-    }
-  }
-
   static final class AbstractStringAssertStringIsNotEmpty {
     @BeforeTemplate
     AbstractStringAssert<?> before(AbstractStringAssert<?> stringAssert) {
@@ -57,41 +44,28 @@ AbstractStringAssert<?> after(AbstractStringAssert<?> stringAssert) {
     }
   }
 
-  static final class AssertThatStringIsNotEmpty {
-    @BeforeTemplate
-    AbstractAssert<?, ?> before(String string) {
-      return assertThat(string.isEmpty()).isFalse();
-    }
-
-    @AfterTemplate
-    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    AbstractAssert<?, ?> after(String string) {
-      return assertThat(string).isNotEmpty();
-    }
-  }
-
   static final class AssertThatStringContains {
     @BeforeTemplate
-    AbstractBooleanAssert<?> before(String string, String substring) {
+    AbstractBooleanAssert<?> before(String string, CharSequence substring) {
       return assertThat(string.contains(substring)).isTrue();
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    AbstractStringAssert<?> after(String string, String substring) {
+    AbstractStringAssert<?> after(String string, CharSequence substring) {
       return assertThat(string).contains(substring);
     }
   }
 
   static final class AssertThatStringDoesNotContain {
     @BeforeTemplate
-    AbstractBooleanAssert<?> before(String string, String substring) {
+    AbstractBooleanAssert<?> before(String string, CharSequence substring) {
       return assertThat(string.contains(substring)).isFalse();
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    AbstractStringAssert<?> after(String string, String substring) {
+    AbstractStringAssert<?> after(String string, CharSequence substring) {
       return assertThat(string).doesNotContain(substring);
     }
   }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssortedRules.java

@@ -121,18 +121,18 @@ Stream<T> after(T object) {
    */
   static final class DisjointSets<T> {
     @BeforeTemplate
-    boolean before(Set<T> set1, Set<T> set2) {
-      return Sets.intersection(set1, set2).isEmpty();
+    boolean before(Set<T> collection1, Set<T> collection2) {
+      return Sets.intersection(collection1, collection2).isEmpty();
     }
 
     @BeforeTemplate
-    boolean before2(Set<T> set1, Set<T> set2) {
-      return set1.stream().noneMatch(set2::contains);
+    boolean before2(Collection<T> collection1, Collection<T> collection2) {
+      return collection1.stream().noneMatch(collection2::contains);
     }
 
     @AfterTemplate
-    boolean after(Set<T> set1, Set<T> set2) {
-      return disjoint(set1, set2);
+    boolean after(Collection<T> collection1, Collection<T> collection2) {
+      return disjoint(collection1, collection2);
     }
   }
 

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java

@@ -158,14 +158,14 @@ boolean after(Collection<T> removeTo, Collection<S> elementsToRemove) {
     }
   }
 
-  static final class SetRemoveAllCollection<T, S extends T> {
+  static final class CollectionRemoveAllFromCollectionBlock<T, S extends T> {
     @BeforeTemplate
-    void before(Set<T> removeFrom, Collection<S> elementsToRemove) {
+    void before(Collection<T> removeFrom, Collection<S> elementsToRemove) {
       elementsToRemove.forEach(removeFrom::remove);
     }
 
     @BeforeTemplate
-    void before2(Set<T> removeFrom, Collection<S> elementsToRemove) {
+    void before2(Collection<T> removeFrom, Collection<S> elementsToRemove) {
       for (T element : elementsToRemove) {
         removeFrom.remove(element);
       }
@@ -175,14 +175,14 @@ void before2(Set<T> removeFrom, Collection<S> elementsToRemove) {
     // that this is supported out of the box. After doing so, also drop the `S extends T` type
     // constraint; ideally this check applies to any `S`.
     @BeforeTemplate
-    void before3(Set<T> removeFrom, Collection<S> elementsToRemove) {
+    void before3(Collection<T> removeFrom, Collection<S> elementsToRemove) {
       for (S element : elementsToRemove) {
         removeFrom.remove(element);
       }
     }
 
     @AfterTemplate
-    void after(Set<T> removeFrom, Collection<S> elementsToRemove) {
+    void after(Collection<T> removeFrom, Collection<S> elementsToRemove) {
       removeFrom.removeAll(elementsToRemove);
     }
   }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/ImmutableMapRules.java

@@ -338,14 +338,14 @@ abstract static class ImmutableMapCopyOfMapsFilterKeys<K, V> {
     abstract boolean keyFilter(@MayOptionallyUse K key);
 
     @BeforeTemplate
-    ImmutableMap<K, V> before(ImmutableMap<K, V> map) {
+    ImmutableMap<K, V> before(Map<K, V> map) {
       return map.entrySet().stream()
           .filter(e -> keyFilter(e.getKey()))
           .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
     }
 
     @AfterTemplate
-    ImmutableMap<K, V> after(ImmutableMap<K, V> map) {
+    ImmutableMap<K, V> after(Map<K, V> map) {
       return ImmutableMap.copyOf(Maps.filterKeys(map, k -> keyFilter(k)));
     }
   }
@@ -359,14 +359,14 @@ abstract static class ImmutableMapCopyOfMapsFilterValues<K, V> {
     abstract boolean valueFilter(@MayOptionallyUse V value);
 
     @BeforeTemplate
-    ImmutableMap<K, V> before(ImmutableMap<K, V> map) {
+    ImmutableMap<K, V> before(Map<K, V> map) {
       return map.entrySet().stream()
           .filter(e -> valueFilter(e.getValue()))
           .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
     }
 
     @AfterTemplate
-    ImmutableMap<K, V> after(ImmutableMap<K, V> map) {
+    ImmutableMap<K, V> after(Map<K, V> map) {
       return ImmutableMap.copyOf(Maps.filterValues(map, v -> valueFilter(v)));
     }
   }

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJCharSequenceRulesTestInput.java

@@ -8,13 +8,16 @@
 
 final class AssertJCharSequenceRulesTest implements RefasterRuleCollectionTestCase {
   void testAssertThatCharSequenceIsEmpty() {
-    assertThat("foo".length()).isEqualTo(0L);
-    assertThat("foo".length()).isNotPositive();
+    assertThat("foo".isEmpty()).isTrue();
+    assertThat("bar".length()).isEqualTo(0L);
+    assertThat("baz".length()).isNotPositive();
   }
 
   ImmutableSet<AbstractAssert<?, ?>> testAssertThatCharSequenceIsNotEmpty() {
     return ImmutableSet.of(
-        assertThat("foo".length()).isNotEqualTo(0), assertThat("bar".length()).isPositive());
+        assertThat("foo".isEmpty()).isFalse(),
+        assertThat("bar".length()).isNotEqualTo(0),
+        assertThat("baz".length()).isPositive());
   }
 
   AbstractAssert<?, ?> testAssertThatCharSequenceHasSize() {

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJCharSequenceRulesTestOutput.java

@@ -9,11 +9,15 @@
 final class AssertJCharSequenceRulesTest implements RefasterRuleCollectionTestCase {
   void testAssertThatCharSequenceIsEmpty() {
     assertThat("foo").isEmpty();
-    assertThat("foo").isEmpty();
+    assertThat("bar").isEmpty();
+    assertThat("baz").isEmpty();
   }
 
   ImmutableSet<AbstractAssert<?, ?>> testAssertThatCharSequenceIsNotEmpty() {
-    return ImmutableSet.of(assertThat("foo").isNotEmpty(), assertThat("bar").isNotEmpty());
+    return ImmutableSet.of(
+        assertThat("foo").isNotEmpty(),
+        assertThat("bar").isNotEmpty(),
+        assertThat("baz").isNotEmpty());
   }
 
   AbstractAssert<?, ?> testAssertThatCharSequenceHasSize() {

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestInput.java

@@ -21,18 +21,10 @@ void testAbstractStringAssertStringIsEmpty() {
     assertThat("foo").isEqualTo("");
   }
 
-  void testAssertThatStringIsEmpty() {
-    assertThat("foo".isEmpty()).isTrue();
-  }
-
   AbstractStringAssert<?> testAbstractStringAssertStringIsNotEmpty() {
     return assertThat("foo").isNotEqualTo("");
   }
 
-  AbstractAssert<?, ?> testAssertThatStringIsNotEmpty() {
-    return assertThat("foo".isEmpty()).isFalse();
-  }
-
   AbstractAssert<?, ?> testAssertThatStringContains() {
     return assertThat("foo".contains("bar")).isTrue();
   }

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/AssertJStringRulesTestOutput.java

@@ -22,18 +22,10 @@ void testAbstractStringAssertStringIsEmpty() {
     assertThat("foo").isEmpty();
   }
 
-  void testAssertThatStringIsEmpty() {
-    assertThat("foo").isEmpty();
-  }
-
   AbstractStringAssert<?> testAbstractStringAssertStringIsNotEmpty() {
     return assertThat("foo").isNotEmpty();
   }
 
-  AbstractAssert<?, ?> testAssertThatStringIsNotEmpty() {
-    return assertThat("foo").isNotEmpty();
-  }
-
   AbstractAssert<?, ?> testAssertThatStringContains() {
     return assertThat("foo").contains("bar");
   }

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java

@@ -62,7 +62,7 @@ boolean testCollectionRemoveAllFromCollectionExpression() {
     return Iterables.removeAll(new ArrayList<>(), ImmutableSet.of("foo"));
   }
 
-  void testSetRemoveAllCollection() {
+  void testCollectionRemoveAllFromCollectionBlock() {
     ImmutableSet.of("foo").forEach(new HashSet<>()::remove);
     for (Number element : ImmutableList.of(1)) {
       new HashSet<Number>().remove(element);

error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java

@@ -58,7 +58,7 @@ boolean testCollectionRemoveAllFromCollectionExpression() {
     return new ArrayList<>().removeAll(ImmutableSet.of("foo"));
   }
 
-  void testSetRemoveAllCollection() {
+  void testCollectionRemoveAllFromCollectionBlock() {
     new HashSet<>().removeAll(ImmutableSet.of("foo"));
     new HashSet<Number>().removeAll(ImmutableList.of(1));
     new HashSet<Number>().removeAll(ImmutableSet.of(2));