fix(reflect): reject char for short and keep mismatch diagnostics

CHANGES.txt

@@ -1,4 +1,5 @@
 Current (7.13.0)
+Fixed: GITHUB-3378, GITHUB-3364: ReflectionRecipes no longer treats Character as assignable to short (char does not widen to short). MethodMatcherException stringifies primitive arrays instead of throwing ClassCastException, leftover data-provider arguments still produce a diagnostic when the injection target is a constructor holder, and the unused out-of-bounds lenientMatch helpers are removed (Burak Kalaycı)
 Fixed: GITHUB-3366: ToStringHelper.omitNulls() and omitEmptyStrings() now inspect the original value instead of the already-stringified form, so ITestResult.toString() no longer renders output={null} when a result has no reporter output (Burak Kalaycı)
 New: GITHUB-3322: A suite file may declare the schema instead of a doctype, with xsi:noNamespaceSchemaLocation="https://testng.org/testng-1.1.xsd" on <suite>, and is then validated against testng-1.1.xsd. A suite declaring neither is validated against the schema too, where it was previously validated by nothing: error() discarded every violation unless a doctype had been seen, and the parser was configured for DTD validation, so a document carrying no DTD had no grammar to violate. A suite declaring a doctype keeps being validated against the DTD, so nothing changes for the files that have one. TestNG always uses the schema it ships, never the URL the file names, so a parse still touches no network (Julien Herr)
 Changed: GITHUB-3322: The hint printed for a suite file that declares no grammar now offers the schema first and the doctype second, and is no longer printed for a file that declares a schema (Julien Herr)

testng-core/src/main/java/org/testng/internal/reflect/MethodMatcherException.java

@@ -1,5 +1,6 @@
 package org.testng.internal.reflect;
 
+import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
@@ -43,7 +44,7 @@ static String generateMessage(
   }
 
   public static String generateMessage(
-      final String message, final Method method, final Object[] args) {
+      final String message, final @Nullable Method method, final Object[] args) {
     return generateMessage(
         message,
         method != null ? method.getName() : null,
@@ -87,10 +88,14 @@ private static String generateMessage(
   }
 
   private static String stringify(Object object) {
-    if (object.getClass().isArray()) {
-      return Arrays.toString((Object[]) object);
-    } else {
+    if (!object.getClass().isArray()) {
       return object.toString();
     }
+    final int length = Array.getLength(object);
+    final Object[] elements = new Object[length];
+    for (int i = 0; i < length; i++) {
+      elements[i] = Array.get(object, i);
+    }
+    return Arrays.toString(elements);
   }
 }

testng-core/src/main/java/org/testng/internal/reflect/ReflectionRecipes.java

@@ -11,6 +11,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import org.jspecify.annotations.Nullable;
 import org.testng.ITestContext;
 import org.testng.ITestResult;
 import org.testng.TestNGException;
@@ -67,7 +68,7 @@ private static void initAssignableMapping() {
     ASSIGNABLE_MAPPING.put(
         long.class, Arrays.asList(Integer.class, Short.class, Character.class, Byte.class));
     ASSIGNABLE_MAPPING.put(int.class, Arrays.asList(Short.class, Character.class, Byte.class));
-    ASSIGNABLE_MAPPING.put(short.class, Arrays.asList(Character.class, Byte.class));
+    ASSIGNABLE_MAPPING.put(short.class, Arrays.asList(Byte.class));
   }
 
   private ReflectionRecipes() {
@@ -170,7 +171,7 @@ public static Class<?>[] classesFromParameters(final Parameter[] parameters) {
    * @param method any valid method.
    * @return extracted method parameters.
    */
-  public static Parameter[] getMethodParameters(final Method method) {
+  public static Parameter[] getMethodParameters(final @Nullable Method method) {
     if (method == null) {
       return new Parameter[] {};
     }
@@ -282,37 +283,6 @@ public static boolean exactMatch(final Class<?>[] classes, final Object[] args)
     return matching;
   }
 
-  /**
-   * Matches an array of parameters to an array of instances.
-   *
-   * @return matches or not
-   * @see #lenientMatch(Class[], Object[])
-   */
-  public static boolean lenientMatch(final Parameter[] parameters, final Object[] args) {
-    return lenientMatch(classesFromParameters(parameters), args);
-  }
-
-  /**
-   * Matches an array of class instances to an array of instances. Such that {int, boolean, float}
-   * matches {int, boolean}
-   *
-   * @param classes array of class instances to check against.
-   * @param args instances to be verified.
-   * @return matches or not
-   */
-  public static boolean lenientMatch(final Class<?>[] classes, final Object[] args) {
-    boolean matching = true;
-    int i = 0;
-    for (final Class<?> clazz : classes) {
-      matching = ReflectionRecipes.isInstanceOf(clazz, args[i]);
-      i++;
-      if (!matching) {
-        break;
-      }
-    }
-    return matching;
-  }
-
   /**
    * Omits 1. org.testng.ITestContext or its implementations from input array 2.
    * org.testng.ITestResult or its implementations from input array 3. org.testng.xml.XmlTest or its
@@ -361,7 +331,7 @@ public static Parameter[] filter(
    * @param parameters array of parameter instances under question.
    * @param filters filters to use.
    * @param args user supplied arguments.
-   * @param injectionMethod current test method.
+   * @param injectionMethod current test method, or {@code null} when there is no holder.
    * @param context current test context.
    * @param testResult on going test results.
    * @return injected arguments.
@@ -370,7 +340,7 @@ public static Object[] inject(
       final Parameter[] parameters,
       final Set<InjectableParameter> filters,
       final Object[] args,
-      final Method injectionMethod,
+      final @Nullable Method injectionMethod,
       final ITestContext context,
       final ITestResult testResult) {
     return nativelyInject(parameters, filters, args, injectionMethod, context, testResult);
@@ -380,7 +350,7 @@ private static Object[] nativelyInject(
       final Parameter[] parameters,
       final Set<InjectableParameter> filters,
       final Object[] args,
-      final Object injectionMethod,
+      final @Nullable Object injectionMethod,
       final ITestContext context,
       final ITestResult testResult) {
     if (filters == null || filters.isEmpty()) {
@@ -431,17 +401,21 @@ private static Object[] nativelyInject(
       String prefix =
           "Missing one or more parameters that are being injected by the data provider. "
               + "Please add the below arguments to the ";
-      String msg = null;
-      if (injectionMethod instanceof Method) {
-        msg =
-            MethodMatcherException.generateMessage(
-                prefix + "method.", (Method) injectionMethod, queue.backingList.toArray());
-      } else if (injectionMethod instanceof Constructor) {
+      final String msg;
+      if (injectionMethod instanceof Constructor) {
         msg =
             MethodMatcherException.generateMessage(
                 prefix + "constructor.",
                 (Constructor<?>) injectionMethod,
                 queue.backingList.toArray());
+      } else if (injectionMethod instanceof Method || injectionMethod == null) {
+        msg =
+            MethodMatcherException.generateMessage(
+                prefix + "method.", (Method) injectionMethod, queue.backingList.toArray());
+      } else {
+        throw new TestNGException(
+            "Injection holder must be a method or constructor, got "
+                + injectionMethod.getClass().getName());
       }
 
       boolean block = RuntimeBehavior.useStrictParameterMatching();

testng-core/src/test/java/test/reflect/ReflectionRecipesTest.java

@@ -3,6 +3,7 @@
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.testng.internal.reflect.ReflectionRecipes.exactMatch;
 import static org.testng.internal.reflect.ReflectionRecipes.getMethodParameters;
 import static org.testng.internal.reflect.ReflectionRecipes.isOrImplementsInterface;
@@ -24,6 +25,7 @@
 import org.testng.annotations.NoInjection;
 import org.testng.annotations.Test;
 import org.testng.internal.reflect.InjectableParameter;
+import org.testng.internal.reflect.MethodMatcherException;
 import org.testng.internal.reflect.ReflectionRecipes;
 import org.testng.log4testng.Logger;
 import org.testng.xml.XmlTest;
@@ -235,6 +237,9 @@ public Object[][] primitiveAndArgument() {
       // Would narrow, so it does not match.
       {int.class, 1L, false},
       {float.class, 1.0d, false},
+      // byte widens to short; char does not (JLS 5.1.2).
+      {short.class, (byte) 1, true},
+      {short.class, 'a', false},
     };
   }
 
@@ -244,6 +249,43 @@ public void testIsInstanceOfHonoursWidening(
     assertThat(ReflectionRecipes.isInstanceOf(primitive, argument)).isEqualTo(expected);
   }
 
+  @Test
+  public void generateMessageStringifiesPrimitiveArray() throws Exception {
+    final Method method = ReflectionRecipesTest.class.getMethod("oneInt", int.class);
+    final String message =
+        MethodMatcherException.generateMessage("probe", method, new Object[] {new int[] {1, 2}});
+    assertThat(message).contains("[1, 2]");
+  }
+
+  @Test
+  public void leftoverArgumentsWithNullMethodKeepDiagnostic() {
+    final String previous = System.getProperty("strictParameterMatch");
+    System.setProperty("strictParameterMatch", "true");
+    try {
+      assertThatThrownBy(
+              () ->
+                  ReflectionRecipes.inject(
+                      getMethodParameters(ReflectionRecipesTest.class, "oneInt"),
+                      InjectableParameter.Assistant.ALL_INJECTS,
+                      new Object[] {1, "leftover"},
+                      (Method) null,
+                      null,
+                      null))
+          .isInstanceOf(MethodMatcherException.class)
+          .hasMessageContaining("Missing one or more parameters")
+          .hasMessageContaining("leftover")
+          .hasMessageContaining("Method: null");
+    } finally {
+      if (previous == null) {
+        System.clearProperty("strictParameterMatch");
+      } else {
+        System.setProperty("strictParameterMatch", previous);
+      }
+    }
+  }
+
+  public static void oneInt(int value) {}
+
   private interface T {
     void s0(TestContextJustForTesting testContext, int i, Boolean b);
 

testng-core/src/test/java/test/reflect/TestMethodMatcher.java

@@ -82,6 +82,10 @@ public Object[][] methodParamFailingPairs() {
       new Object[] {"goodTestIssue122", new Object[] {3, "three", "four"}},
       new Object[] {"badTestIssue122", new Object[] {3, "three", "four"}},
       new Object[] {"mixedArgs", new Object[] {3, true, "three", "four"}},
+      // char does not widen to short; reject here instead of Method.invoke()
+      new Object[] {"takesShort", new Object[] {'a'}},
+      // primitive array must stay in the diagnostic, not ClassCastException
+      new Object[] {"takesString", new Object[] {new int[] {1, 2}}},
     };
   }
 
@@ -110,16 +114,28 @@ public void testNegativeCaseMatcher(
         new DataProviderMethodMatcher(
             new MethodMatcherContext(method, params, iTestContext, iTestResult));
     assertThat(matcher.conforms()).isFalse();
-    assertThatThrownBy(
-            () -> {
-              method.invoke(new TestMethodMatcher(), matcher.getConformingArguments());
-            })
-        .isInstanceOf(MethodMatcherException.class)
-        // separate lines are used here to avoid \n vs \r\n if running tests in Windows
-        .hasMessageContaining(
-            "has no parameters defined but was found to be using a data provider (either explicitly specified or inherited from class level annotation")
-        .hasMessageContaining("Method: ")
-        .hasMessageContaining("Arguments: ");
+    var thrown =
+        assertThatThrownBy(
+                () -> {
+                  method.invoke(new TestMethodMatcher(), matcher.getConformingArguments());
+                })
+            .isInstanceOf(MethodMatcherException.class)
+            // separate lines are used here to avoid \n vs \r\n if running tests in Windows
+            .hasMessageContaining(
+                "has no parameters defined but was found to be using a data provider (either explicitly specified or inherited from class level annotation")
+            .hasMessageContaining("Method: ")
+            .hasMessageContaining("Arguments: ");
+    if ("takesString".equals(methodName)) {
+      thrown.hasMessageContaining("[1, 2]");
+    }
+  }
+
+  public void takesShort(short value) {
+    throw new AssertionError("char must not match short");
+  }
+
+  public void takesString(String value) {
+    throw new AssertionError("int[] must not match String");
   }
 
   public void goodTestIssue122(String s, String[] strings) {