From a9443261318de1326c091c3ef81842ada1e7bfd6 Mon Sep 17 00:00:00 2001 From: Michael Hoisie Date: Tue, 26 Mar 2024 13:13:14 -0700 Subject: [PATCH] Update ShadowWrangler to iterate through methods using getDeclaredMethods Previously, in ShadowWrangler, shadow method lookup was performed using ShadowClass.findDeclaredMethod. It was called once to look for an exact match of a shadow method, and sometimes called again to check for a looseSignatures match. There are plans to add new features and capabilities to the way that shadow methods are matched. For example: * looseSignatures being replaced with a more minimal @ClassName("internal.type") annotation. * If the signature of a method changes across SDK levels, we could introduce different method names that map to the same method name. However, to search for methods that cannot be matched using ShadowClass.findDeclaredMethod, it is required to iterate over all candidate methods using ShadowClass.findDeclaredMethods. There were some questions about the performance of using ShadowClass.findDeclaredMethods + iteration. However, after some preliminary benchmarks, this approach is surprisingly approximately 25% faster than using ShadowClass.findDeclaredMethod. It is perhaps due to the internal caching of ShadowClass.findDeclaredMethods. With this change, it will be possible to perform more advanced filtering and searching for methods. For #8841 PiperOrigin-RevId: 619288002 --- .../internal/bytecode/ShadowWrangler.java | 74 ++++++++++--------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/sandbox/src/main/java/org/robolectric/internal/bytecode/ShadowWrangler.java b/sandbox/src/main/java/org/robolectric/internal/bytecode/ShadowWrangler.java index 1c6d8c19c5d..ab480d69a0e 100644 --- a/sandbox/src/main/java/org/robolectric/internal/bytecode/ShadowWrangler.java +++ b/sandbox/src/main/java/org/robolectric/internal/bytecode/ShadowWrangler.java @@ -18,6 +18,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.annotation.Nonnull; import javax.annotation.Priority; @@ -305,13 +306,8 @@ private Method findShadowMethod( Class[] types, ShadowInfo shadowInfo, Class shadowClass) { - Method method = findShadowMethodDeclaredOnClass(shadowClass, name, types); - - if (method == null && shadowInfo.looseSignatures) { - Class[] genericTypes = MethodType.genericMethodType(types.length).parameterArray(); - method = findShadowMethodDeclaredOnClass(shadowClass, name, genericTypes); - } - + Method method = + findShadowMethodDeclaredOnClass(shadowClass, name, types, shadowInfo.looseSignatures); if (method != null) { return method; } else { @@ -334,41 +330,49 @@ private Method findShadowMethod( } private Method findShadowMethodDeclaredOnClass( - Class shadowClass, String methodName, Class[] paramClasses) { - try { - Method method = shadowClass.getDeclaredMethod(methodName, paramClasses); - - // todo: allow per-version overloading - // if (method == null) { - // String methodPrefix = name + "$$"; - // for (Method candidateMethod : shadowClass.getDeclaredMethods()) { - // if (candidateMethod.getName().startsWith(methodPrefix)) { - // - // } - // } - // } - - if (isValidShadowMethod(method)) { - method.setAccessible(true); - return method; - } else { - return null; + Class shadowClass, String methodName, Class[] paramClasses, boolean looseSignatures) { + Method foundMethod = null; + for (Method method : shadowClass.getDeclaredMethods()) { + if (!method.getName().equals(methodName) + || method.getParameterCount() != paramClasses.length) { + continue; } - } catch (NoSuchMethodException e) { - return null; - } - } + if (!Modifier.isPublic(method.getModifiers()) + && !Modifier.isProtected(method.getModifiers())) { + continue; + } - private boolean isValidShadowMethod(Method method) { - int modifiers = method.getModifiers(); - if (!Modifier.isPublic(modifiers) && !Modifier.isProtected(modifiers)) { - return false; + if (Arrays.equals(method.getParameterTypes(), paramClasses) + && shadowMatcher.matches(method)) { + // Found an exact match, we can exit early. + foundMethod = method; + break; + } + if (looseSignatures) { + boolean allParameterTypesAreObject = true; + for (Class paramClass : method.getParameterTypes()) { + if (!paramClass.equals(Object.class)) { + allParameterTypesAreObject = false; + break; + } + } + if (allParameterTypesAreObject && shadowMatcher.matches(method)) { + // Found a looseSignatures match, but continue looking for an exact match. + foundMethod = method; + } + } } - return shadowMatcher.matches(method); + if (foundMethod != null) { + foundMethod.setAccessible(true); + return foundMethod; + } else { + return null; + } } + @Override public Object intercept(String signature, Object instance, Object[] params, Class theClass) throws Throwable {