Merge pull request #771 from aashikam/annotation-allow

[Master] Update compiler plugin to allow annotations

changelog.md

@@ -1,11 +1,16 @@
 # Change Log
 This file contains all the notable changes done to the Ballerina NATS package through the releases.
 
-[Unreleased]
+## [Unreleased]
 
 ### Changed
-- [[#4237] Exit the Listener When Panic Occurred](https://github.com/ballerina-platform/ballerina-standard-library/issues/4237)
 
+- [[#4733] Changed disallowing service level annotations in the compiler plugin](https://github.com/ballerina-platform/ballerina-standard-library/issues/4733)
+
+## [2.7.0] - 2023-04-10
+
+### Changed
+- [[#4237] Exit the Listener When Panic Occurred](https://github.com/ballerina-platform/ballerina-standard-library/issues/4237)
 
 ## [2.6.0] - 2023-02-20
 

compiler-plugin-tests/src/test/java/io/ballerina/stdlib/nats/plugin/CodeSnippetGenerationCodeActionTest.java

@@ -69,7 +69,7 @@ private CodeActionInfo getExpectedCodeAction(String filePath, int line, int offs
                 LinePosition.from(line, offset));
         CodeActionArgument locationArg = CodeActionArgument.from(NODE_LOCATION, lineRange);
         CodeActionInfo codeAction = CodeActionInfo.from("Insert service template", List.of(locationArg));
-        codeAction.setProviderName("NATS_121/ballerinax/nats/ADD_REMOTE_FUNCTION_CODE_SNIPPET");
+        codeAction.setProviderName("NATS_119/ballerinax/nats/ADD_REMOTE_FUNCTION_CODE_SNIPPET");
         return codeAction;
     }
 }

compiler-plugin-tests/src/test/java/io/ballerina/stdlib/nats/plugin/NatsCompilerPluginTest.java

@@ -214,6 +214,14 @@ public void testValidJetStreamServiceWithReadOnlyMessage() {
         Assert.assertEquals(diagnosticResult.errors().size(), 0);
     }
 
+    @Test
+    public void testValidServicesWithDisplayAnnotation() {
+        Package currentPackage = loadPackage("valid_service_23");
+        PackageCompilation compilation = currentPackage.getCompilation();
+        DiagnosticResult diagnosticResult = compilation.diagnosticResult();
+        Assert.assertEquals(diagnosticResult.errors().size(), 0);
+    }
+
     @Test
     public void testInvalidService1() {
         Package currentPackage = loadPackage("invalid_service_1");
@@ -523,6 +531,26 @@ public void testJetStreamServiceWithInvalidReturnOnMessageMethod() {
         assertDiagnostic(diagnostic, CompilationErrors.INVALID_RETURN_TYPE_ERROR_OR_NIL);
     }
 
+    @Test(description = "NATS service with no service config or service name")
+    public void testInvalidService30() {
+        Package currentPackage = loadPackage("invalid_service_30");
+        PackageCompilation compilation = currentPackage.getCompilation();
+        DiagnosticResult diagnosticResult = compilation.diagnosticResult();
+        Assert.assertEquals(diagnosticResult.errors().size(), 1);
+        Diagnostic diagnostic = (Diagnostic) diagnosticResult.errors().toArray()[0];
+        assertDiagnostic(diagnostic, CompilationErrors.NO_ANNOTATION);
+    }
+
+    @Test(description = "NATS service with no service config or service name")
+    public void testInvalidService31() {
+        Package currentPackage = loadPackage("invalid_service_31");
+        PackageCompilation compilation = currentPackage.getCompilation();
+        DiagnosticResult diagnosticResult = compilation.diagnosticResult();
+        Assert.assertEquals(diagnosticResult.errors().size(), 1);
+        Diagnostic diagnostic = (Diagnostic) diagnosticResult.errors().toArray()[0];
+        assertDiagnostic(diagnostic, CompilationErrors.NO_ANNOTATION);
+    }
+
     private Package loadPackage(String path) {
         Path projectDirPath = RESOURCE_DIRECTORY.resolve(BALLERINA_SOURCES).resolve(path);
         BuildProject project = BuildProject.load(getEnvironmentBuilder(), projectDirPath);

compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_service_30/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "nats_test"
+name = "invalid_service_30"
+version = "0.1.0"

compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_service_30/service.bal

@@ -0,0 +1,28 @@
+// 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.
+
+import ballerinax/nats;
+
+listener nats:Listener subscription = new(nats:DEFAULT_URL);
+
+@display {
+    label: "natsService"
+}
+service nats:Service on subscription {
+
+    remote function onMessage(nats:Message message) returns error? {
+    }
+}

compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_service_31/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "nats_test"
+name = "invalid_service_31"
+version = "0.1.0"

compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_service_31/service.bal

@@ -0,0 +1,28 @@
+// 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.
+
+import ballerinax/nats;
+
+listener nats:Listener subscription = new(nats:DEFAULT_URL);
+
+@display {
+    label: "natsService"
+}
+service nats:Service on subscription {
+
+    remote function onMessage(nats:Message message) returns error? {
+    }
+}

compiler-plugin-tests/src/test/resources/ballerina_sources/valid_service_23/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "nats_test"
+name = "valid_service_23"
+version = "0.1.0"

compiler-plugin-tests/src/test/resources/ballerina_sources/valid_service_23/service.bal

@@ -0,0 +1,55 @@
+// 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.
+
+import ballerinax/nats;
+
+listener nats:Listener subscription = new(nats:DEFAULT_URL);
+
+@display {
+    label: "natsService"
+}
+service "demo.bbe" on subscription {
+
+    remote function onMessage(nats:Message message) {
+    }
+}
+
+@display {
+    label: "natsService"
+}
+@nats:ServiceConfig {
+    subject: "demo.bbe.*"
+}
+service nats:Service on subscription {
+
+    remote function onMessage(nats:Message message) {
+    }
+}
+
+nats:Client natsClient = check new(nats:DEFAULT_URL);
+listener nats:JetStreamListener streamSubscription = new(natsClient);
+
+@display {
+    label: "natsService"
+}
+@nats:StreamServiceConfig {
+    subject: "demo.bbe.*"
+}
+service nats:JetStreamService on streamSubscription {
+
+    remote function onMessage(readonly & nats:JetStreamMessage message) {
+    }
+}

compiler-plugin/src/main/java/io/ballerina/stdlib/nats/plugin/NatsServiceValidator.java

@@ -34,7 +34,6 @@
 import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext;
 import io.ballerina.stdlib.nats.plugin.PluginConstants.CompilationErrors;
 import io.ballerina.tools.diagnostics.DiagnosticSeverity;
-import io.ballerina.tools.diagnostics.Location;
 
 import java.util.List;
 import java.util.Optional;
@@ -56,7 +55,7 @@ public void validate(SyntaxNodeAnalysisContext context) {
 
         if (serviceDeclarationNode.members().isEmpty() || !hasRemoteFunction) {
             context.reportDiagnostic(PluginUtils.getDiagnostic(CompilationErrors.TEMPLATE_CODE_GENERATION_HINT,
-                   DiagnosticSeverity.INTERNAL, serviceDeclarationNode.location()));
+                    DiagnosticSeverity.INTERNAL, serviceDeclarationNode.location()));
         }
 
         validateAttachPoint(context);
@@ -93,44 +92,41 @@ private void validateAttachPoint(SyntaxNodeAnalysisContext context) {
         SemanticModel semanticModel = context.semanticModel();
         ServiceDeclarationNode serviceDeclarationNode = (ServiceDeclarationNode) context.node();
         Optional<Symbol> symbol = semanticModel.symbol(serviceDeclarationNode);
+
         if (symbol.isPresent()) {
             ServiceDeclarationSymbol serviceDeclarationSymbol = (ServiceDeclarationSymbol) symbol.get();
-            Optional<ServiceAttachPoint> attachPoint = serviceDeclarationSymbol.attachPoint();
-            List<AnnotationSymbol> symbolList = serviceDeclarationSymbol.annotations();
-            if (attachPoint.isEmpty()) {
-                if (symbolList.isEmpty()) {
-                    context.reportDiagnostic(PluginUtils.getDiagnostic(CompilationErrors.NO_ANNOTATION,
-                            DiagnosticSeverity.ERROR, serviceDeclarationNode.location()));
-                } else if (symbolList.size() > 1) {
-                    context.reportDiagnostic(PluginUtils.getDiagnostic(CompilationErrors.INVALID_ANNOTATION_NUMBER,
-                            DiagnosticSeverity.ERROR, serviceDeclarationNode.location()));
-                } else {
-                    validateAnnotation(symbolList.get(0), serviceDeclarationNode.location(),
-                            context);
-                }
-            } else {
-                if (attachPoint.get().kind() != ServiceAttachPointKind.STRING_LITERAL) {
-                    if (serviceDeclarationSymbol.annotations().isEmpty()) {
-                        context.reportDiagnostic(PluginUtils.getDiagnostic(
-                                CompilationErrors.INVALID_SERVICE_ATTACH_POINT,
-                                DiagnosticSeverity.ERROR, serviceDeclarationNode.location()));
-                    } else {
-                        validateAnnotation(symbolList.get(0), serviceDeclarationNode.location(),
-                                context);
-                    }
-                }
+            Optional<ServiceAttachPoint> serviceNameAttachPoint = serviceDeclarationSymbol.attachPoint();
+            List<AnnotationSymbol> annotations = serviceDeclarationSymbol.annotations();
+
+            boolean serviceNameIsStringLiteral = serviceNameAttachPoint.isPresent() &&
+                    serviceNameAttachPoint.get().kind() == ServiceAttachPointKind.STRING_LITERAL;
+
+            if (annotations.isEmpty() && !serviceNameIsStringLiteral) {
+                // Case 1: No service name and no annotation
+                reportError(context, CompilationErrors.INVALID_SERVICE_ATTACH_POINT, serviceDeclarationNode);
+            } else if (!hasServiceConfig(annotations) && !serviceNameIsStringLiteral) {
+                // Case 2: Service name is not a string and no annotation
+                reportError(context, CompilationErrors.NO_ANNOTATION, serviceDeclarationNode);
             }
         }
     }
 
-    private void validateAnnotation(AnnotationSymbol annotationSymbol, Location location,
-                                    SyntaxNodeAnalysisContext context) {
-        Optional<ModuleSymbol> moduleSymbolOptional = annotationSymbol.getModule();
-        ModuleSymbol moduleSymbol = moduleSymbolOptional.get();
-        if (!moduleSymbol.id().orgName().equals(PluginConstants.PACKAGE_ORG) ||
-                !moduleSymbol.id().moduleName().equals(PluginConstants.PACKAGE_PREFIX)) {
-            context.reportDiagnostic(PluginUtils.getDiagnostic(CompilationErrors.INVALID_ANNOTATION,
-                    DiagnosticSeverity.ERROR, location));
+    private void reportError(SyntaxNodeAnalysisContext context, CompilationErrors error, Node locationNode) {
+        context.reportDiagnostic(PluginUtils.getDiagnostic(error, DiagnosticSeverity.ERROR, locationNode.location()));
+    }
+
+    private boolean hasServiceConfig(List<AnnotationSymbol> annotationSymbols) {
+        for (AnnotationSymbol annotationSymbol : annotationSymbols) {
+            Optional<ModuleSymbol> moduleSymbolOptional = annotationSymbol.getModule();
+            if (moduleSymbolOptional.isPresent()) {
+                ModuleSymbol moduleSymbol = moduleSymbolOptional.get();
+                if (PluginConstants.PACKAGE_ORG.equals(moduleSymbol.id().orgName()) &&
+                        PluginConstants.PACKAGE_PREFIX.equals(moduleSymbol.id().moduleName())) {
+                    // not checking name as rabbitmq has only two annotations and only one is allowed on services.
+                    return true;
+                }
+            }
         }
+        return false;
     }
 }

compiler-plugin/src/main/java/io/ballerina/stdlib/nats/plugin/PluginConstants.java

@@ -74,15 +74,10 @@ enum CompilationErrors {
                 "NATS_115"),
         INVALID_MULTIPLE_LISTENERS("Multiple listener attachments. Only one nats:Listener is allowed.",
                 "NATS_116"),
-        INVALID_ANNOTATION_NUMBER("Only one service config annotation is allowed.",
-                "NATS_117"),
-        NO_ANNOTATION("No @nats:ServiceConfig{} annotation is found.",
-                "NATS_118"),
-        INVALID_ANNOTATION("Invalid service config annotation. Only @nats:ServiceConfig{} is allowed.",
-                "NATS_119"),
+        NO_ANNOTATION("No ServiceConfig annotation is found.", "NATS_117"),
         INVALID_SERVICE_ATTACH_POINT("Invalid service attach point. Only string literals are allowed.",
-                "NATS_120"),
-        TEMPLATE_CODE_GENERATION_HINT("Template generation for empty service", "NATS_121");
+                "NATS_118"),
+        TEMPLATE_CODE_GENERATION_HINT("Template generation for empty service", "NATS_119");
 
         private final String error;
         private final String errorCode;