Introduce assorted Reactor StepVerifier Refaster rules (#1132)

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

@@ -1768,6 +1768,60 @@ final class ReactorRules {
     }
   }
 
+  /**
+   * Prefer {@link StepVerifier#verify()} over a dangling {@link
+   * StepVerifier#verifyThenAssertThat()}.
+   */
+  // XXX: Application of this rule (and several others in this class) will cause invalid code if the
+  // result of the rewritten expression is dereferenced. Consider introducing a bug checker that
+  // identifies rules that change the return type of an expression and annotates them accordingly.
+  // The associated annotation can then be used to instruct an annotation processor to generate
+  // corresponding `void` rules that match only statements. This would allow the `Refaster` check to
+  // conditionally skip "not fully safe" rules. This allows conditionally flagging more dubious
+  // code, at the risk of compilation failures. With this rule, for example, we want to explicitly
+  // nudge users towards `StepVerifier.Step#assertNext(Consumer)` or
+  // `StepVerifier.Step#expectNext(Object)`, together with `Step#verifyComplete()`.
+  static final class StepVerifierVerify {
+    @BeforeTemplate
+    StepVerifier.Assertions before(StepVerifier stepVerifier) {
+      return stepVerifier.verifyThenAssertThat();
+    }
+
+    @AfterTemplate
+    Duration after(StepVerifier stepVerifier) {
+      return stepVerifier.verify();
+    }
+  }
+
+  /**
+   * Prefer {@link StepVerifier#verify(Duration)} over a dangling {@link
+   * StepVerifier#verifyThenAssertThat(Duration)}.
+   */
+  static final class StepVerifierVerifyDuration {
+    @BeforeTemplate
+    StepVerifier.Assertions before(StepVerifier stepVerifier, Duration duration) {
+      return stepVerifier.verifyThenAssertThat(duration);
+    }
+
+    @AfterTemplate
+    Duration after(StepVerifier stepVerifier, Duration duration) {
+      return stepVerifier.verify(duration);
+    }
+  }
+
+  /** Don't unnecessarily invoke {@link StepVerifier#verifyLater()} multiple times. */
+  static final class StepVerifierVerifyLater {
+    @BeforeTemplate
+    StepVerifier before(StepVerifier stepVerifier) {
+      return stepVerifier.verifyLater().verifyLater();
+    }
+
+    @AfterTemplate
+    StepVerifier after(StepVerifier stepVerifier) {
+      return stepVerifier.verifyLater();
+    }
+  }
+
   /** Don't unnecessarily have {@link StepVerifier.Step} expect no elements. */
   static final class StepVerifierStepIdentity<T> {
     @BeforeTemplate
@@ -1868,6 +1922,12 @@ final class ReactorRules {
       return step.expectErrorMatches(predicate).verify();
     }
 
+    @BeforeTemplate
+    @SuppressWarnings("StepVerifierVerify" /* This is a more specific template. */)
+    StepVerifier.Assertions before2(StepVerifier.LastStep step, Predicate<Throwable> predicate) {
+      return step.expectError().verifyThenAssertThat().hasOperatorErrorMatching(predicate);
+    }
+
     @AfterTemplate
     Duration after(StepVerifier.LastStep step, Predicate<Throwable> predicate) {
       return step.verifyErrorMatches(predicate);
@@ -1890,6 +1950,30 @@ final class ReactorRules {
     }
   }
 
+  /**
+   * Prefer {@link StepVerifier.LastStep#verifyErrorSatisfies(Consumer)} with AssertJ over more
+   * contrived alternatives.
+   */
+  static final class StepVerifierLastStepVerifyErrorSatisfiesAssertJ<T extends Throwable> {
+    @BeforeTemplate
+    @SuppressWarnings("StepVerifierVerify" /* This is a more specific template. */)
+    StepVerifier.Assertions before(StepVerifier.LastStep step, Class<T> clazz, String message) {
+      return Refaster.anyOf(
+          step.expectError()
+              .verifyThenAssertThat()
+              .hasOperatorErrorOfType(clazz)
+              .hasOperatorErrorWithMessage(message),
+          step.expectError(clazz).verifyThenAssertThat().hasOperatorErrorWithMessage(message),
+          step.expectErrorMessage(message).verifyThenAssertThat().hasOperatorErrorOfType(clazz));
+    }
+
+    @AfterTemplate
+    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
+    Duration after(StepVerifier.LastStep step, Class<T> clazz, String message) {
+      return step.verifyErrorSatisfies(t -> assertThat(t).isInstanceOf(clazz).hasMessage(message));
+    }
+  }
+
   /**
    * Prefer {@link StepVerifier.LastStep#verifyErrorMessage(String)} over more verbose alternatives.
    */

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

@@ -598,6 +598,18 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
     return StepVerifier.create(Flux.just(1));
   }
 
+  Object testStepVerifierVerify() {
+    return Mono.empty().as(StepVerifier::create).expectError().verifyThenAssertThat();
+  }
+
+  Object testStepVerifierVerifyDuration() {
+    return Mono.empty().as(StepVerifier::create).expectError().verifyThenAssertThat(Duration.ZERO);
+  }
+
+  StepVerifier testStepVerifierVerifyLater() {
+    return Mono.empty().as(StepVerifier::create).expectError().verifyLater().verifyLater();
+  }
+
   ImmutableSet<StepVerifier.Step<Integer>> testStepVerifierStepIdentity() {
     return ImmutableSet.of(
         Mono.just(1).as(StepVerifier::create).expectNext(),
@@ -638,17 +650,43 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
             .verifyErrorSatisfies(t -> assertThat(t).isInstanceOf(AssertionError.class)));
   }
 
-  Duration testStepVerifierLastStepVerifyErrorMatches() {
-    return Mono.empty()
-        .as(StepVerifier::create)
-        .expectErrorMatches(IllegalArgumentException.class::equals)
-        .verify();
+  ImmutableSet<?> testStepVerifierLastStepVerifyErrorMatches() {
+    return ImmutableSet.of(
+        Mono.empty()
+            .as(StepVerifier::create)
+            .expectErrorMatches(IllegalArgumentException.class::equals)
+            .verify(),
+        Mono.empty()
+            .as(StepVerifier::create)
+            .expectError()
+            .verifyThenAssertThat()
+            .hasOperatorErrorMatching(IllegalStateException.class::equals));
   }
 
   Duration testStepVerifierLastStepVerifyErrorSatisfies() {
     return Mono.empty().as(StepVerifier::create).expectErrorSatisfies(t -> {}).verify();
   }
 
+  ImmutableSet<?> testStepVerifierLastStepVerifyErrorSatisfiesAssertJ() {
+    return ImmutableSet.of(
+        Mono.empty()
+            .as(StepVerifier::create)
+            .expectError()
+            .verifyThenAssertThat()
+            .hasOperatorErrorOfType(IllegalArgumentException.class)
+            .hasOperatorErrorWithMessage("foo"),
+        Mono.empty()
+            .as(StepVerifier::create)
+            .expectError(IllegalStateException.class)
+            .verifyThenAssertThat()
+            .hasOperatorErrorWithMessage("bar"),
+        Mono.empty()
+            .as(StepVerifier::create)
+            .expectErrorMessage("baz")
+            .verifyThenAssertThat()
+            .hasOperatorErrorOfType(AssertionError.class));
+  }
+
   Duration testStepVerifierLastStepVerifyErrorMessage() {
     return Mono.empty().as(StepVerifier::create).expectErrorMessage("foo").verify();
   }

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

@@ -586,6 +586,18 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
     return Flux.just(1).as(StepVerifier::create);
   }
 
+  Object testStepVerifierVerify() {
+    return Mono.empty().as(StepVerifier::create).expectError().verify();
+  }
+
+  Object testStepVerifierVerifyDuration() {
+    return Mono.empty().as(StepVerifier::create).expectError().verify(Duration.ZERO);
+  }
+
+  StepVerifier testStepVerifierVerifyLater() {
+    return Mono.empty().as(StepVerifier::create).expectError().verifyLater();
+  }
+
   ImmutableSet<StepVerifier.Step<Integer>> testStepVerifierStepIdentity() {
     return ImmutableSet.of(
         Mono.just(1).as(StepVerifier::create),
@@ -619,16 +631,36 @@ final class ReactorRulesTest implements RefasterRuleCollectionTestCase {
         Mono.empty().as(StepVerifier::create).verifyError(AssertionError.class));
   }
 
-  Duration testStepVerifierLastStepVerifyErrorMatches() {
-    return Mono.empty()
-        .as(StepVerifier::create)
-        .verifyErrorMatches(IllegalArgumentException.class::equals);
+  ImmutableSet<?> testStepVerifierLastStepVerifyErrorMatches() {
+    return ImmutableSet.of(
+        Mono.empty()
+            .as(StepVerifier::create)
+            .verifyErrorMatches(IllegalArgumentException.class::equals),
+        Mono.empty()
+            .as(StepVerifier::create)
+            .verifyErrorMatches(IllegalStateException.class::equals));
   }
 
   Duration testStepVerifierLastStepVerifyErrorSatisfies() {
     return Mono.empty().as(StepVerifier::create).verifyErrorSatisfies(t -> {});
   }
 
+  ImmutableSet<?> testStepVerifierLastStepVerifyErrorSatisfiesAssertJ() {
+    return ImmutableSet.of(
+        Mono.empty()
+            .as(StepVerifier::create)
+            .verifyErrorSatisfies(
+                t -> assertThat(t).isInstanceOf(IllegalArgumentException.class).hasMessage("foo")),
+        Mono.empty()
+            .as(StepVerifier::create)
+            .verifyErrorSatisfies(
+                t -> assertThat(t).isInstanceOf(IllegalStateException.class).hasMessage("bar")),
+        Mono.empty()
+            .as(StepVerifier::create)
+            .verifyErrorSatisfies(
+                t -> assertThat(t).isInstanceOf(AssertionError.class).hasMessage("baz")));
+  }
+
   Duration testStepVerifierLastStepVerifyErrorMessage() {
     return Mono.empty().as(StepVerifier::create).verifyErrorMessage("foo");
   }