Merge pull request #41856 from MaryamZi/fix-svc-remote-method-annot

Fix annotations for the `service remote function` attachment point

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java

@@ -507,8 +507,9 @@ public void visit(BLangFunction funcNode, AnalyzerData data) {
             // annotation validation for workers is done for the invocation.
             funcNode.annAttachments.forEach(annotationAttachment -> {
                 if (Symbols.isFlagOn(funcNode.symbol.flags, Flags.REMOTE) && funcNode.receiver != null
-                        && Symbols.isService(funcNode.receiver.symbol)) {
+                        && Symbols.isService(funcNode.receiver.getBType().tsymbol)) {
                     annotationAttachment.attachPoints.add(AttachPoint.Point.SERVICE_REMOTE);
+                    annotationAttachment.attachPoints.add(AttachPoint.Point.OBJECT_METHOD);
                 } else if (funcNode.attachedFunction) {
                     annotationAttachment.attachPoints.add(AttachPoint.Point.OBJECT_METHOD);
                 }

tests/jballerina-unit-test/src/test/java/org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue.java

@@ -23,6 +23,7 @@
 import io.ballerina.runtime.api.creators.TypeCreator;
 import io.ballerina.runtime.api.creators.ValueCreator;
 import io.ballerina.runtime.api.types.ObjectType;
+import io.ballerina.runtime.api.types.RemoteMethodType;
 import io.ballerina.runtime.api.types.ResourceMethodType;
 import io.ballerina.runtime.api.types.ServiceType;
 import io.ballerina.runtime.api.utils.StringUtils;
@@ -191,6 +192,16 @@ public static BValue getResourceMethodAnnotations(BObject service, BString metho
         return null;
     }
 
+    public static BValue getRemoteMethodAnnotations(BObject service, BString method, BString annotName) {
+        String methodName = method.getValue();
+        for (RemoteMethodType methodType : ((ServiceType) service.getOriginalType()).getRemoteMethods()) {
+            if (methodType.getName().equals(methodName)) {
+                return (BValue) methodType.getAnnotation(annotName);
+            }
+        }
+        return null;
+    }
+
     public static BArray getParamDefaultability(BObject service, BString name) {
         ServiceType serviceType = (ServiceType) service.getType();
         Optional<ResourceMethodType> func = Arrays.stream(serviceType.getResourceMethods())

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/annotations/AnnotationAttachmentNegativeTest.java

@@ -38,7 +38,7 @@ public class AnnotationAttachmentNegativeTest {
     @BeforeClass
     public void setup() {
         compileResult = BCompileUtil.compile("test-src/annotations/annot_attachments_negative.bal");
-        Assert.assertEquals(compileResult.getErrorCount(), 289);
+        Assert.assertEquals(compileResult.getErrorCount(), 302);
     }
 
     @Test
@@ -537,6 +537,43 @@ public void testInvalidAnnotationAttachmentsOnMembersOfStructuredTypedBindingPat
         validateError(compileResult, index, "undefined annotation 'UndefinedAnnotation'", line += 1, 20);
     }
 
+    @Test
+    public void testInvalidAttachmentOnServiceRemoteMethod() {
+        int index = 289;
+        int line = 1014;
+        validateError(compileResult, index++, "annotation 'v1' is not allowed on service_remote, object_method, " +
+                "function", line, 5);
+        validateError(compileResult, index++, "annotation 'v2' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index++, "annotation 'v6' is not allowed on service_remote, object_method, " +
+                "function", line += 12, 5);
+        validateError(compileResult, index++, "annotation 'v7' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index++, "annotation 'v8' is not allowed on service_remote, object_method, " +
+                "function", ++line, 5);
+        validateError(compileResult, index++, "annotation 'v9' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index++, "annotation 'v10' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index++, "annotation 'v11' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index++, "annotation 'v12' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index++, "annotation 'v13' is not allowed on service_remote, object_method, " +
+                "function", line += 3, 5);
+        validateError(compileResult, index, "annotation 'v15' is not allowed on service_remote, object_method, " +
+                "function", line + 3, 5);
+    }
+
+    @Test
+    public void testInvalidServiceRemoteMethodAttachmentOnNonRemoteServiceMethods() {
+        int index = 300;
+        validateError(compileResult, index++, "annotation 'v26' is not allowed on object_method, function",
+                      1056, 5);
+        validateError(compileResult, index, "annotation 'v26' is not allowed on object_method, function",
+                      1059, 5);
+    }
+
     @AfterClass
     public void tearDown() {
         compileResult = null;

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/annotations/AnnotationAttachmentSymbolsTest.java

@@ -394,6 +394,16 @@ public void testConstAnnots() {
         assertAttachmentSymbol(attachmentsF4.get(0), "v29", true, "increment", -2L);
     }
 
+    @Test
+    public void testAnnotWithServiceRemoteMethodAttachmentPoint() {
+        BLangFunction function = getFunction("ServiceClass.serviceRemoteFn1");
+        List<? extends AnnotationAttachmentSymbol> attachments = function.symbol.getAnnotations();
+        Assert.assertEquals(attachments.size(), 2);
+
+        assertAttachmentSymbol(attachments.get(0), "v31");
+        assertAttachmentSymbol(attachments.get(1), "v32", true, "increment", 1112L);
+    }
+
     private BLangTypeDefinition getTypeDefinition(List<? extends TypeDefinition> typeDefinitions, String name) {
         for (TypeDefinition typeDefinition : typeDefinitions) {
             BLangTypeDefinition bLangTypeDefinition = (BLangTypeDefinition) typeDefinition;

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/annotations/AnnotationAttachmentTest.java

@@ -18,6 +18,7 @@
 
 import io.ballerina.tools.diagnostics.Location;
 import org.ballerinalang.model.elements.PackageID;
+import org.ballerinalang.model.tree.ClassDefinition;
 import org.ballerinalang.model.tree.NodeKind;
 import org.ballerinalang.model.tree.ServiceNode;
 import org.ballerinalang.model.tree.TopLevelNode;
@@ -51,6 +52,7 @@
 import org.wso2.ballerinalang.compiler.tree.expressions.BLangRecordLiteral;
 import org.wso2.ballerinalang.compiler.tree.expressions.BLangSimpleVarRef;
 import org.wso2.ballerinalang.compiler.tree.expressions.BLangTypeConversionExpr;
+import org.wso2.ballerinalang.compiler.tree.types.BLangObjectTypeNode;
 import org.wso2.ballerinalang.compiler.tree.types.BLangTupleTypeNode;
 import org.wso2.ballerinalang.compiler.util.TypeTags;
 
@@ -161,15 +163,7 @@ public void testAnnotOnListener() {
 
     @Test
     public void testAnnotOnServiceOne() {
-        List<BLangAnnotationAttachment> attachments = (List<BLangAnnotationAttachment>)
-                compileResult.getAST().getServices().stream()
-                        .filter(serviceNode ->
-                                serviceNode.getAbsolutePath().stream().anyMatch(p -> p.getValue().contains("ser")))
-                        .findFirst()
-                        .get().getServiceClass().getAnnotationAttachments()
-                        .stream()
-                        .filter(ann -> !isServiceIntropAnnot((BLangAnnotationAttachment) ann))
-                        .collect(Collectors.toList());
+        List<BLangAnnotationAttachment> attachments = getServiceClassAnnotations("ser");
         Assert.assertEquals(attachments.size(), 1);
         assertAnnotationNameAndKeyValuePair(attachments.get(0), "v8", "val", "v8");
     }
@@ -568,6 +562,43 @@ public void testAnnotOnTupleMember() {
         Assert.assertEquals(m1.annAttachments.get(0).annotationName.getValue(), "v30");
     }
 
+    @Test
+    public void testAnnotOnServiceRemoteMethodOfServiceClass() {
+        BLangClassDefinition serviceClass = getClassDefinition(((BLangPackage) compileResult.getAST()).topLevelNodes,
+                                                               "ServiceClass");
+        List<BLangAnnotationAttachment> attachments =
+                serviceClass.functions.stream()
+                        .filter(f -> f.name.value.equals("serviceRemoteFn1"))
+                        .findFirst()
+                        .get().getAnnotationAttachments();
+        Assert.assertEquals(attachments.size(), 2);
+        assertAnnotationNameAndKeyValuePair(attachments.get(0), "v31", "increment", 1111L);
+        assertAnnotationNameAndKeyValuePair(attachments.get(1), "v32", "increment", 1112L);
+    }
+
+    @Test
+    public void testAnnotOnServiceRemoteMethodOfServiceType() {
+        BLangTypeDefinition serviceObject = getTypeDefinition(compileResult.getAST().getTypeDefinitions(),
+                                                              "ServiceObject");
+        List<BLangAnnotationAttachment> attachments =
+                ((BLangObjectTypeNode) serviceObject.typeNode).functions.stream()
+                        .filter(f -> f.name.value.equals("serviceRemoteFn2"))
+                        .findFirst()
+                        .get().getAnnotationAttachments();
+        Assert.assertEquals(attachments.size(), 1);
+        assertAnnotationNameAndKeyValuePair(attachments.get(0), "v31", "increment", 1113L);
+    }
+
+    @Test
+    public void testAnnotOnServiceRemoteMethodOfServiceDecl() {
+        List<BLangAnnotationAttachment> attachments = getServiceClassForServiceDecl("ser2").getFunctions().stream()
+                .filter(f -> f.name.value.equals("serviceRemoteFn3"))
+                .findFirst()
+                .get().getAnnotationAttachments();
+        Assert.assertEquals(attachments.size(), 1);
+        assertAnnotationNameAndKeyValuePair(attachments.get(0), "v32", "increment", 1114L);
+    }
+
     private BLangTypeDefinition getTypeDefinition(List<? extends TypeDefinition> typeDefinitions, String name) {
         for (TypeDefinition typeDefinition : typeDefinitions) {
             BLangTypeDefinition bLangTypeDefinition = (BLangTypeDefinition) typeDefinition;
@@ -592,6 +623,23 @@ private BLangClassDefinition getClassDefinition(List<? extends TopLevelNode> typ
         throw new RuntimeException("Class Definition '" + name + "' not found.");
     }
 
+    private List<BLangAnnotationAttachment> getServiceClassAnnotations(String name) {
+        return (List<BLangAnnotationAttachment>)
+                getServiceClassForServiceDecl(name).getAnnotationAttachments()
+                        .stream()
+                        .filter(ann -> !isServiceIntropAnnot((BLangAnnotationAttachment) ann))
+                        .collect(Collectors.toList());
+    }
+
+    private ClassDefinition getServiceClassForServiceDecl(String name) {
+        return compileResult.getAST().getServices().stream()
+                .filter(serviceNode ->
+                                serviceNode.getAbsolutePath().stream()
+                                        .anyMatch(p -> p.getValue().contains(name)))
+                .findFirst()
+                .get().getServiceClass();
+    }
+
     @AfterClass
     public void tearDown() {
         compileResult = null;

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/annotations/AnnotationRuntimeTest.java

@@ -210,6 +210,12 @@ public void testListExprInConstAnnot() {
         BRunUtil.invoke(resultOne, "testListExprInConstAnnot");
     }
 
+    @Test
+    public void testServiceRemoteMethodAnnotations() {
+        CompileResult result = BCompileUtil.compile("test-src/annotations/service_remote_method_annotations.bal");
+        BRunUtil.invoke(result, "testServiceRemoteMethodAnnotations");
+    }
+
     @AfterClass
     public void tearDown() {
         resultOne = null;

tests/jballerina-unit-test/src/test/resources/test-src/annotations/annot_attachments.bal

@@ -337,3 +337,32 @@ type F3 record {|
 type F4 record {|
     int x;
 |};
+
+public annotation record {| int increment; |} v31 on service remote function;
+public const annotation record {| int increment; |} v32 on source type, source service remote function;
+
+service class ServiceClass {
+    @v31 {
+        increment: 1111
+    }
+    @v32 {
+        increment: 1112
+    }
+    remote function serviceRemoteFn1() {
+    }
+}
+
+type ServiceObject service object {
+    @v31 {
+        increment: 1113
+    }
+    remote function serviceRemoteFn2();
+};
+
+service /ser2 on new Listener() {
+    @v32 {
+        increment: 1114
+    }
+    remote function serviceRemoteFn3() {
+    }
+}

tests/jballerina-unit-test/src/test/resources/test-src/annotations/annot_attachments_negative.bal

@@ -1005,3 +1005,57 @@ function testInvalidAnnotationAttachmentsOnMembersOfStructuredTypedBindingPatter
     error<map<[@UndefinedAnnotation int]>> error () = error("err");
     error<record {|@UndefinedAnnotation int x = 10;|}> error () = error("err");
 }
+
+public annotation v26 on service remote function;
+
+service class ServiceClass2 {
+    string name = "ballerina";
+
+    @v1 {
+        val: "v1"
+    }
+    @v2 {
+        val: "v2"
+    }
+    @v3 { // OK
+        val: "v3"
+    }
+    @v4 { // OK
+        x: 1
+    }
+    @v5 { // OK
+        val: "v5"
+    }
+    @v6 {
+        val: "v6"
+    }
+    @v7
+    @v8 {
+        val: "v8"
+    }
+    @v9 {
+        val: "v9"
+    }
+    @v10 {
+        val: "v10"
+    }
+    @v11 {
+        val: 11
+    }
+    @v12 {
+        val: "v12"
+    }
+    @v13 {
+        val: "v13"
+    }
+    @v15 {
+        val: false
+    }
+    remote function getName() returns string { return self.name; }
+
+    @v26
+    resource function get name() returns string { return self.name; }
+
+    @v26
+    function getFirstName() returns string { return self.name; }
+}

tests/jballerina-unit-test/src/test/resources/test-src/annotations/service_remote_method_annotations.bal

@@ -0,0 +1,56 @@
+// Copyright (c) 2023 WSO2 LLC. (http://www.wso2.org).
+//
+// 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.
+
+import ballerina/jballerina.java;
+
+type Rec record {|
+    int increment;
+|};
+
+public annotation Rec srma on service remote function;
+
+service class ServiceClass {
+    @srma {
+        increment: 1111
+    }
+    remote function serviceRemoteFn() {
+
+    }
+}
+
+function testServiceRemoteMethodAnnotations() returns error? {
+    any annots = getRemoteMethodAnnotations(new ServiceClass(), "serviceRemoteFn", "srma");
+    assertTrue(annots is Rec);
+    Rec rec = <Rec> annots;
+    assertEquality({increment: 1111}, rec);
+}
+
+function getRemoteMethodAnnotations(service object {} obj, string methodName, string annotName) returns any =
+    @java:Method {
+        'class: "org/ballerinalang/nativeimpl/jvm/servicetests/ServiceValue"
+    } external;
+
+function assertTrue(anydata actual) {
+    assertEquality(true, actual);
+}
+
+function assertEquality(anydata expected, anydata actual) {
+    if expected == actual {
+        return;
+    }
+
+    panic error(string `expected ${expected.toBalString()}, found ${actual.toBalString()}`);
+}