From 855394eb64654ce31b835a883fd6486a6c3622b5 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Thu, 5 Feb 2026 09:59:18 +0100 Subject: [PATCH 1/2] XBEAN-355 - XbeanAsmParameterNameLoader incorrectly maps parameter names when methods contain long or double parameters --- .../recipe/XbeanAsmParameterNameLoader.java | 51 ++++++++++++++----- .../xbean/recipe/ParameterNameLoaderTest.java | 11 ++++ 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java b/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java index 8b5b1395..8338df31 100644 --- a/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java +++ b/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java @@ -265,6 +265,7 @@ public Map getExceptions() { return exceptions; } + @Override public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { if (!name.equals(this.methodName)) { return null; @@ -273,6 +274,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si try { final List parameterNames; final boolean isStaticMethod; + final Type[] paramTypes; final int paramLen; if (methodName.equals("")) { @@ -282,10 +284,18 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si } paramLen = constructor.getParameterTypes().length; - parameterNames = new ArrayList(paramLen); - parameterNames.addAll(Collections.nCopies(paramLen, null)); + parameterNames = new ArrayList<>(paramLen); + parameterNames.addAll(Collections.nCopies(paramLen, null)); constructorParameters.put(constructor, parameterNames); isStaticMethod = false; + + // Get ASM Type array for slot computation + paramTypes = new Type[paramLen]; + Class[] types = constructor.getParameterTypes(); + for (int i = 0; i < paramLen; i++) { + paramTypes[i] = Type.getType(types[i]); + } + } else { Method method = methodMap.get(desc); if (method == null) { @@ -293,31 +303,44 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si } paramLen = method.getParameterTypes().length; - parameterNames = new ArrayList(paramLen); - parameterNames.addAll(Collections.nCopies(paramLen, null)); + parameterNames = new ArrayList<>(paramLen); + parameterNames.addAll(Collections.nCopies(paramLen, null)); methodParameters.put(method, parameterNames); isStaticMethod = Modifier.isStatic(method.getModifiers()); + + // Get ASM Type array for slot computation + Class[] types = method.getParameterTypes(); + paramTypes = new Type[types.length]; + for (int i = 0; i < types.length; i++) { + paramTypes[i] = Type.getType(types[i]); + } } + // Build slot -> parameter index map + final Map slotToParamIndex = new HashMap<>(); + int slot = isStaticMethod ? 0 : 1; // slot 0 reserved for "this" in non-static + for (int i = 0; i < paramLen; i++) { + slotToParamIndex.put(slot, i); + slot += paramTypes[i].getSize(); // 1 for normal, 2 for long/double + } + + // Return visitor that assigns names correctly return new MethodVisitor(ASM_VERSION) { - // assume static method until we get a first parameter name + @Override public void visitLocalVariable(String name, String description, String signature, Label start, Label end, int index) { - if (isStaticMethod) { - if (paramLen > index) { - parameterNames.set(index, name); - } - } else if (index > 0) { - // for non-static the 0th arg is "this" so we need to offset by -1 - if (paramLen >= index) { - parameterNames.set(index - 1, name); - } + final Integer paramIndex = slotToParamIndex.get(index); + if (paramIndex != null) { + parameterNames.set(paramIndex, name); } } }; + } catch (Exception e) { this.exceptions.put(signature, e); } + return null; } + } } \ No newline at end of file diff --git a/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java b/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java index 293b608d..01332c70 100644 --- a/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java +++ b/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java @@ -107,6 +107,14 @@ public void testEmptyMethod() throws Exception { assertParameterNames(Collections.emptyList(), method); } + public void testXBEAN355() throws Exception{ + Constructor constructor = TestClass.class.getConstructor(String.class, String.class, int.class, double.class, double.class, double.class, + double.class, double.class, double.class, double.class, boolean.class, boolean.class, boolean.class, int.class, + String.class, boolean.class); + assertParameterNames(Arrays.asList("svmType", "kernelType", "degree", "gamma", "coef0", "nu", + "cost", "eps", "p", "cacheSize", "shrinking", "probability", "crossValidation", "nFold", "modelName", "debug"), constructor); + } + @SuppressWarnings({"UnusedDeclaration"}) private static class ParentTestClass { public void inheritedMethod(Map nothing) {} @@ -119,6 +127,9 @@ public TestClass(int foo) {} public TestClass(Object bar) {} public TestClass(Object[] objectArray) {} private TestClass(Double scotch) {} + public TestClass(String svmType, String kernelType, int degree, double gamma, double coef0, double nu, + double cost, double eps, double p, double cacheSize, boolean shrinking, + boolean probability, boolean crossValidation, int nFold, String modelName, boolean debug) {} public static void factoryMethod(int a, Object b, Long c) {} public static void factoryMethod(int beer) {} From 59dcabea677968d8618df1dc581b91172a1d51b1 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Thu, 5 Feb 2026 13:11:05 +0100 Subject: [PATCH 2/2] Add tests for non static inner class and non static separate class. Fix case for non static inner class --- .../recipe/XbeanAsmParameterNameLoader.java | 18 +++++++++++- .../org/apache/xbean/recipe/MultiArgCtor.java | 25 +++++++++++++++++ .../xbean/recipe/ParameterNameLoaderTest.java | 28 ++++++++++++++++++- 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 xbean-reflect/src/test/java/org/apache/xbean/recipe/MultiArgCtor.java diff --git a/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java b/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java index 8338df31..a95d202f 100644 --- a/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java +++ b/xbean-reflect/src/main/java/org/apache/xbean/recipe/XbeanAsmParameterNameLoader.java @@ -224,9 +224,11 @@ private static class AllParameterNamesDiscoveringVisitor extends ClassVisitor { private final String methodName; private final Map methodMap = new HashMap(); private final Map constructorMap = new HashMap(); + private final Class clazz; public AllParameterNamesDiscoveringVisitor(Class type, String methodName) { super(ASM_VERSION); + this.clazz = type; this.methodName = methodName; List methods = new ArrayList(Arrays.asList(type.getMethods())); @@ -240,6 +242,7 @@ public AllParameterNamesDiscoveringVisitor(Class type, String methodName) { public AllParameterNamesDiscoveringVisitor(Class type) { super(ASM_VERSION); + this.clazz = type; this.methodName = ""; List constructors = new ArrayList(Arrays.asList(type.getConstructors())); @@ -275,8 +278,13 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si final List parameterNames; final boolean isStaticMethod; final Type[] paramTypes; - final int paramLen; + + // Determine if this is a non-static inner class constructor + boolean isNonStaticInner = methodName.equals("") && + clazz.getEnclosingClass() != null && + !Modifier.isStatic(clazz.getModifiers()); + if (methodName.equals("")) { Constructor constructor = constructorMap.get(desc); if (constructor == null) { @@ -316,10 +324,18 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si } } + // Determine the starting parameter index (skip synthetic outer-class param if inner class) + final int paramOffset = (methodName.equals("") && isNonStaticInner) ? 1 : 0; + // Build slot -> parameter index map final Map slotToParamIndex = new HashMap<>(); int slot = isStaticMethod ? 0 : 1; // slot 0 reserved for "this" in non-static for (int i = 0; i < paramLen; i++) { + if (i < paramOffset) { + // skip synthetic outer-class parameter + slot += paramTypes[i].getSize(); + continue; + } slotToParamIndex.put(slot, i); slot += paramTypes[i].getSize(); // 1 for normal, 2 for long/double } diff --git a/xbean-reflect/src/test/java/org/apache/xbean/recipe/MultiArgCtor.java b/xbean-reflect/src/test/java/org/apache/xbean/recipe/MultiArgCtor.java new file mode 100644 index 00000000..257d77bb --- /dev/null +++ b/xbean-reflect/src/test/java/org/apache/xbean/recipe/MultiArgCtor.java @@ -0,0 +1,25 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.xbean.recipe; + +public class MultiArgCtor { + + @SuppressWarnings({"UnusedDeclaration"}) + public MultiArgCtor(String svmType, String kernelType, int degree, double gamma, double coef0, double nu, + double cost, double eps, double p, double cacheSize, boolean shrinking, + boolean probability, boolean crossValidation, int nFold, String modelName, boolean debug) {} +} diff --git a/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java b/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java index 01332c70..5e4c1dca 100644 --- a/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java +++ b/xbean-reflect/src/test/java/org/apache/xbean/recipe/ParameterNameLoaderTest.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; +import junit.framework.Test; import junit.framework.TestCase; /** @@ -107,7 +108,7 @@ public void testEmptyMethod() throws Exception { assertParameterNames(Collections.emptyList(), method); } - public void testXBEAN355() throws Exception{ + public void testXBEAN355() throws Exception{ Constructor constructor = TestClass.class.getConstructor(String.class, String.class, int.class, double.class, double.class, double.class, double.class, double.class, double.class, double.class, boolean.class, boolean.class, boolean.class, int.class, String.class, boolean.class); @@ -115,6 +116,31 @@ public void testXBEAN355() throws Exception{ "cost", "eps", "p", "cacheSize", "shrinking", "probability", "crossValidation", "nFold", "modelName", "debug"), constructor); } + public void testXBEAN355_2() throws Exception{ + Constructor constructor = MultiArgCtor.class.getConstructor(String.class, String.class, int.class, double.class, double.class, double.class, + double.class, double.class, double.class, double.class, boolean.class, boolean.class, boolean.class, int.class, + String.class, boolean.class); + assertParameterNames(Arrays.asList("svmType", "kernelType", "degree", "gamma", "coef0", "nu", + "cost", "eps", "p", "cacheSize", "shrinking", "probability", "crossValidation", "nFold", "modelName", "debug"), constructor); + } + + public void testXBEAN355_3() throws Exception{ + Constructor constructor = NonStaticInnerTestClass.class.getConstructor(ParameterNameLoaderTest.class, String.class, String.class, int.class, double.class, double.class, double.class, + double.class, double.class, double.class, double.class, boolean.class, boolean.class, boolean.class, int.class, + String.class, boolean.class); + assertParameterNames(Arrays.asList(null, "svmType", "kernelType", "degree", "gamma", "coef0", "nu", + "cost", "eps", "p", "cacheSize", "shrinking", "probability", "crossValidation", "nFold", "modelName", "debug"), constructor); + } + + @SuppressWarnings({"UnusedDeclaration"}) + private abstract class NonStaticInnerTestClass extends ParentTestClass { + + public NonStaticInnerTestClass(String svmType, String kernelType, int degree, double gamma, double coef0, double nu, + double cost, double eps, double p, double cacheSize, boolean shrinking, + boolean probability, boolean crossValidation, int nFold, String modelName, boolean debug) {} + + } + @SuppressWarnings({"UnusedDeclaration"}) private static class ParentTestClass { public void inheritedMethod(Map nothing) {}