Skip to content

Commit

Permalink
Use getParameterCount instead of getParameterTypes (#4821)
Browse files Browse the repository at this point in the history
* Use getParameterCount instead of getParameterTypes

Signed-off-by: Sergey Nuyanzin <[email protected]>

* Update changelog

Signed-off-by: Sergey Nuyanzin <[email protected]>

* Apply spotless

Signed-off-by: Sergey Nuyanzin <[email protected]>

* Address feedback

Signed-off-by: Sergey Nuyanzin <[email protected]>

* Address feedback

Signed-off-by: Sergey Nuyanzin <[email protected]>

Signed-off-by: Sergey Nuyanzin <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]>
  • Loading branch information
snuyanzin and dblock authored Nov 1, 2022
1 parent db418fb commit 763ef13
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added resource usage trackers for in-flight cancellation of SearchShardTask ([#4805](https://github.com/opensearch-project/OpenSearch/pull/4805))
- Renamed flaky tests ([#4912](https://github.com/opensearch-project/OpenSearch/pull/4912))
- Update previous release bwc version to 2.5.0 ([#5003](https://github.com/opensearch-project/OpenSearch/pull/5003))
- Use getParameterCount instead of getParameterTypes ([#4821](https://github.com/opensearch-project/OpenSearch/pull/4821))

### Dependencies
- Bumps `log4j-core` from 2.18.0 to 2.19.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public Collection<Method> getTestMethods(Class<?> suiteClass, ClassModel classMo
if (m.getName().startsWith("test")
&& Modifier.isPublic(m.getModifiers())
&& !Modifier.isStatic(m.getModifiers())
&& m.getParameterTypes().length == 0) {
&& m.getParameterCount() == 0) {
result.add(m);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1005,37 +1005,34 @@ private static void assertSyncMethod(Method method, String apiName, List<String>
}

assertEquals("incorrect number of exceptions for method [" + method + "]", 1, method.getExceptionTypes().length);
final Class<?>[] parameterTypes = method.getParameterTypes();
// a few methods don't accept a request object as argument
if (APIS_WITHOUT_REQUEST_OBJECT.contains(apiName)) {
assertEquals("incorrect number of arguments for method [" + method + "]", 1, method.getParameterTypes().length);
assertThat(
"the parameter to method [" + method + "] is the wrong type",
method.getParameterTypes()[0],
equalTo(RequestOptions.class)
);
assertEquals("incorrect number of arguments for method [" + method + "]", 1, method.getParameterCount());
assertThat("the parameter to method [" + method + "] is the wrong type", parameterTypes[0], equalTo(RequestOptions.class));
} else {
assertEquals("incorrect number of arguments for method [" + method + "]", 2, method.getParameterTypes().length);
assertEquals("incorrect number of arguments for method [" + method + "]", 2, method.getParameterCount());
// This is no longer true for all methods. Some methods can contain these 2 args backwards because of deprecation
if (method.getParameterTypes()[0].equals(RequestOptions.class)) {
if (parameterTypes[0].equals(RequestOptions.class)) {
assertThat(
"the first parameter to method [" + method + "] is the wrong type",
method.getParameterTypes()[0],
parameterTypes[0],
equalTo(RequestOptions.class)
);
assertThat(
"the second parameter to method [" + method + "] is the wrong type",
method.getParameterTypes()[1].getSimpleName(),
parameterTypes[1].getSimpleName(),
endsWith("Request")
);
} else {
assertThat(
"the first parameter to method [" + method + "] is the wrong type",
method.getParameterTypes()[0].getSimpleName(),
parameterTypes[0].getSimpleName(),
endsWith("Request")
);
assertThat(
"the second parameter to method [" + method + "] is the wrong type",
method.getParameterTypes()[1],
parameterTypes[1],
equalTo(RequestOptions.class)
);
}
Expand All @@ -1049,39 +1046,40 @@ private static void assertAsyncMethod(Map<String, Set<Method>> methods, Method m
);
assertThat("async method [" + method + "] should return Cancellable", method.getReturnType(), equalTo(Cancellable.class));
assertEquals("async method [" + method + "] should not throw any exceptions", 0, method.getExceptionTypes().length);
final Class<?>[] parameterTypes = method.getParameterTypes();
if (APIS_WITHOUT_REQUEST_OBJECT.contains(apiName.replaceAll("_async$", ""))) {
assertEquals(2, method.getParameterTypes().length);
assertThat(method.getParameterTypes()[0], equalTo(RequestOptions.class));
assertThat(method.getParameterTypes()[1], equalTo(ActionListener.class));
assertEquals(2, parameterTypes.length);
assertThat(parameterTypes[0], equalTo(RequestOptions.class));
assertThat(parameterTypes[1], equalTo(ActionListener.class));
} else {
assertEquals("async method [" + method + "] has the wrong number of arguments", 3, method.getParameterTypes().length);
assertEquals("async method [" + method + "] has the wrong number of arguments", 3, method.getParameterCount());
// This is no longer true for all methods. Some methods can contain these 2 args backwards because of deprecation
if (method.getParameterTypes()[0].equals(RequestOptions.class)) {
if (parameterTypes[0].equals(RequestOptions.class)) {
assertThat(
"the first parameter to async method [" + method + "] should be a request type",
method.getParameterTypes()[0],
parameterTypes[0],
equalTo(RequestOptions.class)
);
assertThat(
"the second parameter to async method [" + method + "] is the wrong type",
method.getParameterTypes()[1].getSimpleName(),
parameterTypes[1].getSimpleName(),
endsWith("Request")
);
} else {
assertThat(
"the first parameter to async method [" + method + "] should be a request type",
method.getParameterTypes()[0].getSimpleName(),
parameterTypes[0].getSimpleName(),
endsWith("Request")
);
assertThat(
"the second parameter to async method [" + method + "] is the wrong type",
method.getParameterTypes()[1],
parameterTypes[1],
equalTo(RequestOptions.class)
);
}
assertThat(
"the third parameter to async method [" + method + "] is the wrong type",
method.getParameterTypes()[2],
parameterTypes[2],
equalTo(ActionListener.class)
);
}
Expand All @@ -1094,16 +1092,17 @@ private static void assertSubmitTaskMethod(
ClientYamlSuiteRestSpec restSpec
) {
String methodName = extractMethodName(apiName);
final Class<?>[] parameterTypes = method.getParameterTypes();
assertTrue("submit task method [" + method.getName() + "] doesn't have corresponding sync method", methods.containsKey(methodName));
assertEquals("submit task method [" + method + "] has the wrong number of arguments", 2, method.getParameterTypes().length);
assertEquals("submit task method [" + method + "] has the wrong number of arguments", 2, method.getParameterCount());
assertThat(
"the first parameter to submit task method [" + method + "] is the wrong type",
method.getParameterTypes()[0].getSimpleName(),
parameterTypes[0].getSimpleName(),
endsWith("Request")
);
assertThat(
"the second parameter to submit task method [" + method + "] is the wrong type",
method.getParameterTypes()[1],
parameterTypes[1],
equalTo(RequestOptions.class)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,9 @@ private static void addFactoryMethod(Map<String, Class<?>> additionalClasses, Cl
}

additionalClasses.put(factoryClass.getName(), factoryClass);
for (int i = 0; i < factoryMethod.getParameterTypes().length; ++i) {
Class<?> parameterClazz = factoryMethod.getParameterTypes()[i];
final Class<?>[] parameterTypes = factoryMethod.getParameterTypes();
for (int i = 0; i < parameterTypes.length; ++i) {
Class<?> parameterClazz = parameterTypes[i];
additionalClasses.put(parameterClazz.getName(), parameterClazz);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,12 @@ private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context
}
}

for (int count = 0; count < newFactory.getParameterTypes().length; ++count) {
final Class<?>[] parameterTypes = newFactory.getParameterTypes();
for (int count = 0; count < parameterTypes.length; ++count) {
writer.visitField(
Opcodes.ACC_PRIVATE | Opcodes.ACC_FINAL,
"$arg" + count,
Type.getType(newFactory.getParameterTypes()[count]).getDescriptor(),
Type.getType(parameterTypes[count]).getDescriptor(),
null,
null
).visitEnd();
Expand All @@ -211,7 +212,7 @@ private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context
);
org.objectweb.asm.commons.Method init = new org.objectweb.asm.commons.Method(
"<init>",
MethodType.methodType(void.class, newFactory.getParameterTypes()).toMethodDescriptorString()
MethodType.methodType(void.class, parameterTypes).toMethodDescriptorString()
);

GeneratorAdapter constructor = new GeneratorAdapter(
Expand All @@ -223,10 +224,10 @@ private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context
constructor.loadThis();
constructor.invokeConstructor(OBJECT_TYPE, base);

for (int count = 0; count < newFactory.getParameterTypes().length; ++count) {
for (int count = 0; count < parameterTypes.length; ++count) {
constructor.loadThis();
constructor.loadArg(count);
constructor.putField(Type.getType("L" + className + ";"), "$arg" + count, Type.getType(newFactory.getParameterTypes()[count]));
constructor.putField(Type.getType("L" + className + ";"), "$arg" + count, Type.getType(parameterTypes[count]));
}

constructor.returnValue();
Expand All @@ -247,7 +248,7 @@ private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context
MethodType.methodType(newInstance.getReturnType(), newInstance.getParameterTypes()).toMethodDescriptorString()
);

List<Class<?>> parameters = new ArrayList<>(Arrays.asList(newFactory.getParameterTypes()));
List<Class<?>> parameters = new ArrayList<>(Arrays.asList(parameterTypes));
parameters.addAll(Arrays.asList(newInstance.getParameterTypes()));

org.objectweb.asm.commons.Method constru = new org.objectweb.asm.commons.Method(
Expand All @@ -264,9 +265,9 @@ private <T> Type generateStatefulFactory(Loader loader, ScriptContext<T> context
adapter.newInstance(WriterConstants.CLASS_TYPE);
adapter.dup();

for (int count = 0; count < newFactory.getParameterTypes().length; ++count) {
for (int count = 0; count < parameterTypes.length; ++count) {
adapter.loadThis();
adapter.getField(Type.getType("L" + className + ";"), "$arg" + count, Type.getType(newFactory.getParameterTypes()[count]));
adapter.getField(Type.getType("L" + className + ";"), "$arg" + count, Type.getType(parameterTypes[count]));
}

adapter.loadArgs();
Expand Down Expand Up @@ -334,13 +335,14 @@ private <T> T generateFactory(Loader loader, ScriptContext<T> context, Type clas
}
}

final Class<?>[] parameterTypes = reflect.getParameterTypes();
org.objectweb.asm.commons.Method instance = new org.objectweb.asm.commons.Method(
reflect.getName(),
MethodType.methodType(reflect.getReturnType(), reflect.getParameterTypes()).toMethodDescriptorString()
MethodType.methodType(reflect.getReturnType(), parameterTypes).toMethodDescriptorString()
);
org.objectweb.asm.commons.Method constru = new org.objectweb.asm.commons.Method(
"<init>",
MethodType.methodType(void.class, reflect.getParameterTypes()).toMethodDescriptorString()
MethodType.methodType(void.class, parameterTypes).toMethodDescriptorString()
);

GeneratorAdapter adapter = new GeneratorAdapter(
Expand Down Expand Up @@ -421,9 +423,7 @@ private <T> T generateFactory(Loader loader, ScriptContext<T> context, Type clas

private void writeNeedsMethods(Class<?> clazz, ClassWriter writer, Set<String> extractedVariables) {
for (Method method : clazz.getMethods()) {
if (method.getName().startsWith("needs")
&& method.getReturnType().equals(boolean.class)
&& method.getParameterTypes().length == 0) {
if (method.getName().startsWith("needs") && method.getReturnType().equals(boolean.class) && method.getParameterCount() == 0) {
String name = method.getName();
name = name.substring(5);
name = Character.toLowerCase(name.charAt(0)) + name.substring(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public ScriptClassInfo(PainlessLookup painlessLookup, Class<?> baseClass) {
+ "] has more than one."
);
}
} else if (m.getName().startsWith("needs") && m.getReturnType() == boolean.class && m.getParameterTypes().length == 0) {
} else if (m.getName().startsWith("needs") && m.getReturnType() == boolean.class && m.getParameterCount() == 0) {
needsMethods.add(new org.objectweb.asm.commons.Method(m.getName(), NEEDS_PARAMETER_METHOD_TYPE.toMethodDescriptorString()));
} else if (m.getName().startsWith("get")
&& m.getName().equals("getClass") == false
Expand Down Expand Up @@ -124,7 +124,7 @@ public ScriptClassInfo(PainlessLookup painlessLookup, Class<?> baseClass) {
FunctionTable.LocalFunction defConverter = null;
for (java.lang.reflect.Method m : baseClass.getMethods()) {
if (m.getName().startsWith("convertFrom")
&& m.getParameterTypes().length == 1
&& m.getParameterCount() == 1
&& m.getReturnType() == returnType
&& Modifier.isStatic(m.getModifiers())) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2168,9 +2168,10 @@ private void generateBridgeMethod(PainlessClassBuilder painlessClassBuilder, Pai
bridgeMethodWriter.loadArg(0);
}

for (int typeParameterCount = 0; typeParameterCount < javaMethod.getParameterTypes().length; ++typeParameterCount) {
final Class<?>[] typeParameters = javaMethod.getParameterTypes();
for (int typeParameterCount = 0; typeParameterCount < typeParameters.length; ++typeParameterCount) {
bridgeMethodWriter.loadArg(typeParameterCount + bridgeTypeParameterOffset);
Class<?> typeParameter = javaMethod.getParameterTypes()[typeParameterCount];
Class<?> typeParameter = typeParameters[typeParameterCount];

if (typeParameter == Byte.class) bridgeMethodWriter.invokeStatic(DEF_UTIL_TYPE, DEF_TO_B_BYTE_IMPLICIT);
else if (typeParameter == Short.class) bridgeMethodWriter.invokeStatic(DEF_UTIL_TYPE, DEF_TO_B_SHORT_IMPLICIT);
Expand Down
Loading

0 comments on commit 763ef13

Please sign in to comment.