Skip to content

Commit

Permalink
Merge pull request #41556 from gabilang/fix-overloaded-interop-calls
Browse files Browse the repository at this point in the history 
Fix overloaded method interop calls when parameter types are not specified
  • Loading branch information
@warunalakshitha
warunalakshitha authored Dec 20, 2023
2 parents 122d089 + 238d921 commit 0716102
Show file tree
Hide file tree
Showing 10 changed files with 290 additions and 77 deletions.
113 changes: 62 additions & 51 deletions ...ng/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.StringJoiner;

Expand Down Expand Up @@ -83,8 +81,8 @@
*/
class JMethodResolver {

private ClassLoader classLoader;
private SymbolTable symbolTable;
private final ClassLoader classLoader;
private final SymbolTable symbolTable;
private final BType[] definedReadOnlyMemberTypes;

JMethodResolver(ClassLoader classLoader, SymbolTable symbolTable) {
Expand Down Expand Up @@ -224,24 +222,10 @@ private JMethod resolve(JMethodRequest jMethodRequest, List<JMethod> jMethods) {
if (jMethods.size() == 1 && noConstraints) {
return jMethods.get(0);
} else if (noConstraints) {
Optional<JMethod> covariantRetTypeMethod = findCovariantReturnTypeMethod(jMethods);
if (covariantRetTypeMethod.isPresent()) {
return covariantRetTypeMethod.get();
}

int paramCount = jMethods.get(0).getParamTypes().length;
if (jMethodRequest.kind == JMethodKind.CONSTRUCTOR) {
throw new JInteropException(OVERLOADED_METHODS,
"Overloaded constructors with '" + paramCount + "' parameter(s) in class '" +
jMethodRequest.declaringClass + "', please specify class names for each parameter " +
"in 'paramTypes' field in the annotation");
} else {
throw new JInteropException(OVERLOADED_METHODS,
"Overloaded methods '" + jMethodRequest.methodName + "' with '" + paramCount +
"' parameter(s) in class '" + jMethodRequest.declaringClass +
"', please specify class names for each parameter " +
"with 'paramTypes' field in the annotation");
if (areAllMethodsOverridden(jMethods, jMethodRequest.declaringClass)) {
return jMethods.get(0);
}
throwOverloadedMethodError(jMethodRequest, jMethods.get(0).getParamTypes().length);
}

JMethod jMethod = resolveExactMethod(jMethodRequest.declaringClass, jMethodRequest.methodName,
Expand All @@ -252,28 +236,51 @@ private JMethod resolve(JMethodRequest jMethodRequest, List<JMethod> jMethods) {
return jMethod;
}

private Optional<JMethod> findCovariantReturnTypeMethod(List<JMethod> jMethods) {
private boolean areAllMethodsOverridden(List<JMethod> jMethods, Class<?> clazz) {
if (jMethods.get(0).getKind() == JMethodKind.CONSTRUCTOR) {
return false;
}
for (int i = 0; i < jMethods.size(); i++) {
Method method1 = (Method) jMethods.get(i).getMethod();
for (int k = i + 1; k < jMethods.size(); k++) {
JMethod ithMethod = jMethods.get(i);
JMethod kthMethod = jMethods.get(k);

if (ithMethod.getReturnType().isAssignableFrom(kthMethod.getReturnType()) ||
kthMethod.getReturnType().isAssignableFrom(ithMethod.getReturnType())) {
if (ithMethod.getParamTypes().length != kthMethod.getParamTypes().length) {
// This occurs when there are static methods and instance methods and the static method
// has one more parameter than the instance method. Also this occurs when an interop
// method in an object maps to instance methods of which one accepting self and another
// that doesn't.
throw new JInteropException(
OVERLOADED_METHODS, "Overloaded methods cannot be differentiated. Please specify the " +
"parameterTypes for each parameter in 'paramTypes' field in the annotation");
}
return Optional.of(ithMethod);
Method method2 = (Method) jMethods.get(k).getMethod();
if (!isOverridden(method1, method2, clazz)) {
return false;
}
}
}
return Optional.empty();
return true;
}

private boolean isOverridden(Method method1, Method method2, Class<?> clazz) {
if (method1.getParameterCount() != method2.getParameterCount()) {
// This occurs when there are static methods and instance methods, and the static method has one more
// parameter than the instance method. Additionally, this occurs when an interop method in an object
// maps to instance methods, one accepting `self` and another that doesn't.
throw new JInteropException(OVERLOADED_METHODS, "Overloaded methods cannot be differentiated. " +
"Please specify the parameter types for each parameter in 'paramTypes' field in the annotation");
}
// Returns false if return types are not covariant
Method currentMethod;
Method otherMethod;
if (method2.getReturnType().isAssignableFrom(method1.getReturnType())) {
currentMethod = method1;
otherMethod = method2;
} else if (method1.getReturnType().isAssignableFrom(method2.getReturnType())) {
currentMethod = method2;
otherMethod = method1;
} else {
return false;
}

try {
Method superMethod = clazz.getSuperclass()
.getDeclaredMethod(currentMethod.getName(), currentMethod.getParameterTypes());
return Arrays.equals(superMethod.getParameterTypes(), otherMethod.getParameterTypes()) &&
superMethod.getReturnType().equals(otherMethod.getReturnType());
} catch (NoSuchMethodException e) {
return false;
}
}

private void validateMethodSignature(JMethodRequest jMethodRequest, JMethod jMethod) {
Expand Down Expand Up @@ -317,8 +324,7 @@ private void validateExceptionTypes(JMethodRequest jMethodRequest, JMethod jMeth
"': expected '" + expectedRetTypeName + "', found '" + returnType + "'");
} else if (jMethodRequest.returnsBErrorType && !throwsCheckedException && !returnsErrorValue) {
String errorMsgPart;
if (returnType instanceof BUnionType) {
BUnionType bUnionReturnType = (BUnionType) returnType;
if (returnType instanceof BUnionType bUnionReturnType) {
BType modifiedRetType = BUnionType.create(null, getNonErrorMembers(bUnionReturnType));
errorMsgPart = "expected '" + modifiedRetType + "', found '" + returnType + "'";
} else {
Expand All @@ -335,8 +341,7 @@ private String getExpectedReturnType(BType retType) {
if (retType.tag == TypeTags.NIL || (retType instanceof BTypeReferenceType &&
((BTypeReferenceType) retType).referredType.tag == TypeTags.ERROR)) {
return "error";
} else if (retType instanceof BUnionType) {
BUnionType bUnionReturnType = (BUnionType) retType;
} else if (retType instanceof BUnionType bUnionReturnType) {
BType modifiedRetType = BUnionType.create(null, getNonErrorMembers(bUnionReturnType));
return modifiedRetType + "|error";
} else {
Expand Down Expand Up @@ -520,6 +525,7 @@ private boolean isValidParamBType(Class<?> jType, BType bType, boolean isLastPar
case TypeTags.RECORD:
return this.classLoader.loadClass(BMap.class.getCanonicalName()).isAssignableFrom(jType);
case TypeTags.JSON:
case TypeTags.READONLY:
return jTypeName.equals(J_OBJECT_TNAME);
case TypeTags.OBJECT:
return this.classLoader.loadClass(BObject.class.getCanonicalName()).isAssignableFrom(jType);
Expand Down Expand Up @@ -547,16 +553,13 @@ private boolean isValidParamBType(Class<?> jType, BType bType, boolean isLastPar
}
}
return true;
case TypeTags.READONLY:
return jTypeName.equals(J_OBJECT_TNAME);
case TypeTags.FINITE:
if (jTypeName.equals(J_OBJECT_TNAME)) {
return true;
}

Set<BLangExpression> valueSpace = ((BFiniteType) bType).getValueSpace();
for (Iterator<BLangExpression> iterator = valueSpace.iterator(); iterator.hasNext(); ) {
BLangExpression value = iterator.next();
for (BLangExpression value : valueSpace) {
if (!isValidParamBType(jType, value.getBType(), isLastParam, restParamExist)) {
return false;
}
Expand Down Expand Up @@ -669,11 +672,7 @@ private boolean isValidReturnBType(Class<?> jType, BType bType, JMethodRequest j
return true;
}

if (isValidReturnBType(jType, symbolTable.jsonType, jMethodRequest, visitedSet)) {
return true;
}

return false;
return isValidReturnBType(jType, symbolTable.jsonType, jMethodRequest, visitedSet);
case TypeTags.OBJECT:
return this.classLoader.loadClass(BObject.class.getCanonicalName()).isAssignableFrom(jType);
case TypeTags.ERROR:
Expand Down Expand Up @@ -1007,4 +1006,16 @@ private void throwParamCountMismatchError(JMethodRequest jMethodRequest) throws
"Parameter count does not match with Java method '" + jMethodRequest.methodName +
"' found in class '" + jMethodRequest.declaringClass.getName() + "'");
}

private void throwOverloadedMethodError(JMethodRequest jMethodRequest, int paramCount) throws JInteropException {
if (jMethodRequest.kind == JMethodKind.CONSTRUCTOR) {
throw new JInteropException(OVERLOADED_METHODS, "Overloaded constructors with '" + paramCount +
"' parameter(s) in class '" + jMethodRequest.declaringClass.getName() +
"', please specify the parameter types for each parameter in 'paramTypes' field in the annotation");
} else {
throw new JInteropException(OVERLOADED_METHODS, "Overloaded methods '" + jMethodRequest.methodName +
"' with '" + paramCount + "' parameter(s) in class '" + jMethodRequest.declaringClass.getName() +
"', please specify the parameter types for each parameter in 'paramTypes' field in the annotation");
}
}
}
56 changes: 42 additions & 14 deletions ...t-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,34 +401,62 @@ public void testResolveWithInstanceAndStatic() {
String path = "test-src/javainterop/negative/method_resolve_error.bal";
CompileResult compileResult = BCompileUtil.compile(path);
compileResult.getDiagnostics();
Assert.assertEquals(compileResult.getDiagnostics().length, 2);
Assert.assertEquals(compileResult.getDiagnostics().length, 8);
BAssertUtil.validateError(compileResult, 0,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " +
"cannot be differentiated. Please specify the parameterTypes for each " +
"cannot be differentiated. Please specify the parameter types for each " +
"parameter in 'paramTypes' field in the annotation'",
"method_resolve_error.bal", 19, 1);
"method_resolve_error.bal", 19, 1);
BAssertUtil.validateError(compileResult, 1,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " +
"cannot be differentiated. Please specify the parameterTypes for each " +
"cannot be differentiated. Please specify the parameter types for each " +
"parameter in 'paramTypes' field in the annotation'",
"method_resolve_error.bal", 24, 1);

"method_resolve_error.bal", 24, 1);
BAssertUtil.validateError(compileResult, 2,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods cannot be " +
"differentiated. Please specify the parameter types for each parameter in 'paramTypes' " +
"field in the annotation'", "method_resolve_error.bal", 29, 1);
BAssertUtil.validateError(compileResult, 3,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods cannot be " +
"differentiated. Please specify the parameter types for each parameter in 'paramTypes' " +
"field in the annotation'", "method_resolve_error.bal", 33, 1);
BAssertUtil.validateError(compileResult, 4,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getDescription' " +
"with '1' parameter(s) in class " +
"'org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', please specify the " +
"parameter types for each parameter in 'paramTypes' field in the annotation'",
"method_resolve_error.bal", 38, 1);
BAssertUtil.validateError(compileResult, 5,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded constructors with '1' " +
"parameter(s) in class 'java.lang.Byte', please specify the parameter types for each " +
"parameter in 'paramTypes' field in the annotation'",
"method_resolve_error.bal", 42, 1);
BAssertUtil.validateError(compileResult, 6,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " +
"'getCategorization' with '2' parameter(s) in class " +
"'org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', please specify the " +
"parameter types for each parameter in 'paramTypes' field in the annotation'",
"method_resolve_error.bal", 46, 1);
}

@Test
public void testOverloadedMethods() {
String path = "test-src/javainterop/negative/overloaded_methods.bal";
CompileResult compileResult = BCompileUtil.compile(path);
Assert.assertEquals(compileResult.getDiagnostics().length, 2);
Assert.assertEquals(compileResult.getDiagnostics().length, 3);
BAssertUtil.validateError(compileResult, 0,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " +
"cannot be differentiated. Please specify the parameterTypes for each " +
"parameter in 'paramTypes' field in the annotation'",
"overloaded_methods.bal", 24, 5);
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods cannot be " +
"differentiated. Please specify the parameter types for each parameter in 'paramTypes' field " +
"in the annotation'", "overloaded_methods.bal", 24, 5);
BAssertUtil.validateError(compileResult, 1,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " +
"cannot be differentiated. Please specify the parameterTypes for each parameter in " +
"'paramTypes' field in the annotation'", "overloaded_methods.bal", 30, 1);
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods cannot be " +
"differentiated. Please specify the parameter types for each parameter in 'paramTypes' field " +
"in the annotation'", "overloaded_methods.bal", 30, 1);
BAssertUtil.validateError(compileResult, 2,
"{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getPrice' with '1' " +
"parameter(s) in class 'org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " +
"please specify the parameter types for each parameter in 'paramTypes' " +
"field in the annotation'", "overloaded_methods.bal", 35, 1);
}

@Test
Expand Down
8 changes: 1 addition & 7 deletions ...t/src/test/java/org/ballerinalang/test/javainterop/overloading/OverriddenMethodTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
*/
package org.ballerinalang.test.javainterop.overloading;

import io.ballerina.runtime.api.values.BArray;
import org.ballerinalang.test.BCompileUtil;
import org.ballerinalang.test.BRunUtil;
import org.ballerinalang.test.CompileResult;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
Expand All @@ -41,11 +39,7 @@ public void setup() {

@Test(description = "Test invoking an overridden and overloaded java method")
public void testOverriddenMethods() {
Object val = BRunUtil.invoke(result, "testOverriddenMethods");
BArray returns = (BArray) val;
Assert.assertEquals(returns.size(), 2);
Assert.assertEquals(returns.get(0).toString(), "Motor Car : Honda");
Assert.assertEquals(returns.get(1).toString(), "GrazeHonda");
BRunUtil.invoke(result, "testOverriddenMethods");
}

@AfterClass
Expand Down
25 changes: 24 additions & 1 deletion ...erina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/
public class Car extends Vehicle {

private String model;
private final String model;

public Car(String name, String model) {

Expand All @@ -41,4 +41,27 @@ public String getDescription(String prefix) {

return prefix + this.model;
}

public long getSeatCount() {
return 4;
}

public static String getMaxSpeed(String model) {
if (model.equals("BMW")) {
return "200MPH";
}
return "160MPH";
}

public Object[] getCategorization(int num, String category) {
return new Object[] {num, category};
}

public Object getBatteryType(long prefix) {
return prefix + " EVL-HP-LiPo powered";
}

public static String getMillage(long val) {
return val + "MPG";
}
}
Loading

0 comments on commit 0716102

Please sign in to comment.