Remove variants of assertThrows accepting ThrowingSupplier

JUnit Jupiter 5.3.0 introduced new variants of `assertThrows()` that
accept `ThrowingSupplier` arguments instead of `Executable` (see #1394).
However, this change prevents existing code from compiling against
Jupiter 5.3.0 if the code in question used a method reference for an
overloaded method with a `void` return type. Consequently, this was a
breaking change.

Note that overloaded methods with non-void return types are not
affected. For example, even though `java.util.concurrent.Future` has
two `get(...)` methods, it still can be used as a method reference as
`future::get` without suffering from issues with type inference since
such a method can always properly be inferred to be a
`ThrowingSupplier`.

This commit fixes the breaking change by removing all variants of
`assertThrows` that accept a `ThrowingSupplier`. In addition, this
commit reverts the related changes to `ThrowingSupplier` (i.e., it no
longer implements `Executable` via a `default` interface method).

Issue: #1576

documentation/src/docs/asciidoc/release-notes/release-notes-5.3.1.adoc

@@ -1,7 +1,7 @@
 [[release-notes-5.3.1]]
 == 5.3.1
 
-*Date of Release:* ❓
+*Date of Release:* September ❓, 2018
 
 *Scope:* Bug fixes since 5.3.0
 
@@ -30,7 +30,16 @@ repository on GitHub.
 
 ==== Bug Fixes
 
-* ❓
+* Invocations of `assertThrows()` that are passed a method reference for an overloaded
+  method with a `void` return type once again compile.
+  - For example, given an instance of `java.lang.Object` stored in a variable named
+    `object`, `assertThrows(Exception.class, object::wait)` compiled against JUnit 5.2.0,
+    failed to compile against JUnit 5.3.0, but now compiles against JUnit 5.3.1.
+
+==== Breaking Changes
+
+* In order to revert the aforementioned breaking change, variants of `assertThrows()`
+  introduced in JUnit 5.3.0 that accept `ThrowingSupplier` arguments have been removed.
 
 
 [[release-notes-5.3.1-junit-vintage]]

junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertThrows.java

@@ -18,8 +18,6 @@
 import java.util.function.Supplier;
 
 import org.junit.jupiter.api.function.Executable;
-import org.junit.jupiter.api.function.ThrowingSupplier;
-import org.junit.platform.commons.util.StringUtils;
 import org.opentest4j.AssertionFailedError;
 
 /**
@@ -35,49 +33,25 @@ private AssertThrows() {
 	}
 
 	static <T extends Throwable> T assertThrows(Class<T> expectedType, Executable executable) {
-		return assertThrows(expectedType, asSupplier(executable), (Object) null);
+		return assertThrows(expectedType, executable, (Object) null);
 	}
 
 	static <T extends Throwable> T assertThrows(Class<T> expectedType, Executable executable, String message) {
-		return assertThrows(expectedType, asSupplier(executable), (Object) message);
+		return assertThrows(expectedType, executable, (Object) message);
 	}
 
 	static <T extends Throwable> T assertThrows(Class<T> expectedType, Executable executable,
 			Supplier<String> messageSupplier) {
 
-		return assertThrows(expectedType, asSupplier(executable), (Object) messageSupplier);
-	}
-
-	/**
-	 * @since 5.3
-	 */
-	static <T extends Throwable> T assertThrows(Class<T> expectedType, ThrowingSupplier<?> supplier) {
-		return assertThrows(expectedType, supplier::get, (Object) null);
-	}
-
-	/**
-	 * @since 5.3
-	 */
-	static <T extends Throwable> T assertThrows(Class<T> expectedType, ThrowingSupplier<?> supplier, String message) {
-		return assertThrows(expectedType, supplier::get, (Object) message);
-	}
-
-	/**
-	 * @since 5.3
-	 */
-	static <T extends Throwable> T assertThrows(Class<T> expectedType, ThrowingSupplier<?> supplier,
-			Supplier<String> messageSupplier) {
-
-		return assertThrows(expectedType, supplier::get, (Object) messageSupplier);
+		return assertThrows(expectedType, executable, (Object) messageSupplier);
 	}
 
 	@SuppressWarnings("unchecked")
-	private static <T extends Throwable> T assertThrows(Class<T> expectedType, ResultAwareThrowingSupplier<?> supplier,
+	private static <T extends Throwable> T assertThrows(Class<T> expectedType, Executable executable,
 			Object messageOrSupplier) {
 
-		Object result;
 		try {
-			result = supplier.get();
+			executable.execute();
 		}
 		catch (Throwable actualException) {
 			if (expectedType.isInstance(actualException)) {
@@ -90,54 +64,9 @@ private static <T extends Throwable> T assertThrows(Class<T> expectedType, Resul
 			}
 		}
 
-		String includedResult = supplier.includeResult()
-				? String.format(" (returned %s).", StringUtils.nullSafeToString(result))
-				: ".";
 		String message = buildPrefix(nullSafeGet(messageOrSupplier))
-				+ String.format("Expected %s to be thrown, but nothing was thrown", getCanonicalName(expectedType))
-				+ includedResult;
+				+ String.format("Expected %s to be thrown, but nothing was thrown.", getCanonicalName(expectedType));
 		throw new AssertionFailedError(message);
 	}
 
-	private interface ResultAwareThrowingSupplier<T> extends ThrowingSupplier<T> {
-
-		/**
-		 * Determine if the result of invoking {@link #get()} should be included
-		 * in the assertion failure message if this supplier returns an actual
-		 * result instead of throwing an exception.
-		 *
-		 * @return {@code true} by default; can be overridden in concrete implementations
-		 */
-		default boolean includeResult() {
-			return true;
-		}
-	}
-
-	private static ResultAwareThrowingSupplier<Void> asSupplier(Executable executable) {
-		return new ResultAwareThrowingSupplierAdapter(executable);
-	}
-
-	/**
-	 * Adapts an {@link Executable} to the {@link ResultAwareThrowingSupplier} API.
-	 */
-	private static class ResultAwareThrowingSupplierAdapter implements ResultAwareThrowingSupplier<Void> {
-
-		private final Executable executable;
-
-		ResultAwareThrowingSupplierAdapter(Executable executable) {
-			this.executable = executable;
-		}
-
-		@Override
-		public Void get() throws Throwable {
-			executable.execute();
-			return null;
-		}
-
-		@Override
-		public boolean includeResult() {
-			return false;
-		}
-	}
-
 }

junit-jupiter-api/src/main/java/org/junit/jupiter/api/Assertions.java

@@ -1211,75 +1211,6 @@ public static <T extends Throwable> T assertThrows(Class<T> expectedType, Execut
 		return AssertThrows.assertThrows(expectedType, executable, messageSupplier);
 	}
 
-	// --- supplier ---
-
-	/**
-	 * <em>Asserts</em> that execution of the given {@code supplier} throws
-	 * an exception of the {@code expectedType} and returns the exception.
-	 *
-	 * <p>If no exception is thrown, or if an exception of a different type is
-	 * thrown, this method will fail.
-	 *
-	 * <p>If the given {@link ThrowingSupplier} returns a result instead of
-	 * throwing an exception, the result will be included in the failure message.
-	 *
-	 * <p>If you do not want to perform additional checks on the exception instance,
-	 * simply ignore the return value.
-	 *
-	 * @since 5.3
-	 */
-	@API(status = STABLE, since = "5.3")
-	public static <T extends Throwable> T assertThrows(Class<T> expectedType, ThrowingSupplier<?> supplier) {
-		return AssertThrows.assertThrows(expectedType, supplier);
-	}
-
-	/**
-	 * <em>Asserts</em> that execution of the given {@code supplier} throws
-	 * an exception of the {@code expectedType} and returns the exception.
-	 *
-	 * <p>If no exception is thrown, or if an exception of a different type is
-	 * thrown, this method will fail.
-	 *
-	 * <p>If the given {@link ThrowingSupplier} returns a result instead of
-	 * throwing an exception, the result will be included in the failure message.
-	 *
-	 * <p>If you do not want to perform additional checks on the exception instance,
-	 * simply ignore the return value.
-	 *
-	 * @since 5.3
-	 */
-	@API(status = STABLE, since = "5.3")
-	public static <T extends Throwable> T assertThrows(Class<T> expectedType, ThrowingSupplier<?> supplier,
-			String message) {
-
-		return AssertThrows.assertThrows(expectedType, supplier, message);
-	}
-
-	/**
-	 * <em>Asserts</em> that execution of the given {@code supplier} throws
-	 * an exception of the {@code expectedType} and returns the exception.
-	 *
-	 * <p>If no exception is thrown, or if an exception of a different type is
-	 * thrown, this method will fail.
-	 *
-	 * <p>If necessary, the failure message will be retrieved lazily from the
-	 * supplied {@code messageSupplier}.
-	 *
-	 * <p>If the given {@link ThrowingSupplier} returns a result instead of
-	 * throwing an exception, the result will be included in the failure message.
-	 *
-	 * <p>If you do not want to perform additional checks on the exception instance,
-	 * simply ignore the return value.
-	 *
-	 * @since 5.3
-	 */
-	@API(status = STABLE, since = "5.3")
-	public static <T extends Throwable> T assertThrows(Class<T> expectedType, ThrowingSupplier<?> supplier,
-			Supplier<String> messageSupplier) {
-
-		return AssertThrows.assertThrows(expectedType, supplier, messageSupplier);
-	}
-
 	// --- executable ---
 
 	/**

junit-jupiter-api/src/main/java/org/junit/jupiter/api/function/ThrowingSupplier.java

@@ -23,15 +23,6 @@
  * {@link java.util.function.Supplier}, except that a {@code ThrowingSupplier}
  * can throw any kind of exception, including checked exceptions.
  *
- * <p>As of JUnit Jupiter 5.3, {@code ThrowingSupplier} extends
- * {@link Executable}, providing a <em>default</em> implementation of
- * {@link #execute()} that delegates to {@link #get()} and ignores the return
- * value. This allows the Java compiler to disambiguate between
- * {@code ThrowingSupplier} and {@code Executable} when performing type
- * inference for a lambda expression or method reference supplied to
- * an overloaded method that accepts either a {@code ThrowingSupplier} or an
- * {@code Executable}.
- *
  * <h4>Rationale for throwing {@code Throwable} instead of {@code Exception}</h4>
  *
  * <p>Although Java applications typically throw exceptions that are instances
@@ -51,23 +42,7 @@
  */
 @FunctionalInterface
 @API(status = STABLE, since = "5.0")
-public interface ThrowingSupplier<T> extends Executable {
-
-	/**
-	 * Delegates to {@link #get()} and ignores the return value.
-	 *
-	 * <p>This default method is not intended to be overridden. See
-	 * {@linkplain ThrowingSupplier class-level documentation} for further
-	 * details.
-	 *
-	 * @since 5.3
-	 * @see #get()
-	 */
-	@Override
-	@API(status = STABLE, since = "5.3")
-	default void execute() throws Throwable {
-		get();
-	}
+public interface ThrowingSupplier<T> {
 
 	/**
 	 * Get a result, potentially throwing an exception.

junit-jupiter-engine/src/test/java/org/junit/jupiter/api/AssertDoesNotThrowAssertionsTests.java

@@ -35,17 +35,19 @@ class AssertDoesNotThrowAssertionsTests {
 	private static final ThrowingSupplier<String> something = () -> "enigma";
 
 	@Test
-	void assertDoesNotThrowWithFutureMethodReference() {
+	void assertDoesNotThrowWithMethodReferenceForNonVoidReturnType() {
 		FutureTask<String> future = new FutureTask<>(() -> {
 			return "foo";
 		});
 		future.run();
 
 		String result;
 
-		// Current compiler's type inference
-		result = assertDoesNotThrow(future::get);
-		assertEquals("foo", result);
+		// Current compiler's type inference: does NOT compile since the compiler
+		// cannot figure out which overloaded variant of assertDoesNotThrow() to
+		// invoke (i.e., Executable vs. ThrowingSupplier).
+		//
+		// result = assertDoesNotThrow(future::get);
 
 		// Explicitly as an Executable
 		assertDoesNotThrow((Executable) future::get);
@@ -61,7 +63,9 @@ void assertDoesNotThrowWithMethodReferenceForVoidReturnType() {
 
 		// Note: the following does not compile since the compiler cannot properly
 		// perform type inference for a method reference for an overloaded method
-		// that has a void return type such as Foo.overloaded(...)
+		// that has a void return type such as Foo.overloaded(...), IFF the
+		// compiler is simultaneously trying to pick which overloaded variant
+		// of assertDoesNotThrow() to invoke.
 		//
 		// assertDoesNotThrow(foo::overloaded);