From 62d144550d075656c26d2ef6b8232646d2900804 Mon Sep 17 00:00:00 2001
From: Stephan Schroevers <stephan.schroevers@teampicnic.com>
Date: Sat, 17 Jul 2021 17:21:29 +0200
Subject: [PATCH] WIP Try `baseline-error-prone`

---
 .../CollectionTemplates.java                  |  2 ++
 .../refastertemplates/OptionalTemplates.java  |  2 ++
 .../refastertemplates/StreamTemplates.java    | 13 -------
 .../bugpatterns/StreamTemplatesTestInput.java |  4 ---
 .../StreamTemplatesTestOutput.java            |  4 ---
 pom.xml                                       | 35 ++++++++++++++++++-
 6 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java
index aea3761b..1d326a32 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/CollectionTemplates.java
@@ -158,8 +158,10 @@ final class CollectionTemplates {
   }
 
   /** Prefer {@link ArrayList#ArrayList(Collection)} over the Guava alternative. */
+  // XXX: Drop this template if we adopt `baseline-error-prone`.
   static final class NewArrayListFromCollection<T> {
     @BeforeTemplate
+    @SuppressWarnings("PreferCollectionConstructors")
     ArrayList<T> before(Collection<T> collection) {
       return Lists.newArrayList(collection);
     }
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/OptionalTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/OptionalTemplates.java
index ac55ec15..153cea60 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/OptionalTemplates.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/OptionalTemplates.java
@@ -167,6 +167,7 @@ final class OptionalTemplates {
    */
   abstract static class MapOptionalToBoolean<T> {
     @BeforeTemplate
+    @SuppressWarnings("OptionalOrElseMethodInvocation")
     boolean before(Optional<T> optional, Function<? super T, Boolean> predicate) {
       return optional.map(predicate).orElse(Refaster.anyOf(false, Boolean.FALSE));
     }
@@ -317,6 +318,7 @@ final class OptionalTemplates {
   /** Prefer {@link Optional#or(Supplier)} over more verbose alternatives. */
   abstract static class OptionalOrOtherOptional<T> {
     @BeforeTemplate
+    @SuppressWarnings("OptionalOrElseGetValue")
     Optional<T> before(Optional<T> optional1, Optional<T> optional2) {
       // XXX: Note that rewriting the first and third variant will change the code's behavior if
       // `optional2` has side-effects.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/StreamTemplates.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/StreamTemplates.java
index 7471ce3f..95bede9a 100644
--- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/StreamTemplates.java
+++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/StreamTemplates.java
@@ -24,19 +24,6 @@ import java.util.stream.Stream;
 final class StreamTemplates {
   private StreamTemplates() {}
 
-  /** Prefer {@link Stream#empty()} over less clear alternatives. */
-  static final class EmptyStream<T> {
-    @BeforeTemplate
-    Stream<T> before() {
-      return Stream.of();
-    }
-
-    @AfterTemplate
-    Stream<T> after() {
-      return Stream.empty();
-    }
-  }
-
   /** Prefer {@link Stream#ofNullable(Object)} over more contrived alternatives. */
   static final class StreamOfNullable<T> {
     @BeforeTemplate
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestInput.java
index 46175bbf..89802340 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestInput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestInput.java
@@ -17,10 +17,6 @@ final class StreamTemplatesTest implements RefasterTemplateTestCase {
     return ImmutableSet.of(Objects.class, Streams.class, not(null), reverseOrder());
   }
 
-  Stream<String> testEmptyStream() {
-    return Stream.of();
-  }
-
   ImmutableSet<Stream<String>> testStreamOfNullable() {
     return ImmutableSet.of(
         Stream.of("a").filter(Objects::nonNull), Optional.ofNullable("b").stream());
diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestOutput.java
index 2a224d4a..e677f4ce 100644
--- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestOutput.java
+++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/bugpatterns/StreamTemplatesTestOutput.java
@@ -19,10 +19,6 @@ final class StreamTemplatesTest implements RefasterTemplateTestCase {
     return ImmutableSet.of(Objects.class, Streams.class, not(null), reverseOrder());
   }
 
-  Stream<String> testEmptyStream() {
-    return Stream.empty();
-  }
-
   ImmutableSet<Stream<String>> testStreamOfNullable() {
     return ImmutableSet.of(Stream.ofNullable("a"), Stream.ofNullable("b"));
   }
diff --git a/pom.xml b/pom.xml
index 66b72dc1..6417735a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1392,12 +1392,12 @@
                                     <artifactId>assertj-error-prone</artifactId>
                                     <version>${version.palantir-assertj-automation}</version>
                                 </path>
+                                -->
                                 <path>
                                     <groupId>com.palantir.baseline</groupId>
                                     <artifactId>baseline-error-prone</artifactId>
                                     <version>${version.palantir-baseline}</version>
                                 </path>
-                                -->
                                 <path>
                                     <groupId>com.uber.nullaway</groupId>
                                     <artifactId>nullaway</artifactId>
@@ -1437,6 +1437,39 @@
                                     -Xep:Java7ApiChecker:OFF
                                     <!-- We don't target Android. -->
                                     -Xep:StaticOrDefaultInterfaceMethod:OFF
+                                    <!--
+                                      Flags for `baseline-error-prone`.
+                                    -->
+                                    <!-- We prefer to name loggers name `LOG`,
+                                    rather than `log`. -->
+                                    -Xep:ConsistentLoggerName:OFF
+                                    <!-- XXX: This check discourages use of
+                                    parallel streams, which is good, but also
+                                    suggests a Palantir alternative. Consider
+                                    introducing a similar but less opinionated
+                                    check. -->
+                                    -Xep:DangerousParallelStreamUsage:OFF
+                                    <!-- Nice idea, but too expensive due to
+                                    (recursive) invocation of
+                                    `SuggestedFixes.compilesWithFix`. -->
+                                    -Xep:LambdaMethodReference:OFF
+                                    <!-- We don't use Palantir's `safe-logging`
+                                    library. -->
+                                    -Xep:PreferSafeLoggingPreconditions:OFF
+                                    <!-- We don't use Palantir's `safe-logging`
+                                    library. -->
+                                    -Xep:Slf4jLogsafeArgs:OFF
+                                    <!-- XXX: This check suggests prefixing
+                                    unused parameters with an underscore.
+                                    Discuss this idea within the team. -->
+                                    -Xep:StrictUnusedVariable:OFF
+                                    <!-- Ideally `@VisibleForTesting` members
+                                    are package-private, but there are
+                                    reasonable exceptions. -->
+                                    -Xep:VisibleForTestingPackagePrivate:OFF
+                                    <!--
+                                      Flags for `nullaway`.
+                                    -->
                                     -XepOpt:NullAway:AnnotatedPackages=tech.picnic
                                     -XepOpt:NullAway:AssertsEnabled=true
                                     -XepOpt:NullAway:CheckOptionalEmptiness=true