From 35fb0f74f4c589410b8eb1ff60468b4e40240aee Mon Sep 17 00:00:00 2001
From: Stephan Schroevers <stephan.schroevers@teampicnic.com>
Date: Sun, 11 Feb 2024 12:02:30 +0100
Subject: [PATCH] Suggestions

---
 ...LexicographicalAnnotationAttributeListing.java |  3 +++
 .../errorprone/refasterrules/CollectionRules.java | 15 ++++++++++++---
 .../refasterrules/CollectionRulesTestInput.java   |  3 ++-
 .../refasterrules/CollectionRulesTestOutput.java  |  3 ++-
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java
index bd6fe649b25..8083da5d5bd 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java
@@ -52,6 +52,9 @@
  * <p>The idea behind this checker is that maintaining a sorted sequence simplifies conflict
  * resolution, and can even avoid it if two branches add the same entry.
  */
+// XXX: In some places we declare a `@SuppressWarnings` annotation with a final value of
+// `key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict`. That entry must stay
+// last. Consider adding (generic?) support for such cases.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary = "Where possible, sort annotation array attributes lexicographically",
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java
index bdcffa86615..cd341e791de 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/CollectionRules.java
@@ -35,14 +35,21 @@ private CollectionRules() {}
    */
   static final class CollectionIsEmpty<T> {
     @BeforeTemplate
-    @SuppressWarnings("java:S1155" /* This violation will be rewritten. */)
+    @SuppressWarnings({
+      "java:S1155" /* This violation will be rewritten. */,
+      "LexicographicalAnnotationAttributeListing" /* `key-*` entry must remain last. */,
+      "OptionalFirstCollectionElement" /* This is a more specific template. */,
+      "StreamIsEmpty" /* This is a more specific template. */,
+      "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict"
+    })
     boolean before(Collection<T> collection) {
       return Refaster.anyOf(
           collection.size() == 0,
           collection.size() <= 0,
           collection.size() < 1,
           Iterables.isEmpty(collection),
-          collection.stream().findAny().isEmpty());
+          collection.stream().findAny().isEmpty(),
+          collection.stream().findFirst().isEmpty());
     }
 
     @BeforeTemplate
@@ -338,7 +345,9 @@ Iterator<T> after(ImmutableCollection<T> collection) {
 
   /**
    * Don't use the ternary operator to extract the first element of a possibly-empty {@link
-   * Collection} as an {@link Optional}.
+   * Collection} as an {@link Optional}, and (when applicable) prefer {@link Stream#findFirst()}
+   * over {@link Stream#findAny()} to communicate that the collection's first element (if any,
+   * according to iteration order) will be returned.
    */
   static final class OptionalFirstCollectionElement<T> {
     @BeforeTemplate
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java
index 56f194cd4db..186da035b7a 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestInput.java
@@ -30,7 +30,8 @@ ImmutableSet<Boolean> testCollectionIsEmpty() {
         ImmutableSet.of(6).size() >= 1,
         Iterables.isEmpty(ImmutableSet.of(7)),
         ImmutableSet.of(8).stream().findAny().isEmpty(),
-        ImmutableSet.of(9).asList().isEmpty());
+        ImmutableSet.of(9).stream().findFirst().isEmpty(),
+        ImmutableSet.of(10).asList().isEmpty());
   }
 
   ImmutableSet<Integer> testCollectionSize() {
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java
index 96719dcdbf0..ba09e13e63e 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/CollectionRulesTestOutput.java
@@ -30,7 +30,8 @@ ImmutableSet<Boolean> testCollectionIsEmpty() {
         !ImmutableSet.of(6).isEmpty(),
         ImmutableSet.of(7).isEmpty(),
         ImmutableSet.of(8).isEmpty(),
-        ImmutableSet.of(9).isEmpty());
+        ImmutableSet.of(9).isEmpty(),
+        ImmutableSet.of(10).isEmpty());
   }
 
   ImmutableSet<Integer> testCollectionSize() {