Merge pull request #233 from basil/asm

Reduce ASM dep in `ClassDescriptor`
jenkinsci · Jun 1, 2021 · 3da2d02 · 3da2d02
2 parents afc1340 + 8f337b8
commit 3da2d02
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 2 deletions.
diff --git a/core/src/main/java/org/kohsuke/stapler/CaptureParameterNameTransformation.java b/core/src/main/java/org/kohsuke/stapler/CaptureParameterNameTransformation.java
@@ -44,6 +44,7 @@
  * Groovy AST transformation that capture necessary parameter names.
  *
  * @author Kohsuke Kawaguchi
+ * @see CapturedParameterNames
  */
 @MetaInfServices
 @GroovyASTTransformation

diff --git a/core/src/main/java/org/kohsuke/stapler/CapturedParameterNames.java b/core/src/main/java/org/kohsuke/stapler/CapturedParameterNames.java
@@ -32,6 +32,7 @@
  * to do than generating the same files that the annotation processor does.
  *
  * @author Kohsuke Kawaguchi
+ * @see CaptureParameterNameTransformation
  */
 @Retention(RUNTIME)
 public @interface CapturedParameterNames {

diff --git a/core/src/main/java/org/kohsuke/stapler/ClassDescriptor.java b/core/src/main/java/org/kohsuke/stapler/ClassDescriptor.java
@@ -35,9 +35,11 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Executable;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.lang.reflect.Parameter;
 import java.lang.reflect.ParameterizedType;
 import java.net.URL;
 import java.util.ArrayList;
@@ -52,6 +54,7 @@
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.logging.Logger;
+import java.util.stream.Stream;
 
 import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.WARNING;
@@ -191,9 +194,15 @@ public static String[] loadParameterNames(Method m) {
         CapturedParameterNames cpn = m.getAnnotation(CapturedParameterNames.class);
         if(cpn!=null)   return cpn.value();
 
+        // reflection is the most efficient and supported system
+        String[] n = loadParameterNamesFromReflection(m);
+        if (n != null) {
+            return n;
+        }
+
         // debug information, if present, is more trustworthy
         try {
-            String[] n = ASM.loadParametersFromAsm(m);
+            n = ASM.loadParametersFromAsm(m);
             if (n!=null)    return n;
         } catch (LinkageError e) {
             LOGGER.log(FINE, "Incompatible ASM", e);
@@ -228,9 +237,15 @@ public static String[] loadParameterNames(Constructor<?> m) {
         CapturedParameterNames cpn = m.getAnnotation(CapturedParameterNames.class);
         if(cpn!=null)   return cpn.value();
 
+        // reflection is the most efficient and supported system
+        String[] n = loadParameterNamesFromReflection(m);
+        if (n != null) {
+            return n;
+        }
+
         // debug information, if present, is more trustworthy
         try {
-            String[] n = ASM.loadParametersFromAsm(m);
+            n = ASM.loadParametersFromAsm(m);
             if (n!=null)    return n;
         } catch (LinkageError e) {
             LOGGER.log(FINE, "Incompatible ASM", e);
@@ -242,6 +257,15 @@ public static String[] loadParameterNames(Constructor<?> m) {
         return EMPTY_ARRAY;
     }
 
+    static String[] loadParameterNamesFromReflection(final Executable m) {
+        Parameter[] ps = m.getParameters();
+        if (Stream.of(ps).allMatch(Parameter::isNamePresent)) {
+            return Stream.of(ps).map(Parameter::getName).toArray(String[]::new);
+        } else {
+            return null;
+        }
+    }
+
     /**
      * Determines the constructor parameter names.
      *

diff --git a/core/src/test/java/org/kohsuke/stapler/ClassDescriptorTest.java b/core/src/test/java/org/kohsuke/stapler/ClassDescriptorTest.java
@@ -28,6 +28,32 @@ public class ClassDescriptorTest {
         assertEquals("[a, b, x]",Arrays.asList(names).toString());
     }
 
+    @Test public void loadParameterNamesFromReflection() throws Exception {
+        // collect test cases
+        Map<String,Method> testCases = new HashMap<String,Method>();
+        for (Method m : ClassDescriptorTest.class.getDeclaredMethods())
+            if (m.getName().startsWith("methodWith"))
+                testCases.put(m.getName().substring(10), m);
+        // expected results
+        Map<String,String[]> expected = new HashMap<String,String[]>();
+        expected.put("NoParams", new String[0]);
+        expected.put("NoParams_static", new String[0]);
+        expected.put("ManyParams", new String[] { "a", "b", "c", "d", "e", "f", "g", "h", "i" });
+        expected.put("Params_static", new String[] { "abc", "def", "ghi" });
+        // run tests
+        for (Map.Entry<String,String[]> entry : expected.entrySet()) {
+            Method testMethod = testCases.get(entry.getKey());
+            assertNotNull("Method missing for " + entry.getKey(), testMethod);
+            String[] result = ClassDescriptor.loadParameterNamesFromReflection(testMethod);
+            assertNotNull("Null result for " + entry.getKey(), result);
+            if (!Arrays.equals(entry.getValue(), result)) {
+                StringBuilder buf = new StringBuilder('|');
+                for (String s : result) buf.append(s).append('|');
+                fail("Unexpected result for " + entry.getKey() + ": " + buf);
+            }
+        }
+    }
+
     @Test public void loadParametersFromAsm() throws Exception {
         // get private method that is being tested
         Method lpfa = ClassDescriptor.ASM.class.getDeclaredMethod(