From d6b01db6b1a7ea10d481bd6baf77e26a427436e9 Mon Sep 17 00:00:00 2001 From: gabilang Date: Mon, 23 Oct 2023 10:29:17 +0530 Subject: [PATCH 01/12] Fix overloaded method cases --- .../bir/codegen/interop/JMethodResolver.java | 67 +++++++++++++------ .../basic/instance_method_tests.bal | 3 +- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index 3fb27f038d47..64b1f771a9b1 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -47,12 +47,12 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Executable; import java.lang.reflect.Method; +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; @@ -223,9 +223,8 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { if (jMethods.size() == 1 && noConstraints) { return jMethods.get(0); } else if (noConstraints) { - Optional covariantRetTypeMethod = findCovariantReturnTypeMethod(jMethods); - if (covariantRetTypeMethod.isPresent()) { - return covariantRetTypeMethod.get(); + if (areAllMethodsOverridden(jMethods)) { + return jMethods.get(0); } int paramCount = jMethods.get(0).getParamTypes().length; @@ -251,28 +250,54 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { return jMethod; } - private Optional findCovariantReturnTypeMethod(List jMethods) { + private boolean areAllMethodsOverridden(List jMethods) { + if (jMethods.get(0).getKind() == JMethodKind.CONSTRUCTOR) { + return false; + } for (int i = 0; i < jMethods.size(); i++) { 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 ithMethod = (Method) jMethods.get(i).getMethod(); + Method kthMethod = (Method) jMethods.get(k).getMethod(); + if (!isOverridden(ithMethod, kthMethod, ithMethod.getDeclaringClass())) { + return false; } } } - return Optional.empty(); + return true; + } + + private boolean isOverridden(Method method1, Method method2, Class clazz) { + // Static methods cannot be overridden + if (Modifier.isStatic(method1.getModifiers()) || Modifier.isStatic(method2.getModifiers()) || + 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. 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 false if returns 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) { diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/basic/instance_method_tests.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/basic/instance_method_tests.bal index 47f8269d41c8..60707dc25fb2 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/basic/instance_method_tests.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/basic/instance_method_tests.bal @@ -235,7 +235,8 @@ function hashCode(handle receiver) returns int = @java:Method { } external; function newByte(int val) returns handle = @java:Constructor { - 'class: "java.lang.Byte" + 'class: "java.lang.Byte", + paramTypes: ["byte"] } external; function getStringFromFutureResult(handle receiver) returns string = @java:Method { From 1568a7ff536e0e554733ec398f50609053682335 Mon Sep 17 00:00:00 2001 From: gabilang Date: Mon, 23 Oct 2023 13:51:16 +0530 Subject: [PATCH 02/12] Add tests for overloaded method cases --- .../basic/NegativeValidationTest.java | 24 ++++++++- .../overloading/OverriddenMethodTests.java | 8 +-- .../test/javainterop/overloading/pkg/Car.java | 6 ++- .../overloading/pkg/SportsCar.java | 50 +++++++++++++++++++ .../javainterop/overloading/pkg/Vehicle.java | 2 +- .../negative/method_resolve_error.bal | 16 ++++++ .../overloading/overridden_method_test.bal | 27 +++++++++- 7 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java index c73e6c70a6bf..782cc91c7b98 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java @@ -407,7 +407,7 @@ 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, 6); BAssertUtil.validateError(compileResult, 0, "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " + "cannot be differentiated. Please specify the parameterTypes for each " + @@ -418,7 +418,27 @@ public void testResolveWithInstanceAndStatic() { "cannot be differentiated. Please specify the parameterTypes for each " + "parameter in 'paramTypes' field in the annotation'", "method_resolve_error.bal", 24, 1); - + BAssertUtil.validateError(compileResult, 2, + "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " + + "cannot be differentiated. Please specify the parameterTypes 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 parameterTypes 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 " + + "'class org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " + + "please specify class names for each parameter with '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 'class java.lang.Byte', please specify class names for each " + + "parameter in 'paramTypes' field in the annotation'", + "method_resolve_error.bal", 42, 1); } @Test diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/OverriddenMethodTests.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/OverriddenMethodTests.java index 0d235d3b3250..fc4d3d6729cb 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/OverriddenMethodTests.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/OverriddenMethodTests.java @@ -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; @@ -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 diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java index 8f2424736749..0b07bcd35604 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java @@ -22,7 +22,7 @@ */ public class Car extends Vehicle { - private String model; + private final String model; public Car(String name, String model) { @@ -41,4 +41,8 @@ public String getDescription(String prefix) { return prefix + this.model; } + + public long getSeatCount() { + return 4; + } } diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java new file mode 100644 index 000000000000..9e2a80869e8b --- /dev/null +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. 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.ballerinalang.test.javainterop.overloading.pkg; + +public class SportsCar extends Car { + + private final long seatCount; + + public SportsCar(String name, String model, long seatCount) { + + super(name, model); + this.seatCount = seatCount; + } + + @Override + public String getDescription(String prefix) { + return prefix + " seat count: " + seatCount; + } + + public long getSeatCount() { + return seatCount; + } + + public String getDescription(long[] numProps) { + return getName() + "| no of seats: " + seatCount + "| max speed: " + numProps[0] + "| mileage: " + numProps[1]; + } + + public static long getSeatCount(String model) { + if (model.equals("Nissan-GTR")) { + return 4; + } + return 2; + } +} diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Vehicle.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Vehicle.java index 76118c18f6d4..fce097d4ff9d 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Vehicle.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Vehicle.java @@ -24,7 +24,7 @@ */ public class Vehicle { - private String name; + private final String name; public Vehicle(String name) { diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal index 8f2a601633d9..27bba65069ec 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal @@ -26,3 +26,19 @@ function hashCodeStatic(int receiver) returns int = @java:Method { 'class: "java.lang.Byte" } external; +function getSeatCount(handle receiver) returns int = @java:Method { + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + +function getSeatCountWithModel(handle receiver) returns int = @java:Method { + name: "getSeatCount", + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + +function getDescription(handle receiver, handle val) returns handle = @java:Method { + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + +function newByte(int val) returns handle = @java:Constructor { + 'class: "java.lang.Byte" +} external; diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal index c6833bc3592f..ba0de2c6f0b5 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal @@ -1,4 +1,5 @@ import ballerina/jballerina.java; +import ballerina/test; function newVehicle(handle strName) returns handle = @java:Constructor { 'class:"org.ballerinalang.test.javainterop.overloading.pkg.Vehicle", @@ -9,6 +10,10 @@ function newCar(handle strName, handle strModel) returns handle = @java:Construc 'class:"org.ballerinalang.test.javainterop.overloading.pkg.Car" } external; +function newSportsCar(handle strName, handle strModel, int seatCount) returns handle = @java:Constructor { + 'class:"org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + function getName(handle receiver) returns handle = @java:Method { 'class:"org.ballerinalang.test.javainterop.overloading.pkg.Car" @@ -18,8 +23,18 @@ function getDescription(handle receiver, handle inputStr) returns handle = @java 'class:"org.ballerinalang.test.javainterop.overloading.pkg.Car" } external; +function getSeatCount(handle receiver) returns int = @java:Method { + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar", + paramTypes: [] +} external; -public function testOverriddenMethods() returns [string?, string?] { +function getSeatCountWithModel(handle receiver) returns int = @java:Method { + name: "getSeatCount", + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar", + paramTypes: ["java.lang.String"] +} external; + +public function testOverriddenMethods() { handle strName1 = java:fromString("Generic vehicle"); handle vehicle = newVehicle(strName1); @@ -27,7 +42,15 @@ public function testOverriddenMethods() returns [string?, string?] { handle strModel = java:fromString("Honda"); handle car = newCar(strName2, strModel); + handle strName3 = java:fromString("Sports Car"); + handle strModel2 = java:fromString("Nissan-GTR"); + handle sportsCar = newSportsCar(strName3, strModel2, 4); + handle carName = getName(car); handle carDesc = getDescription(car, java:fromString("Graze")); - return [java:toString(carName), java:toString(carDesc)]; + + test:assertEquals(java:toString(carName), "Motor Car : Honda"); + test:assertEquals(java:toString(carDesc), "GrazeHonda"); + test:assertEquals(getSeatCount(sportsCar), 4); + test:assertEquals(getSeatCountWithModel(java:fromString("Mazda MX-5")), 2); } From e25bc8a757bd513f84a34ae679faec8cdb5dc450 Mon Sep 17 00:00:00 2001 From: gabilang Date: Tue, 24 Oct 2023 10:21:00 +0530 Subject: [PATCH 03/12] Use declaredClass from jMethodRequest --- .../compiler/bir/codegen/interop/JMethodResolver.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index 64b1f771a9b1..4cc6c1fba6e8 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -223,7 +223,7 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { if (jMethods.size() == 1 && noConstraints) { return jMethods.get(0); } else if (noConstraints) { - if (areAllMethodsOverridden(jMethods)) { + if (areAllMethodsOverridden(jMethods, jMethodRequest.declaringClass)) { return jMethods.get(0); } @@ -250,7 +250,7 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { return jMethod; } - private boolean areAllMethodsOverridden(List jMethods) { + private boolean areAllMethodsOverridden(List jMethods, Class declaringClass) { if (jMethods.get(0).getKind() == JMethodKind.CONSTRUCTOR) { return false; } @@ -258,7 +258,7 @@ private boolean areAllMethodsOverridden(List jMethods) { for (int k = i + 1; k < jMethods.size(); k++) { Method ithMethod = (Method) jMethods.get(i).getMethod(); Method kthMethod = (Method) jMethods.get(k).getMethod(); - if (!isOverridden(ithMethod, kthMethod, ithMethod.getDeclaringClass())) { + if (!isOverridden(ithMethod, kthMethod, declaringClass)) { return false; } } From a30c79246dcf0ca0ef59946aa22c5019e1b210c7 Mon Sep 17 00:00:00 2001 From: gabilang Date: Tue, 24 Oct 2023 12:19:33 +0530 Subject: [PATCH 04/12] Fix issues with static methods with same name and parameters --- .../bir/codegen/interop/JMethodResolver.java | 5 ++++- .../test/javainterop/overloading/pkg/Car.java | 7 +++++++ .../javainterop/overloading/pkg/SportsCar.java | 7 +++++++ .../overloading/overridden_method_test.bal | 15 +++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index 4cc6c1fba6e8..801e2300882d 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -268,7 +268,7 @@ private boolean areAllMethodsOverridden(List jMethods, Class declari private boolean isOverridden(Method method1, Method method2, Class clazz) { // Static methods cannot be overridden - if (Modifier.isStatic(method1.getModifiers()) || Modifier.isStatic(method2.getModifiers()) || + if ((Modifier.isStatic(method1.getModifiers()) ^ Modifier.isStatic(method2.getModifiers())) || 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. Also, this occurs when an interop method in an object maps to @@ -293,6 +293,9 @@ private boolean isOverridden(Method method1, Method method2, Class clazz) { try { Method superMethod = clazz.getSuperclass().getDeclaredMethod(currentMethod.getName(), currentMethod.getParameterTypes()); + if (Modifier.isStatic(currentMethod.getModifiers())) { + return superMethod.equals(otherMethod); + } return Arrays.equals(superMethod.getParameterTypes(), otherMethod.getParameterTypes()) && superMethod.getReturnType().equals(otherMethod.getReturnType()); } catch (NoSuchMethodException e) { diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java index 0b07bcd35604..a215650a6795 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java @@ -45,4 +45,11 @@ public String getDescription(String prefix) { public long getSeatCount() { return 4; } + + public static String getMaxSpeed(String model) { + if (model.equals("BMW")) { + return "200MPH"; + } + return "160MPH"; + } } diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java index 9e2a80869e8b..f6b7a5fdfcc2 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java @@ -47,4 +47,11 @@ public static long getSeatCount(String model) { } return 2; } + + public static String getMaxSpeed(String model) { + if (model.equals("Nissan-GTR")) { + return "300MPH"; + } + return "250MPH"; + } } diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal index ba0de2c6f0b5..14eae51c3c18 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal @@ -34,6 +34,16 @@ function getSeatCountWithModel(handle receiver) returns int = @java:Method { paramTypes: ["java.lang.String"] } external; +function getSportsCarMaxSpeed(handle model) returns handle = @java:Method { + name: "getMaxSpeed", + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + +function getCarMaxSpeed(handle model) returns handle = @java:Method { + name: "getMaxSpeed", + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.Car" +} external; + public function testOverriddenMethods() { handle strName1 = java:fromString("Generic vehicle"); handle vehicle = newVehicle(strName1); @@ -49,8 +59,13 @@ public function testOverriddenMethods() { handle carName = getName(car); handle carDesc = getDescription(car, java:fromString("Graze")); + handle sportsCarMaxSpeed = getSportsCarMaxSpeed(java:fromString("BMW-S6")); + handle carMaxSpeed = getCarMaxSpeed(java:fromString("BMW")); + test:assertEquals(java:toString(carName), "Motor Car : Honda"); test:assertEquals(java:toString(carDesc), "GrazeHonda"); test:assertEquals(getSeatCount(sportsCar), 4); test:assertEquals(getSeatCountWithModel(java:fromString("Mazda MX-5")), 2); + test:assertEquals(java:toString(sportsCarMaxSpeed), "250MPH"); + test:assertEquals(java:toString(carMaxSpeed), "200MPH"); } From 4616a8444ef188f8ddee3b8c061b40c5679cc308 Mon Sep 17 00:00:00 2001 From: gabilang Date: Mon, 30 Oct 2023 14:49:37 +0530 Subject: [PATCH 05/12] Address review suggestions --- .../bir/codegen/interop/JMethodResolver.java | 2 +- .../basic/NegativeValidationTest.java | 16 +++++++++++----- .../test/javainterop/overloading/pkg/Car.java | 12 ++++++++++++ .../javainterop/overloading/pkg/SportsCar.java | 12 ++++++++++++ .../negative/method_resolve_error.bal | 10 ++++++++++ .../overloading/overridden_method_test.bal | 5 +++++ 6 files changed, 51 insertions(+), 6 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index 801e2300882d..5ef2d923bfef 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -275,7 +275,7 @@ private boolean isOverridden(Method method1, Method method2, Class clazz) { // 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"); + "parameter types for each parameter in 'paramTypes' field in the annotation"); } // Return false if returns types are not covariant Method currentMethod; diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java index 782cc91c7b98..7ce58b069143 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java @@ -407,25 +407,25 @@ 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, 6); + 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); 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); BAssertUtil.validateError(compileResult, 2, "{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", 29, 1); BAssertUtil.validateError(compileResult, 3, "{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", 33, 1); BAssertUtil.validateError(compileResult, 4, @@ -439,6 +439,12 @@ public void testResolveWithInstanceAndStatic() { "parameter(s) in class 'class java.lang.Byte', please specify class names 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 " + + "'class org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " + + "please specify class names for each parameter with 'paramTypes' field in the annotation'", + "method_resolve_error.bal", 46, 1); } @Test diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java index a215650a6795..8d22a10125e0 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/Car.java @@ -52,4 +52,16 @@ public static String getMaxSpeed(String model) { } 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"; + } } diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java index f6b7a5fdfcc2..16171743452d 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java @@ -54,4 +54,16 @@ public static String getMaxSpeed(String model) { } return "250MPH"; } + + public char[] getCategorization(char category, char type) { + return new char[] {category, type}; + } + + public String[] getCategorization(String category, String type) { + return new String[] {category, type}; + } + + public String getBatteryType(String prefix) { + return prefix + " EV-HY3-Lithium-ion power"; + } } diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal index 27bba65069ec..46330e31f014 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/method_resolve_error.bal @@ -42,3 +42,13 @@ function getDescription(handle receiver, handle val) returns handle = @java:Meth function newByte(int val) returns handle = @java:Constructor { 'class: "java.lang.Byte" } external; + +function getCarCategorization(handle receiver, handle val1, handle val2) returns handle = @java:Method { + name: "getCategorization", + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + +function getBatteryType(handle receiver, handle val) returns handle = @java:Method { + name: "getBatteryType", + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal index 14eae51c3c18..d39e87233cba 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/overloading/overridden_method_test.bal @@ -44,6 +44,10 @@ function getCarMaxSpeed(handle model) returns handle = @java:Method { 'class: "org.ballerinalang.test.javainterop.overloading.pkg.Car" } external; +function getMillage(int val) returns handle = @java:Method { + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; + public function testOverriddenMethods() { handle strName1 = java:fromString("Generic vehicle"); handle vehicle = newVehicle(strName1); @@ -68,4 +72,5 @@ public function testOverriddenMethods() { test:assertEquals(getSeatCountWithModel(java:fromString("Mazda MX-5")), 2); test:assertEquals(java:toString(sportsCarMaxSpeed), "250MPH"); test:assertEquals(java:toString(carMaxSpeed), "200MPH"); + test:assertEquals(java:toString(getMillage(40)), "40MPG"); } From 495833b43418d00e9e9d2ffdebfde3f44d7a7f7e Mon Sep 17 00:00:00 2001 From: gabilang Date: Tue, 31 Oct 2023 09:11:02 +0530 Subject: [PATCH 06/12] Fix test conflicts --- .../test/javainterop/basic/NegativeValidationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java index b211fed0b5d6..7e7c8f5b432a 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java @@ -448,12 +448,12 @@ public void testOverloadedMethods() { Assert.assertEquals(compileResult.getDiagnostics().length, 2); 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'", "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 " + + "cannot be differentiated. Please specify the parameter types for each parameter in " + "'paramTypes' field in the annotation'", "overloaded_methods.bal", 30, 1); } From 95818e12b20dae66196554eafb6e387c233a23bf Mon Sep 17 00:00:00 2001 From: gabilang Date: Thu, 2 Nov 2023 15:31:26 +0530 Subject: [PATCH 07/12] Add tests with static methods --- .../javainterop/basic/NegativeValidationTest.java | 7 ++++++- .../javainterop/overloading/pkg/SportsCar.java | 14 ++++++++++++++ .../javainterop/negative/overloaded_methods.bal | 4 ++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java index 7e7c8f5b432a..9cf9a670d298 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java @@ -445,7 +445,7 @@ public void testResolveWithInstanceAndStatic() { 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 parameter types for each " + @@ -455,6 +455,11 @@ public void testOverloadedMethods() { "{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 'class org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " + + "please specify class names for each parameter with 'paramTypes' field in the annotation'", + "overloaded_methods.bal", 35, 1); } @Test diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java index 16171743452d..9ba9ca02c04d 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java @@ -66,4 +66,18 @@ public String[] getCategorization(String category, String type) { public String getBatteryType(String prefix) { return prefix + " EV-HY3-Lithium-ion power"; } + + public static String getPrice(String model) { + if (model.equals("Nissan-GTR")) { + return "USD 100,000"; + } + return "USD 50,000"; + } + + public static Object getPrice(Object type) { + if (type.equals("BMW-S6")) { + return " 120,000"; + } + return " 60,000"; + } } diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/overloaded_methods.bal b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/overloaded_methods.bal index 3da564d53ca4..a35ab4e46f44 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/overloaded_methods.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/javainterop/negative/overloaded_methods.bal @@ -31,3 +31,7 @@ function testOverloadedMethods(int[] arr, string str) returns string = @java:Met 'class: "org/ballerinalang/nativeimpl/jvm/tests/StaticMethods", name: "testOverloadedMethods" } external; + +function getPrice(handle val) returns handle = @java:Method { + 'class: "org.ballerinalang.test.javainterop.overloading.pkg.SportsCar" +} external; From 9125173cc60876b94bf818692383061e79a520d4 Mon Sep 17 00:00:00 2001 From: gabilang Date: Thu, 2 Nov 2023 16:17:18 +0530 Subject: [PATCH 08/12] Address reviews --- .../compiler/bir/codegen/interop/JMethodResolver.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index 7b016b892122..6734b90112dc 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -255,10 +255,10 @@ private boolean areAllMethodsOverridden(List jMethods, Class declari 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++) { - Method ithMethod = (Method) jMethods.get(i).getMethod(); - Method kthMethod = (Method) jMethods.get(k).getMethod(); - if (!isOverridden(ithMethod, kthMethod, declaringClass)) { + Method method2 = (Method) jMethods.get(k).getMethod(); + if (!isOverridden(method1, method2, declaringClass)) { return false; } } From 2a57b24c92b677ef9cf5527489cc381c510766aa Mon Sep 17 00:00:00 2001 From: gabilang Date: Mon, 6 Nov 2023 16:50:57 +0530 Subject: [PATCH 09/12] Remove unnecessary static method checks --- .../bir/codegen/interop/JMethodResolver.java | 48 +++++++-------- .../basic/NegativeValidationTest.java | 59 ++++++++++--------- 2 files changed, 52 insertions(+), 55 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index 6734b90112dc..a63527e8b4ef 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -223,22 +223,10 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { if (jMethods.size() == 1 && noConstraints) { return jMethods.get(0); } else if (noConstraints) { - if (areAllMethodsOverridden(jMethods, jMethodRequest.declaringClass)) { + if (areAllMethodsOverridden(jMethods, jMethodRequest)) { return jMethods.get(0); - } - - 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"); + throwOverloadedMethodError(jMethodRequest, jMethods.get(0).getParamTypes().length); } } @@ -250,7 +238,7 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { return jMethod; } - private boolean areAllMethodsOverridden(List jMethods, Class declaringClass) { + private boolean areAllMethodsOverridden(List jMethods, JMethodRequest jMethodRequest) { if (jMethods.get(0).getKind() == JMethodKind.CONSTRUCTOR) { return false; } @@ -258,7 +246,7 @@ private boolean areAllMethodsOverridden(List jMethods, Class declari Method method1 = (Method) jMethods.get(i).getMethod(); for (int k = i + 1; k < jMethods.size(); k++) { Method method2 = (Method) jMethods.get(k).getMethod(); - if (!isOverridden(method1, method2, declaringClass)) { + if (!isOverridden(method1, method2, jMethodRequest)) { return false; } } @@ -266,16 +254,13 @@ private boolean areAllMethodsOverridden(List jMethods, Class declari return true; } - private boolean isOverridden(Method method1, Method method2, Class clazz) { + private boolean isOverridden(Method method1, Method method2, JMethodRequest jMethodRequest) { // Static methods cannot be overridden - if ((Modifier.isStatic(method1.getModifiers()) ^ Modifier.isStatic(method2.getModifiers())) || - method1.getParameterCount() != method2.getParameterCount()) { + 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. 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 " + - "parameter types for each parameter in 'paramTypes' field in the annotation"); + throwOverloadedMethodError(jMethodRequest, method1.getParameterCount()); } // Return false if returns types are not covariant Method currentMethod; @@ -291,11 +276,8 @@ private boolean isOverridden(Method method1, Method method2, Class clazz) { } try { - Method superMethod = clazz.getSuperclass().getDeclaredMethod(currentMethod.getName(), - currentMethod.getParameterTypes()); - if (Modifier.isStatic(currentMethod.getModifiers())) { - return superMethod.equals(otherMethod); - } + Method superMethod = jMethodRequest.declaringClass.getSuperclass() + .getDeclaredMethod(currentMethod.getName(), currentMethod.getParameterTypes()); return Arrays.equals(superMethod.getParameterTypes(), otherMethod.getParameterTypes()) && superMethod.getReturnType().equals(otherMethod.getReturnType()); } catch (NoSuchMethodException e) { @@ -1034,4 +1016,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"); + } + } } diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java index 9cf9a670d298..79d7672ba96b 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java @@ -404,40 +404,41 @@ public void testResolveWithInstanceAndStatic() { Assert.assertEquals(compileResult.getDiagnostics().length, 8); BAssertUtil.validateError(compileResult, 0, "{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", 19, 1); + "'hashCode' with '0' 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", 19, 1); BAssertUtil.validateError(compileResult, 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'", - "method_resolve_error.bal", 24, 1); + "'hashCode' with '0' 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", 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); + "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getSeatCount' " + + "with '0' 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", 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); + "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getSeatCount' with " + + "'0' 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", 33, 1); BAssertUtil.validateError(compileResult, 4, "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getDescription' " + "with '1' parameter(s) in class " + - "'class org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " + - "please specify class names for each parameter with 'paramTypes' field in the annotation'", + "'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 'class java.lang.Byte', please specify class names for each " + + "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 " + - "'class org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " + - "please specify class names for each parameter with 'paramTypes' field in the annotation'", + "'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); } @@ -447,19 +448,21 @@ public void testOverloadedMethods() { CompileResult compileResult = BCompileUtil.compile(path); Assert.assertEquals(compileResult.getDiagnostics().length, 3); BAssertUtil.validateError(compileResult, 0, - "{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); + "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getResource' with " + + "'2' parameter(s) in class 'org.ballerinalang.nativeimpl.jvm.tests.StaticMethods', " + + "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 parameter types for each parameter in " + - "'paramTypes' field in the annotation'", "overloaded_methods.bal", 30, 1); + "'testOverloadedMethods' with '2' parameter(s) in class " + + "'org.ballerinalang.nativeimpl.jvm.tests.StaticMethods', " + + "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 'class org.ballerinalang.test.javainterop.overloading.pkg.SportsCar', " + - "please specify class names for each parameter with 'paramTypes' field in the annotation'", - "overloaded_methods.bal", 35, 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 From 7ca46031e83c9c40e7ddbf832fdc24b9eeef42fd Mon Sep 17 00:00:00 2001 From: gabilang Date: Tue, 7 Nov 2023 09:36:16 +0530 Subject: [PATCH 10/12] Restore the earlier error message and refactor code --- .../bir/codegen/interop/JMethodResolver.java | 42 +++++++------------ .../basic/NegativeValidationTest.java | 38 +++++++---------- 2 files changed, 32 insertions(+), 48 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index a63527e8b4ef..c1103c388029 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -50,7 +50,6 @@ 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.Set; @@ -82,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) { @@ -223,11 +222,10 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { if (jMethods.size() == 1 && noConstraints) { return jMethods.get(0); } else if (noConstraints) { - if (areAllMethodsOverridden(jMethods, jMethodRequest)) { + if (areAllMethodsOverridden(jMethods, jMethodRequest.declaringClass)) { return jMethods.get(0); - } else { - throwOverloadedMethodError(jMethodRequest, jMethods.get(0).getParamTypes().length); } + throwOverloadedMethodError(jMethodRequest, jMethods.get(0).getParamTypes().length); } JMethod jMethod = resolveExactMethod(jMethodRequest.declaringClass, jMethodRequest.methodName, @@ -238,7 +236,7 @@ private JMethod resolve(JMethodRequest jMethodRequest, List jMethods) { return jMethod; } - private boolean areAllMethodsOverridden(List jMethods, JMethodRequest jMethodRequest) { + private boolean areAllMethodsOverridden(List jMethods, Class clazz) { if (jMethods.get(0).getKind() == JMethodKind.CONSTRUCTOR) { return false; } @@ -246,7 +244,7 @@ private boolean areAllMethodsOverridden(List jMethods, JMethodRequest j Method method1 = (Method) jMethods.get(i).getMethod(); for (int k = i + 1; k < jMethods.size(); k++) { Method method2 = (Method) jMethods.get(k).getMethod(); - if (!isOverridden(method1, method2, jMethodRequest)) { + if (!isOverridden(method1, method2, clazz)) { return false; } } @@ -254,13 +252,14 @@ private boolean areAllMethodsOverridden(List jMethods, JMethodRequest j return true; } - private boolean isOverridden(Method method1, Method method2, JMethodRequest jMethodRequest) { - // Static methods cannot be overridden + 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. Also, this occurs when an interop method in an object maps to // instance methods of which one accepting self and another that doesn't. - throwOverloadedMethodError(jMethodRequest, method1.getParameterCount()); + throw new JInteropException( + OVERLOADED_METHODS, "Overloaded methods cannot be differentiated. Please specify the " + + "parameter types for each parameter in 'paramTypes' field in the annotation"); } // Return false if returns types are not covariant Method currentMethod; @@ -276,7 +275,7 @@ private boolean isOverridden(Method method1, Method method2, JMethodRequest jMet } try { - Method superMethod = jMethodRequest.declaringClass.getSuperclass() + Method superMethod = clazz.getSuperclass() .getDeclaredMethod(currentMethod.getName(), currentMethod.getParameterTypes()); return Arrays.equals(superMethod.getParameterTypes(), otherMethod.getParameterTypes()) && superMethod.getReturnType().equals(otherMethod.getReturnType()); @@ -326,8 +325,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 { @@ -344,8 +342,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 { @@ -528,7 +525,7 @@ private boolean isValidParamBType(Class jType, BType bType, boolean isLastPar case TypeTags.MAP: case TypeTags.RECORD: return this.classLoader.loadClass(BMap.class.getCanonicalName()).isAssignableFrom(jType); - case TypeTags.JSON: + case TypeTags.JSON, TypeTags.READONLY: return jTypeName.equals(J_OBJECT_TNAME); case TypeTags.OBJECT: return this.classLoader.loadClass(BObject.class.getCanonicalName()).isAssignableFrom(jType); @@ -556,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 valueSpace = ((BFiniteType) bType).getValueSpace(); - for (Iterator iterator = valueSpace.iterator(); iterator.hasNext(); ) { - BLangExpression value = iterator.next(); + for (BLangExpression value : valueSpace) { if (!isValidParamBType(jType, value.getBType(), isLastParam, restParamExist)) { return false; } @@ -678,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: diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java index 79d7672ba96b..32da1137cfa1 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/basic/NegativeValidationTest.java @@ -404,25 +404,22 @@ public void testResolveWithInstanceAndStatic() { Assert.assertEquals(compileResult.getDiagnostics().length, 8); BAssertUtil.validateError(compileResult, 0, "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " + - "'hashCode' with '0' 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", 19, 1); + "cannot be differentiated. Please specify the parameter types for each " + + "parameter in 'paramTypes' field in the annotation'", + "method_resolve_error.bal", 19, 1); BAssertUtil.validateError(compileResult, 1, "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods " + - "'hashCode' with '0' 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", 24, 1); + "cannot be differentiated. Please specify the parameter types for each " + + "parameter in 'paramTypes' field in the annotation'", + "method_resolve_error.bal", 24, 1); BAssertUtil.validateError(compileResult, 2, - "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getSeatCount' " + - "with '0' 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", 29, 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'", "method_resolve_error.bal", 29, 1); BAssertUtil.validateError(compileResult, 3, - "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getSeatCount' with " + - "'0' 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", 33, 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'", "method_resolve_error.bal", 33, 1); BAssertUtil.validateError(compileResult, 4, "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getDescription' " + "with '1' parameter(s) in class " + @@ -448,15 +445,12 @@ public void testOverloadedMethods() { CompileResult compileResult = BCompileUtil.compile(path); Assert.assertEquals(compileResult.getDiagnostics().length, 3); BAssertUtil.validateError(compileResult, 0, - "{ballerina/jballerina.java}OVERLOADED_METHODS 'Overloaded methods 'getResource' with " + - "'2' parameter(s) in class 'org.ballerinalang.nativeimpl.jvm.tests.StaticMethods', " + - "please specify the parameter types for each parameter in 'paramTypes' field " + + "{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 " + - "'testOverloadedMethods' with '2' parameter(s) in class " + - "'org.ballerinalang.nativeimpl.jvm.tests.StaticMethods', " + - "please specify the parameter types for each parameter in 'paramTypes' field " + + "{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' " + From 7bed77498587c873c95586dda3ba0326df88fca5 Mon Sep 17 00:00:00 2001 From: gabilang Date: Thu, 9 Nov 2023 12:56:31 +0530 Subject: [PATCH 11/12] Remove unnecessary line break --- .../compiler/bir/codegen/interop/JMethodResolver.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index c1103c388029..a7e63bef4c82 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -257,9 +257,8 @@ private boolean isOverridden(Method method1, Method method2, Class clazz) { // 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 " + - "parameter types for each parameter in 'paramTypes' field in the annotation"); + throw new JInteropException(OVERLOADED_METHODS, "Overloaded methods cannot be differentiated. " + + "Please specify the parameter types for each parameter in 'paramTypes' field in the annotation"); } // Return false if returns types are not covariant Method currentMethod; From 238d921b380d535dd3648982b2d4f200bc9f120d Mon Sep 17 00:00:00 2001 From: gabilang Date: Wed, 6 Dec 2023 13:48:00 +0530 Subject: [PATCH 12/12] Address review suggestions --- .../compiler/bir/codegen/interop/JMethodResolver.java | 11 ++++++----- .../test/javainterop/overloading/pkg/SportsCar.java | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java index a7e63bef4c82..d41a5e17ad72 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java @@ -254,13 +254,13 @@ private boolean areAllMethodsOverridden(List jMethods, Class clazz) 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. Also, this occurs when an interop method in an object maps to - // instance methods of which one accepting self and another that doesn't. + // 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"); } - // Return false if returns types are not covariant + // Returns false if return types are not covariant Method currentMethod; Method otherMethod; if (method2.getReturnType().isAssignableFrom(method1.getReturnType())) { @@ -524,7 +524,8 @@ private boolean isValidParamBType(Class jType, BType bType, boolean isLastPar case TypeTags.MAP: case TypeTags.RECORD: return this.classLoader.loadClass(BMap.class.getCanonicalName()).isAssignableFrom(jType); - case TypeTags.JSON, TypeTags.READONLY: + case TypeTags.JSON: + case TypeTags.READONLY: return jTypeName.equals(J_OBJECT_TNAME); case TypeTags.OBJECT: return this.classLoader.loadClass(BObject.class.getCanonicalName()).isAssignableFrom(jType); diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java index 9ba9ca02c04d..cf9447316bba 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/overloading/pkg/SportsCar.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com) All Rights Reserved. + * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com). * * WSO2 LLC. licenses this file to you under the Apache License, * Version 2.0 (the "License"); you may not use this file except