Introduce new @MethodSource syntax to differentiate overloaded local factory methods

Closes #3080
Closes #3101

Author: juliette-derancourt

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

@@ -38,7 +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
 
 ==== Deprecations and Breaking Changes
 

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

@@ -1406,9 +1406,11 @@ include::{testDir}/example/ExternalMethodSourceDemo.java[tags=external_MethodSou
 Factory methods can declare parameters, which will be provided by registered
 implementations of the `ParameterResolver` extension API. In the following example, the
 factory method is referenced by its name since there is only one such method in the test
-class. If there are several methods with the same name, the factory method must be
-referenced by its fully qualified method name – for example,
-`@MethodSource("example.MyTests#factoryMethodWithArguments(java.lang.String)")`.
+class. If there are several local methods with the same name, parameters can also be
+provided to differentiate them – for example, `@MethodSource("factoryMethod()")` or
+`@MethodSource("factoryMethod(java.lang.String)")`. Alternatively, the factory method
+can be referenced by its fully qualified method name, e.g.
+`@MethodSource("example.MyTests#factoryMethod(java.lang.String)")`.
 
 [source,java,indent=0]
 ----

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

@@ -28,6 +28,7 @@
 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;
@@ -62,10 +63,21 @@ private Method getFactoryMethod(ExtensionContext context, String factoryMethodNa
 		if (StringUtils.isBlank(factoryMethodName)) {
 			factoryMethodName = testMethod.getName();
 		}
-		if (factoryMethodName.contains(".") || factoryMethodName.contains("#")) {
+		if (looksLikeAFullyQualifiedMethodName(factoryMethodName)) {
 			return getFactoryMethodByFullyQualifiedName(factoryMethodName);
 		}
-		return getFactoryMethodBySimpleName(context.getRequiredTestClass(), testMethod, factoryMethodName);
+		return getFactoryMethodBySimpleOrQualifiedName(context.getRequiredTestClass(), testMethod, factoryMethodName);
+	}
+
+	private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodName) {
+		if (factoryMethodName.contains("#")) {
+			return true;
+		}
+		if (factoryMethodName.contains(".") && factoryMethodName.contains("(")) {
+			// Excluding cases of simple method names with parameters
+			return factoryMethodName.indexOf(".") < factoryMethodName.indexOf("(");
+		}
+		return factoryMethodName.contains(".");
 	}
 
 	private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodName) {
@@ -79,19 +91,41 @@ private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodN
 				methodParameters, className)));
 	}
 
+	private Method getFactoryMethodBySimpleOrQualifiedName(Class<?> testClass, Method testMethod,
+			String simpleOrQualifiedMethodName) {
+		String[] methodParts = ReflectionUtils.parseQualifiedMethodName(simpleOrQualifiedMethodName);
+		String methodSimpleName = methodParts[0];
+		String methodParameters = methodParts[1];
+
+		List<Method> factoryMethods = findFactoryMethodsBySimpleName(testClass, testMethod, methodSimpleName);
+		if (factoryMethods.size() == 1) {
+			return factoryMethods.get(0);
+		}
+
+		List<Method> exactMatches = filterFactoryMethodsWithMatchingParameters(factoryMethods,
+			simpleOrQualifiedMethodName, methodParameters);
+		Preconditions.condition(exactMatches.size() == 1,
+			() -> format("%d factory methods named [%s] were found in class [%s]: %s", factoryMethods.size(),
+				simpleOrQualifiedMethodName, testClass.getName(), factoryMethods));
+		return exactMatches.get(0);
+	}
+
 	/**
 	 * Find all methods in the given {@code testClass} with the desired {@code factoryMethodName}
 	 * which have return types that can be converted to a {@link Stream}, ignoring the
 	 * {@code testMethod} itself as well as any {@code @Test}, {@code @TestTemplate},
 	 * or {@code @TestFactory} methods with the same name.
 	 */
-	private Method getFactoryMethodBySimpleName(Class<?> testClass, Method testMethod, String factoryMethodName) {
+	private List<Method> findFactoryMethodsBySimpleName(Class<?> testClass, Method testMethod,
+			String factoryMethodName) {
 		Predicate<Method> isCandidate = candidate -> factoryMethodName.equals(candidate.getName())
 				&& !testMethod.equals(candidate);
 		List<Method> candidates = ReflectionUtils.findMethods(testClass, isCandidate);
+
 		Predicate<Method> isFactoryMethod = method -> isConvertibleToStream(method.getReturnType())
 				&& !isTestMethod(method);
 		List<Method> factoryMethods = candidates.stream().filter(isFactoryMethod).collect(toList());
+
 		Preconditions.condition(factoryMethods.size() > 0, () -> {
 			// If we didn't find the factory method using the isFactoryMethod Predicate, perhaps
 			// the specified factory method has an invalid return type or is a test method.
@@ -104,10 +138,18 @@ private Method getFactoryMethodBySimpleName(Class<?> testClass, Method testMetho
 			// Otherwise, report that we didn't find anything.
 			return format("Could not find factory method [%s] in class [%s]", factoryMethodName, testClass.getName());
 		});
-		Preconditions.condition(factoryMethods.size() == 1,
-			() -> format("%d factory methods named [%s] were found in class [%s]: %s", factoryMethods.size(),
-				factoryMethodName, testClass.getName(), factoryMethods));
-		return factoryMethods.get(0);
+		return factoryMethods;
+	}
+
+	private static List<Method> filterFactoryMethodsWithMatchingParameters(List<Method> factoryMethods,
+			String factoryMethodName, String factoryMethodParameters) {
+		if (!factoryMethodName.endsWith(")")) {
+			// If parameters are not specified, no choice is made
+			return factoryMethods;
+		}
+		Predicate<Method> hasRequiredParameters = method -> factoryMethodParameters.equals(
+			ClassUtils.nullSafeToString(method.getParameterTypes()));
+		return factoryMethods.stream().filter(hasRequiredParameters).collect(toList());
 	}
 
 	private boolean isTestMethod(Method candidate) {

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

@@ -394,6 +394,20 @@ void providesArgumentsUsingFullyQualifiedNameWithParameter() {
 			assertThat(arguments).containsExactly(array("foo!"), array("bar!"));
 		}
 
+		@Test
+		void providesArgumentsUsingSimpleNameWithoutParameter() {
+			var arguments = provideArguments("stringStreamProviderWithOrWithoutParameter()");
+
+			assertThat(arguments).containsExactly(array("foo"), array("bar"));
+		}
+
+		@Test
+		void providesArgumentsUsingSimpleNameWithParameter() {
+			var arguments = provideArguments("stringStreamProviderWithOrWithoutParameter(java.lang.String)");
+
+			assertThat(arguments).containsExactly(array("foo!"), array("bar!"));
+		}
+
 		@Test
 		void throwsExceptionWhenSeveralFactoryMethodsWithSameNameAreAvailable() {
 			var exception = assertThrows(PreconditionViolationException.class,

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

@@ -909,21 +909,44 @@ public static String[] parseFullyQualifiedMethodName(String fullyQualifiedMethod
 					+ "and then the method name, optionally followed by a parameter list enclosed in parentheses.");
 
 		String className = fullyQualifiedMethodName.substring(0, indexOfFirstHashtag);
-		String methodPart = fullyQualifiedMethodName.substring(indexOfFirstHashtag + 1);
-		String methodName = methodPart;
+		String qualifiedMethodName = fullyQualifiedMethodName.substring(indexOfFirstHashtag + 1);
+		String[] methodPart = parseQualifiedMethodName(qualifiedMethodName);
+
+		return new String[] { className, methodPart[0], methodPart[1] };
+	}
+
+	/**
+	 * Parse the supplied method name into a 2-element {@code String[]} with
+	 * the following content.
+	 *
+	 * <ul>
+	 *   <li>index {@code 0}: the name of the method</li>
+	 *   <li>index {@code 1}: a comma-separated list of parameter types, or a
+	 *       blank string if the method does not declare any formal parameters</li>
+	 * </ul>
+	 *
+	 * @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")
+	public static String[] parseQualifiedMethodName(String qualifiedMethodName) {
+		String methodName = qualifiedMethodName;
 		String methodParameters = "";
 
-		if (methodPart.endsWith("()")) {
-			methodName = methodPart.substring(0, methodPart.length() - 2);
+		if (qualifiedMethodName.endsWith("()")) {
+			methodName = qualifiedMethodName.substring(0, qualifiedMethodName.length() - 2);
 		}
-		else if (methodPart.endsWith(")")) {
-			int indexOfLastOpeningParenthesis = methodPart.lastIndexOf('(');
-			if ((indexOfLastOpeningParenthesis > 0) && (indexOfLastOpeningParenthesis < methodPart.length() - 1)) {
-				methodName = methodPart.substring(0, indexOfLastOpeningParenthesis);
-				methodParameters = methodPart.substring(indexOfLastOpeningParenthesis + 1, methodPart.length() - 1);
+		else if (qualifiedMethodName.endsWith(")")) {
+			int indexOfLastOpeningParenthesis = qualifiedMethodName.lastIndexOf('(');
+			if ((indexOfLastOpeningParenthesis > 0)
+					&& (indexOfLastOpeningParenthesis < qualifiedMethodName.length() - 1)) {
+				methodName = qualifiedMethodName.substring(0, indexOfLastOpeningParenthesis);
+				methodParameters = qualifiedMethodName.substring(indexOfLastOpeningParenthesis + 1,
+					qualifiedMethodName.length() - 1);
 			}
 		}
-		return new String[] { className, methodName, methodParameters };
+
+		return new String[] { methodName, methodParameters };
 	}
 
 	/**

platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java

@@ -55,6 +55,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.fixtures.TrackLogRecords;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.PreconditionViolationException;
 import org.junit.platform.commons.logging.LogRecordListener;
@@ -700,6 +702,25 @@ void parseFullyQualifiedMethodNameForMethodWithMultipleParameters() {
 				.containsExactly("com.example.Test", "method", "int, java.lang.Object");
 	}
 
+	@ParameterizedTest
+	@ValueSource(strings = { "method", "method()" })
+	void parseSimpleMethodNameForMethodWithoutParameters(String methodName) {
+		assertThat(ReflectionUtils.parseQualifiedMethodName(methodName))//
+				.containsExactly("method", "");
+	}
+
+	@Test
+	void parseSimpleMethodNameForMethodWithSingleParameter() {
+		assertThat(ReflectionUtils.parseQualifiedMethodName("method(java.lang.Object)"))//
+				.containsExactly("method", "java.lang.Object");
+	}
+
+	@Test
+	void parseSimpleMethodNameForMethodWithMultipleParameters() {
+		assertThat(ReflectionUtils.parseQualifiedMethodName("method(int, java.lang.Object)"))//
+				.containsExactly("method", "int, java.lang.Object");
+	}
+
 	@Test
 	@SuppressWarnings("deprecation")
 	void getOutermostInstancePreconditions() {