Fix bug and polish contribution

This commit fixes a bug regarding whitespace in parameter lists for
local factory methods specified via `@MethodSource`. Specifically,
whitespace is now supported.

For example, @MethodSource("data(int, java.lang.String)") is now
supported (note the space after the comma).

As a side note, whitespace in parameter lists was already supported for
a fully qualified method name.

See #3080, #3101

Author: sbrannen

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

@@ -38,8 +38,8 @@ JUnit repository on GitHub.
 
 ==== Bug Fixes
 
-* Introduce new `@MethodSource` syntax to add the possibility to explicitly select
-  an overloaded local factory method without specifying its fully qualified name
+* New `@MethodSource` syntax for explicitly selecting an overloaded local factory method
+  without specifying its fully qualified name.
 
 ==== Deprecations and Breaking Changes
 

junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java

@@ -11,13 +11,14 @@
 package org.junit.jupiter.params.provider;
 
 import static java.lang.String.format;
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.junit.platform.commons.util.AnnotationUtils.isAnnotated;
 import static org.junit.platform.commons.util.CollectionUtils.isConvertibleToStream;
 
 import java.lang.reflect.Method;
-import java.util.Arrays;
 import java.util.List;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
@@ -28,7 +29,6 @@
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.params.support.AnnotationConsumer;
 import org.junit.platform.commons.JUnitException;
-import org.junit.platform.commons.util.ClassUtils;
 import org.junit.platform.commons.util.CollectionUtils;
 import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.commons.util.ReflectionUtils;
@@ -50,7 +50,7 @@ public void accept(MethodSource annotation) {
 	public Stream<Arguments> provideArguments(ExtensionContext context) {
 		Object testInstance = context.getTestInstance().orElse(null);
 		// @formatter:off
-		return Arrays.stream(this.methodNames)
+		return stream(this.methodNames)
 				.map(factoryMethodName -> getFactoryMethod(context, factoryMethodName))
 				.map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance))
 				.flatMap(CollectionUtils::toStream)
@@ -73,11 +73,20 @@ private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodNa
 		if (factoryMethodName.contains("#")) {
 			return true;
 		}
-		if (factoryMethodName.contains(".") && factoryMethodName.contains("(")) {
-			// Excluding cases of simple method names with parameters
-			return factoryMethodName.indexOf(".") < factoryMethodName.indexOf("(");
+		int indexOfDot = factoryMethodName.indexOf(".");
+		if (indexOfDot == -1) {
+			return false;
 		}
-		return factoryMethodName.contains(".");
+		int indexOfOpeningParenthesis = factoryMethodName.indexOf("(");
+		if (indexOfOpeningParenthesis > 0) {
+			// Exclude simple method names with parameters
+			return indexOfDot < indexOfOpeningParenthesis;
+		}
+		// If we get this far, we conclude the supplied factory method name "looks"
+		// like it was intended to be a fully qualified method name, even if the
+		// syntax is invalid. We do this in order to provide better diagnostics for
+		// the user when a fully qualified method name is in fact invalid.
+		return true;
 	}
 
 	private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodName) {
@@ -143,12 +152,21 @@ private List<Method> findFactoryMethodsBySimpleName(Class<?> testClass, Method t
 
 	private static List<Method> filterFactoryMethodsWithMatchingParameters(List<Method> factoryMethods,
 			String factoryMethodName, String factoryMethodParameters) {
+
 		if (!factoryMethodName.endsWith(")")) {
-			// If parameters are not specified, no choice is made
+			// If parameters are not specified, nothing is filtered.
 			return factoryMethods;
 		}
-		Predicate<Method> hasRequiredParameters = method -> factoryMethodParameters.equals(
-			ClassUtils.nullSafeToString(method.getParameterTypes()));
+
+		// Compare against canonical parameter list, ignoring whitespace.
+		String parameterList = factoryMethodParameters.replaceAll("\\s+", "");
+		Predicate<Method> hasRequiredParameters = method -> {
+			if (parameterList.isEmpty()) {
+				return method.getParameterCount() == 0;
+			}
+			return parameterList.equals(stream(method.getParameterTypes()).map(Class::getName).collect(joining(",")));
+		};
+
 		return factoryMethods.stream().filter(hasRequiredParameters).collect(toList());
 	}
 

junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java

@@ -37,6 +37,7 @@
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.DefaultExecutableInvoker;
 import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
+import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.PreconditionViolationException;
 import org.junit.platform.commons.util.ReflectionUtils;
@@ -408,16 +409,25 @@ void providesArgumentsUsingSimpleNameWithParameter() {
 			assertThat(arguments).containsExactly(array("foo!"), array("bar!"));
 		}
 
+		@ParameterizedTest
+		@ValueSource(strings = { "java.lang.String,java.lang.String", "java.lang.String, java.lang.String",
+				"java.lang.String,    java.lang.String" })
+		void providesArgumentsUsingSimpleNameWithMultipleParameters(String params) {
+			var arguments = provideArguments("stringStreamProviderWithOrWithoutParameter(" + params + ")");
+			assertThat(arguments).containsExactly(array("foo!!"), array("bar!!"));
+		}
+
 		@Test
 		void throwsExceptionWhenSeveralFactoryMethodsWithSameNameAreAvailable() {
 			var exception = assertThrows(PreconditionViolationException.class,
 				() -> provideArguments("stringStreamProviderWithOrWithoutParameter").toArray());
 
 			assertThat(exception.getMessage())//
-					.startsWith("2 factory methods named [stringStreamProviderWithOrWithoutParameter] were found in "
+					.startsWith("3 factory methods named [stringStreamProviderWithOrWithoutParameter] were found in "
 							+ "class [org.junit.jupiter.params.provider.MethodArgumentsProviderTests$TestCase]: ")//
 					.contains("stringStreamProviderWithOrWithoutParameter()",
-						"stringStreamProviderWithOrWithoutParameter(java.lang.String)");
+						"stringStreamProviderWithOrWithoutParameter(java.lang.String)",
+						"stringStreamProviderWithOrWithoutParameter(java.lang.String,java.lang.String)");
 		}
 
 		@Test
@@ -428,6 +438,16 @@ void providesArgumentsUsingFactoryMethodSelectedViaFullyQualifiedNameWithParamet
 			assertThat(arguments).containsExactly(array("foo!"), array("bar!"));
 		}
 
+		@ParameterizedTest
+		@ValueSource(strings = { "java.lang.String,java.lang.String", "java.lang.String, java.lang.String",
+				"java.lang.String,    java.lang.String" })
+		void providesArgumentsUsingFactoryMethodSelectedViaFullyQualifiedNameWithMultipleParameters(String params) {
+			var arguments = provideArguments(
+				TestCase.class.getName() + "#stringStreamProviderWithOrWithoutParameter(" + params + ")");
+
+			assertThat(arguments).containsExactly(array("foo!!"), array("bar!!"));
+		}
+
 		@Test
 		void providesArgumentsUsingFactoryMethodSelectedViaFullyQualifiedNameWithoutParameter() {
 			var arguments = provideArguments(
@@ -522,6 +542,10 @@ static Stream<String> stringStreamProviderWithOrWithoutParameter(String paramete
 			return stringStreamProviderWithParameter(parameter);
 		}
 
+		static Stream<String> stringStreamProviderWithOrWithoutParameter(String parameter1, String parameter2) {
+			return stringStreamProviderWithParameter(parameter1 + parameter2);
+		}
+
 		// @ParameterizedTest
 		// @MethodSource // use default, inferred factory method
 		void overloadedStringStreamProvider(Object parameter) {

junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java

@@ -928,7 +928,7 @@ public static String[] parseFullyQualifiedMethodName(String fullyQualifiedMethod
 	 * @param qualifiedMethodName a qualified method name, never {@code null} or blank
 	 * @return a 2-element array of strings containing the parsed values
 	 */
-	@API(status = INTERNAL, since = "1.9")
+	@API(status = INTERNAL, since = "1.9.2")
 	public static String[] parseQualifiedMethodName(String qualifiedMethodName) {
 		String methodName = qualifiedMethodName;
 		String methodParameters = "";