Generalize AssertThatThrownBy Refaster rule (#1477)

By replacing it with the `AssertThatThrownByAsInstanceOfThrowable` and `AssertThatThrownByIsInstanceOf` rules that are slightly more type-safe.
PicnicSupermarket · Dec 23, 2024 · f124749 · f124749
1 parent 2a3d9c8
commit f124749
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 17 deletions.
diff --git a/...trib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java b/...trib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java
@@ -7,7 +7,10 @@
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatNullPointerException;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.InstanceOfAssertFactories.throwable;
+import static org.assertj.core.api.InstanceOfAssertFactories.type;
 
+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.Repeated;
@@ -16,6 +19,7 @@
 import org.assertj.core.api.AbstractObjectAssert;
 import org.assertj.core.api.AbstractThrowableAssert;
 import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
+import org.assertj.core.api.ThrowableAssertAlternative;
 import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
 /**
@@ -31,6 +35,21 @@
 final class AssertJThrowingCallableRules {
   private AssertJThrowingCallableRules() {}
 
+  static final class AssertThatThrownByIsInstanceOf<T extends Throwable> {
+    @BeforeTemplate
+    void before(ThrowingCallable throwingCallable, Class<T> exceptionType) {
+      Refaster.anyOf(
+          assertThatThrownBy(throwingCallable).asInstanceOf(throwable(exceptionType)),
+          assertThatThrownBy(throwingCallable).asInstanceOf(type(exceptionType)));
+    }
+
+    @AfterTemplate
+    @UseImportPolicy(STATIC_IMPORT_ALWAYS)
+    void after(ThrowingCallable throwingCallable, Class<T> exceptionType) {
+      assertThatThrownBy(throwingCallable).isInstanceOf(exceptionType);
+    }
+  }
+
   static final class AssertThatThrownByIllegalArgumentException {
     @BeforeTemplate
     AbstractObjectAssert<?, ?> before(ThrowingCallable throwingCallable) {
@@ -535,24 +554,24 @@ static final class AssertThatThrownByIOExceptionHasMessageNotContaining {
     }
   }
 
-  static final class AssertThatThrownBy {
+  static final class AssertThatThrownByAsInstanceOfThrowable<T extends Throwable> {
     @BeforeTemplate
-    AbstractObjectAssert<?, ?> before(
-        ThrowingCallable throwingCallable, Class<? extends Throwable> exceptionType) {
+    ThrowableAssertAlternative<T> before(
+        ThrowingCallable throwingCallable, Class<T> exceptionType) {
       return assertThatExceptionOfType(exceptionType).isThrownBy(throwingCallable);
     }
 
     @AfterTemplate
     @UseImportPolicy(STATIC_IMPORT_ALWAYS)
-    AbstractObjectAssert<?, ?> after(
-        ThrowingCallable throwingCallable, Class<? extends Throwable> exceptionType) {
-      return assertThatThrownBy(throwingCallable).isInstanceOf(exceptionType);
+    AbstractThrowableAssert<?, T> after(ThrowingCallable throwingCallable, Class<T> exceptionType) {
+      return assertThatThrownBy(throwingCallable).asInstanceOf(throwable(exceptionType));
     }
   }
 
   static final class AssertThatThrownByHasMessage {
     @BeforeTemplate
-    @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
+    @SuppressWarnings(
+        "AssertThatThrownByAsInstanceOfThrowable" /* This is a more specific template. */)
     AbstractObjectAssert<?, ?> before(
         ThrowingCallable throwingCallable,
         Class<? extends Throwable> exceptionType,
@@ -574,7 +593,8 @@ static final class AssertThatThrownByHasMessage {
 
   static final class AssertThatThrownByRootCauseHasMessage {
     @BeforeTemplate
-    @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
+    @SuppressWarnings(
+        "AssertThatThrownByAsInstanceOfThrowable" /* This is a more specific template. */)
     AbstractObjectAssert<?, ?> before(
         ThrowingCallable throwingCallable,
         Class<? extends Throwable> exceptionType,
@@ -600,7 +620,8 @@ static final class AssertThatThrownByRootCauseHasMessage {
 
   static final class AssertThatThrownByHasMessageParameters {
     @BeforeTemplate
-    @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
+    @SuppressWarnings(
+        "AssertThatThrownByAsInstanceOfThrowable" /* This is a more specific template. */)
     AbstractObjectAssert<?, ?> before(
         ThrowingCallable throwingCallable,
         Class<? extends Throwable> exceptionType,
@@ -626,7 +647,8 @@ static final class AssertThatThrownByHasMessageParameters {
 
   static final class AssertThatThrownByHasMessageStartingWith {
     @BeforeTemplate
-    @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
+    @SuppressWarnings(
+        "AssertThatThrownByAsInstanceOfThrowable" /* This is a more specific template. */)
     AbstractObjectAssert<?, ?> before(
         ThrowingCallable throwingCallable,
         Class<? extends Throwable> exceptionType,
@@ -650,7 +672,8 @@ static final class AssertThatThrownByHasMessageStartingWith {
 
   static final class AssertThatThrownByHasMessageContaining {
     @BeforeTemplate
-    @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
+    @SuppressWarnings(
+        "AssertThatThrownByAsInstanceOfThrowable" /* This is a more specific template. */)
     AbstractObjectAssert<?, ?> before(
         ThrowingCallable throwingCallable,
         Class<? extends Throwable> exceptionType,
@@ -674,7 +697,8 @@ static final class AssertThatThrownByHasMessageContaining {
 
   static final class AssertThatThrownByHasMessageNotContaining {
     @BeforeTemplate
-    @SuppressWarnings("AssertThatThrownBy" /* This is a more specific template. */)
+    @SuppressWarnings(
+        "AssertThatThrownByAsInstanceOfThrowable" /* This is a more specific template. */)
     AbstractObjectAssert<?, ?> before(
         ThrowingCallable throwingCallable,
         Class<? extends Throwable> exceptionType,

diff --git a/...resources/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRulesTestInput.java b/...resources/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRulesTestInput.java
@@ -6,6 +6,8 @@
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatNullPointerException;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.InstanceOfAssertFactories.throwable;
+import static org.assertj.core.api.InstanceOfAssertFactories.type;
 
 import com.google.common.collect.ImmutableSet;
 import org.assertj.core.api.AbstractObjectAssert;
@@ -20,7 +22,13 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
         assertThatIOException(),
         assertThatIllegalArgumentException(),
         assertThatIllegalStateException(),
-        assertThatNullPointerException());
+        assertThatNullPointerException(),
+        type(Throwable.class));
+  }
+
+  void testAssertThatThrownByIsInstanceOf() {
+    assertThatThrownBy(() -> {}).asInstanceOf(throwable(IllegalArgumentException.class));
+    assertThatThrownBy(() -> {}).asInstanceOf(type(IllegalArgumentException.class));
   }
 
   AbstractObjectAssert<?, ?> testAssertThatThrownByIllegalArgumentException() {
@@ -148,7 +156,7 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
     return assertThatIOException().isThrownBy(() -> {}).withMessageNotContaining("foo");
   }
 
-  AbstractObjectAssert<?, ?> testAssertThatThrownBy() {
+  AbstractObjectAssert<?, ?> testAssertThatThrownByAsInstanceOfThrowable() {
     return assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> {});
   }
 

diff --git a/...esources/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRulesTestOutput.java b/...esources/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRulesTestOutput.java
@@ -6,6 +6,8 @@
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatNullPointerException;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.InstanceOfAssertFactories.throwable;
+import static org.assertj.core.api.InstanceOfAssertFactories.type;
 
 import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
@@ -21,7 +23,13 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
         assertThatIOException(),
         assertThatIllegalArgumentException(),
         assertThatIllegalStateException(),
-        assertThatNullPointerException());
+        assertThatNullPointerException(),
+        type(Throwable.class));
+  }
+
+  void testAssertThatThrownByIsInstanceOf() {
+    assertThatThrownBy(() -> {}).isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> {}).isInstanceOf(IllegalArgumentException.class);
   }
 
   AbstractObjectAssert<?, ?> testAssertThatThrownByIllegalArgumentException() {
@@ -180,8 +188,8 @@ public ImmutableSet<Object> elidedTypesAndStaticImports() {
         .hasMessageNotContaining("foo");
   }
 
-  AbstractObjectAssert<?, ?> testAssertThatThrownBy() {
-    return assertThatThrownBy(() -> {}).isInstanceOf(IllegalArgumentException.class);
+  AbstractObjectAssert<?, ?> testAssertThatThrownByAsInstanceOfThrowable() {
+    return assertThatThrownBy(() -> {}).asInstanceOf(throwable(IllegalArgumentException.class));
   }
 
   AbstractObjectAssert<?, ?> testAssertThatThrownByHasMessage() {