Merge pull request #40901 from LakshanWeerasinghe/fix-#40619

Add missing qualifiers for the object type symbol when loaded through BIR
ballerina-platform · Aug 10, 2023 · 8a0431e · 8a0431e
2 parents 4af6439 + dda9b34
commit 8a0431e
Show file tree

Hide file tree

Showing 15 changed files with 319 additions and 59 deletions.
diff --git a/...r/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/BIRPackageSymbolEnter.java b/...r/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/BIRPackageSymbolEnter.java
@@ -1631,41 +1631,27 @@ public BType readType(int cpI) throws IOException {
                     }
                     return finiteType;
                 case TypeTags.OBJECT:
-                    boolean service = inputStream.readByte() == 1;
-
                     pkgCpIndex = inputStream.readInt();
                     pkgId = getPackageId(pkgCpIndex);
 
                     String objName = getStringCPEntryValue(inputStream);
-                    var objFlags = (inputStream.readBoolean() ? Flags.CLASS : 0) | Flags.PUBLIC;
-                    objFlags = inputStream.readBoolean() ? objFlags | Flags.CLIENT : objFlags;
+                    long objSymFlags = inputStream.readLong();
                     BObjectTypeSymbol objectSymbol;
 
-                    if (Symbols.isFlagOn(objFlags, Flags.CLASS)) {
-                        objectSymbol = Symbols.createClassSymbol(objFlags, names.fromString(objName),
-                                                                 env.pkgSymbol.pkgID, null, env.pkgSymbol,
-                                                                 symTable.builtinPos, COMPILED_SOURCE, false);
+                    if (Symbols.isFlagOn(objSymFlags, Flags.CLASS)) {
+                        objectSymbol = Symbols.createClassSymbol(objSymFlags, names.fromString(objName),
+                                env.pkgSymbol.pkgID, null, env.pkgSymbol,
+                                symTable.builtinPos, COMPILED_SOURCE, false);
                     } else {
-                        objectSymbol = Symbols.createObjectSymbol(objFlags, names.fromString(objName),
-                                                                  env.pkgSymbol.pkgID, null, env.pkgSymbol,
-                                                                  symTable.builtinPos, COMPILED_SOURCE);
+                        objectSymbol = Symbols.createObjectSymbol(objSymFlags, names.fromString(objName),
+                                env.pkgSymbol.pkgID, null, env.pkgSymbol,
+                                symTable.builtinPos, COMPILED_SOURCE);
                     }
 
                     objectSymbol.scope = new Scope(objectSymbol);
                     BObjectType objectType;
                     // Below is a temporary fix, need to fix this properly by using the type tag
                     objectType = new BObjectType(objectSymbol);
-
-                    if (service) {
-                        objectType.flags |= Flags.SERVICE;
-                        objectSymbol.flags |= Flags.SERVICE;
-                    }
-                    if (isImmutable(flags)) {
-                        objectSymbol.flags |= Flags.READONLY;
-                    }
-                    if (Symbols.isFlagOn(flags, Flags.ANONYMOUS)) {
-                        objectSymbol.flags |= Flags.ANONYMOUS;
-                    }
                     objectType.flags = flags;
                     objectSymbol.type = objectType;
                     addShapeCP(objectType, cpI);

diff --git a/...allerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/writer/BIRTypeWriter.java b/...allerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/writer/BIRTypeWriter.java
@@ -439,14 +439,6 @@ public void visit(BRecordType bRecordType) {
 
     @Override
     public void visit(BObjectType bObjectType) {
-        //This is to say this is an object, this is a temporary fix object - 1, service - 0,
-        // ideal fix would be to use the type tag to
-        // differentiate. TODO fix later
-        if ((bObjectType.flags & Flags.SERVICE) == Flags.SERVICE) {
-            buff.writeByte(1);
-        } else {
-            buff.writeByte(0);
-        }
         writeObjectAndServiceTypes(bObjectType);
         writeTypeIds(bObjectType.typeIdSet);
     }
@@ -460,9 +452,7 @@ private void writeObjectAndServiceTypes(BObjectType bObjectType) {
         BTypeDefinitionSymbol typDefSymbol = ((BObjectTypeSymbol) tSymbol).typeDefinitionSymbol;
         buff.writeInt(addStringCPEntry(Objects.requireNonNullElse(typDefSymbol, tSymbol).name.value));
 
-        //TODO below two line are a temp solution, introduce a generic concept
-        buff.writeBoolean(Symbols.isFlagOn(tSymbol.flags, Flags.CLASS)); // Abstract object or not
-        buff.writeBoolean(Symbols.isFlagOn(tSymbol.flags, Flags.CLIENT));
+        buff.writeLong(tSymbol.flags);
         buff.writeInt(bObjectType.fields.size());
         for (BField field : bObjectType.fields.values()) {
             buff.writeInt(addStringCPEntry(field.name.value));

diff --git a/...ballerina-lang/src/main/java/org/wso2/ballerinalang/programfile/ProgramFileConstants.java b/...ballerina-lang/src/main/java/org/wso2/ballerinalang/programfile/ProgramFileConstants.java
@@ -25,9 +25,9 @@ public class ProgramFileConstants {
 
     public static final int MAGIC_NUMBER = 0xBA1DA4CE;
     public static final short VERSION_NUMBER = 50;
-    public static final int BIR_VERSION_NUMBER = 69;
-    public static final short MIN_SUPPORTED_VERSION = 69;
-    public static final short MAX_SUPPORTED_VERSION = 69;
+    public static final int BIR_VERSION_NUMBER = 70;
+    public static final short MIN_SUPPORTED_VERSION = 70;
+    public static final short MAX_SUPPORTED_VERSION = 70;
 
     // todo move this to a proper place
     public static final String[] SUPPORTED_PLATFORMS = {"java17", "java11"};

diff --git a/docs/bir-spec/src/main/resources/kaitai/bir.ksy b/docs/bir-spec/src/main/resources/kaitai/bir.ksy
@@ -294,16 +294,12 @@ types:
         type: s4
   type_object_or_service:
     seq:
-      - id: is_object_type
-        type: s1
       - id: pkd_id_cp_index
         type: s4
       - id: name_cp_index
         type: s4
-      - id: is_abstract
-        type: u1
-      - id: is_client
-        type: u1
+      - id: object_symbol_flags
+        type: s8
       - id: object_fields_count
         type: s4
       - id: object_fields

diff --git a/...-variable/config-rename-positional-capability/variableAssignmentRequiredCodeAction38.json b/...-variable/config-rename-positional-capability/variableAssignmentRequiredCodeAction38.json
@@ -20,7 +20,7 @@
               "character": 4
             }
           },
-          "newText": "object {public isolated function iterator() returns object {public isolated function next() returns record {|int value;|}?;};} objectResult = "
+          "newText": "isolated object {public isolated function iterator() returns isolated object {public isolated function next() returns record {|int value;|}?;};} objectResult = "
         }
       ],
       "command": {
@@ -30,7 +30,7 @@
           "createVariable5.bal",
           {
             "line": 85,
-            "character": 131
+            "character": 149
           }
         ]
       },

diff --git a/...-variable/config-rename-positional-capability/variableAssignmentRequiredCodeAction39.json b/...-variable/config-rename-positional-capability/variableAssignmentRequiredCodeAction39.json
@@ -20,7 +20,7 @@
               "character": 4
             }
           },
-          "newText": "object {public isolated function iterator() returns object {public isolated function next() returns record {|int value;|}?;};} objectResult = "
+          "newText": "isolated object {public isolated function iterator() returns isolated object {public isolated function next() returns record {|int value;|}?;};} objectResult = "
         }
       ],
       "command": {
@@ -30,7 +30,7 @@
           "createVariable5.bal",
           {
             "line": 86,
-            "character": 131
+            "character": 149
           }
         ]
       },

diff --git a/...langserver-core/src/test/resources/completion/annotation_ctx/config/langAnnotations1.json b/...langserver-core/src/test/resources/completion/annotation_ctx/config/langAnnotations1.json
@@ -197,11 +197,11 @@
     {
       "label": "...()",
       "kind": "Function",
-      "detail": "object {public isolated function iterator() returns object {public isolated function next() returns record {|int value;|}?;};}",
+      "detail": "isolated object {public isolated function iterator() returns isolated object {public isolated function next() returns record {|int value;|}?;};}",
       "documentation": {
         "right": {
           "kind": "markdown",
-          "value": "**Package:** _ballerina/lang.annotations:0.0.0_  \n  \n  \n  \n  \n**Return** `object {public isolated function iterator() returns object {public isolated function next() returns record {|int value;|}?;};}`   \n  \n"
+          "value": "**Package:** _ballerina/lang.annotations:0.0.0_  \n  \n  \n  \n  \n**Return** `isolated object {public isolated function iterator() returns isolated object {public isolated function next() returns record {|int value;|}?;};}`   \n  \n"
         }
       },
       "sortText": "A",
@@ -212,11 +212,11 @@
     {
       "label": "..<()",
       "kind": "Function",
-      "detail": "object {public isolated function iterator() returns object {public isolated function next() returns record {|int value;|}?;};}",
+      "detail": "isolated object {public isolated function iterator() returns isolated object {public isolated function next() returns record {|int value;|}?;};}",
       "documentation": {
         "right": {
           "kind": "markdown",
-          "value": "**Package:** _ballerina/lang.annotations:0.0.0_  \n  \n  \n  \n  \n**Return** `object {public isolated function iterator() returns object {public isolated function next() returns record {|int value;|}?;};}`   \n  \n"
+          "value": "**Package:** _ballerina/lang.annotations:0.0.0_  \n  \n  \n  \n  \n**Return** `isolated object {public isolated function iterator() returns isolated object {public isolated function next() returns record {|int value;|}?;};}`   \n  \n"
         }
       },
       "sortText": "A",

diff --git a/...llerina-compiler-api-test/src/test/java/io/ballerina/semantic/api/test/SymbolBIRTest.java b/...llerina-compiler-api-test/src/test/java/io/ballerina/semantic/api/test/SymbolBIRTest.java
@@ -58,11 +58,16 @@
 import java.util.Objects;
 import java.util.Optional;
 
+import static io.ballerina.compiler.api.symbols.Qualifier.CLIENT;
+import static io.ballerina.compiler.api.symbols.Qualifier.DISTINCT;
+import static io.ballerina.compiler.api.symbols.Qualifier.ISOLATED;
+import static io.ballerina.compiler.api.symbols.Qualifier.SERVICE;
 import static io.ballerina.compiler.api.symbols.SymbolKind.ANNOTATION;
 import static io.ballerina.compiler.api.symbols.SymbolKind.CONSTANT;
 import static io.ballerina.compiler.api.symbols.SymbolKind.FUNCTION;
 import static io.ballerina.compiler.api.symbols.SymbolKind.MODULE;
 import static io.ballerina.compiler.api.symbols.SymbolKind.SERVICE_DECLARATION;
+import static io.ballerina.compiler.api.symbols.SymbolKind.TYPE;
 import static io.ballerina.compiler.api.symbols.SymbolKind.TYPE_DEFINITION;
 import static io.ballerina.compiler.api.symbols.SymbolKind.VARIABLE;
 import static io.ballerina.compiler.api.symbols.TypeDescKind.INT;
@@ -175,17 +180,15 @@ public void testSymbolLookupInBIR() {
 
         BallerinaModule fooModule = (BallerinaModule) symbolsInScope.stream()
                 .filter(sym -> sym.getName().get().equals("testproject")).findAny().get();
-        SemanticAPITestUtils.assertList(fooModule.functions(), List.of("loadHuman", 
+        SemanticAPITestUtils.assertList(fooModule.functions(), List.of("loadHuman",
                 "testAnonTypeDefSymbolsIsNotVisible", "add", "testFnA", "testFnB"));
         SemanticAPITestUtils.assertList(fooModule.constants(), List.of("RED", "GREEN", "BLUE", "PI", "TRUE", "FALSE"));
-
-        SemanticAPITestUtils.assertList(fooModule.typeDefinitions(), List.of("HumanObj", "ApplicationResponseError", 
-                "Person", "BasicType", "Digit", "FileNotFoundError", "EofError", "Error", "Pet", "Student", "Cat", 
-                "Annot", "Detail", "Service", "FnTypeA", "FnTypeB", "Address"));
-
-
         SemanticAPITestUtils.assertList(fooModule.classes(), List.of("PersonObj", "Dog", "EmployeeObj", "Human", 
                 "Client", "Response"));
+        SemanticAPITestUtils.assertList(fooModule.typeDefinitions(), List.of("HumanObj", "ApplicationResponseError",
+                "Person", "BasicType", "Digit", "FileNotFoundError", "EofError", "Error", "Pet", "Student", "Cat",
+                "Annot", "Detail", "Service", "FnTypeA", "FnTypeB", "Address", "InterceptorClient", 
+                "InterceptorService"));
         SemanticAPITestUtils.assertList(fooModule.enums(), List.of("Colour"));
 
         List<String> allSymbols = getSymbolNames(fooPkgSymbol, 0);
@@ -319,8 +322,8 @@ public static void assertList(List<Symbol> actualValues, List<SymbolInfo> expect
 
         for (SymbolInfo val : expectedValues) {
             assertTrue(actualValues.stream()
-                               .anyMatch(sym -> val.equals(new SymbolInfo(sym.getName().get(), sym.kind()))),
-                       "Symbol not found: " + val);
+                            .anyMatch(sym -> val.equals(new SymbolInfo(sym.getName().get(), sym.kind()))),
+                    "Symbol not found: " + val);
 
         }
     }
@@ -434,4 +437,31 @@ public void testIntersectionTypeDefSymbolPosBIR() {
         assertEquals(lineRange.endLine().line(), 147);
         assertEquals(lineRange.endLine().offset(), 2);
     }
+
+    @Test
+    public void testObjectTypeSymbolDefQualifiers() {
+        Project project = BCompileUtil.loadProject("test-src/object_symbol_qualifiers.bal");
+        Document srcFile = getDocumentForSingleSource(project);
+        SemanticModel model = getDefaultModulesSemanticModel(project);
+
+        Optional<Symbol> symbol = model.symbol(srcFile, from(19, 20));
+        assertTrue(symbol.isPresent());
+        Symbol sym = symbol.get();
+        assertEquals(sym.kind(), TYPE);
+        TypeSymbol typeSymbol = ((TypeReferenceTypeSymbol) sym).typeDescriptor();
+        assertEquals(typeSymbol.typeKind(), OBJECT);
+        ObjectTypeSymbol clientObjectTSymbol = (ObjectTypeSymbol) typeSymbol;
+        assertEquals(clientObjectTSymbol.qualifiers().size(), 3);
+        assertEquals(clientObjectTSymbol.qualifiers(), List.of(DISTINCT, ISOLATED, CLIENT));
+
+        symbol = model.symbol(srcFile, from(23, 23));
+        assertTrue(symbol.isPresent());
+        sym = symbol.get();
+        assertEquals(sym.kind(), TYPE);
+        typeSymbol = ((TypeReferenceTypeSymbol) sym).typeDescriptor();
+        assertEquals(typeSymbol.typeKind(), OBJECT);
+        ObjectTypeSymbol serviceObjectTSymbol = (ObjectTypeSymbol) typeSymbol;
+        assertEquals(serviceObjectTSymbol.qualifiers().size(), 3);
+        assertEquals(serviceObjectTSymbol.qualifiers(), List.of(DISTINCT, ISOLATED, SERVICE));
+    }
 }
diff --git a/tests/ballerina-compiler-api-test/src/test/resources/test-src/object_symbol_qualifiers.bal b/tests/ballerina-compiler-api-test/src/test/resources/test-src/object_symbol_qualifiers.bal
@@ -0,0 +1,25 @@
+// 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
+// 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 testorg/testproject;
+
+function testInterceptorClient() {
+    testproject:InterceptorClient ic;
+}
+
+function testInterceptorService() {
+    testproject:InterceptorService 'is;
+}
diff --git a/tests/ballerina-compiler-api-test/src/test/resources/test-src/testproject/type_defs.bal b/tests/ballerina-compiler-api-test/src/test/resources/test-src/testproject/type_defs.bal
@@ -146,3 +146,11 @@ public type Address readonly & record {
     string street;
     string city;
 };
+
+public type InterceptorClient distinct isolated client object {
+    isolated remote function execute() returns anydata|error;
+};
+
+public type InterceptorService distinct isolated service object {
+    isolated remote function execute() returns anydata|error;
+};
diff --git a/...allerina-unit-test/src/test/java/org/ballerinalang/test/bala/object/ObjectInBalaTest.java b/...allerina-unit-test/src/test/java/org/ballerinalang/test/bala/object/ObjectInBalaTest.java
@@ -41,6 +41,7 @@ public void setup() {
         BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project");
         BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project_two");
         BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project_utils");
+        BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/bir_test_project");
 
         result = BCompileUtil.compile("test-src/bala/test_bala/object/test_objects.bal");
     }
@@ -551,6 +552,31 @@ public void testObjectInclusionWithMethodWithParameters() {
         BRunUtil.invoke(result, "testObjectInclusionWithMethodWithParameters");
     }
 
+    @Test (description = "Negative test to verify object qualifiers load properly from BIR")
+    public void testDistinctIsolatedObjectsNegative() {
+        CompileResult result = BCompileUtil.compile("test-src/bala/test_bala/object/test_bir_negative.bal");
+        int i = 0;
+        BAssertUtil.validateError(result, i++, 
+                "incompatible types: expected 'bir/objs:0.1.0:Foo', found 'bir/objs:0.1.0:Bar'", 24, 18);
+        BAssertUtil.validateError(result, i++, 
+                "incompatible types: expected 'bir/objs:0.1.0:Bar', found 'bir/objs:0.1.0:Foo'", 28, 18);
+        BAssertUtil.validateError(result, i++,
+                "incompatible types: expected 'Foo', found 'bir/objs:0.1.0:Foo'", 29, 13);
+        BAssertUtil.validateError(result, i++, 
+                "incompatible types: expected 'bir/objs:0.1.0:Qux', found 'bir/objs:0.1.0:Xyz'", 33, 18);
+        BAssertUtil.validateError(result, i++, 
+                "incompatible types: expected 'bir/objs:0.1.0:Xyz', found 'bir/objs:0.1.0:Qux'", 37, 18);
+        BAssertUtil.validateError(result, i++,
+                "incompatible types: expected 'bir/objs:0.1.0:Foo', found 'Foo'", 41, 18);
+        Assert.assertEquals(result.getErrorCount(), i);
+    }
+
+    @Test(description = "Positive test to verify object isolated and distinct qualifiers load properly from BIR")
+    public void testObjectTypeAssignabilityWithQualifiers() {
+        CompileResult result = BCompileUtil.compile("test-src/bala/test_bala/object/test_bir_positive.bal");
+        BRunUtil.invoke(result, "testObjectTypeAssignabilityWithQualifiers");
+    }
+
     @AfterClass
     public void tearDown() {
         result = null;

diff --git a/...llerina-unit-test/src/test/resources/test-src/bala/test_bala/object/test_bir_negative.bal b/...llerina-unit-test/src/test/resources/test-src/bala/test_bala/object/test_bir_negative.bal
@@ -0,0 +1,42 @@
+// Copyright (c) 2023 WSO2 LLC. (http://www.wso2.org) 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 bir/objs;
+
+public type Foo distinct isolated client object {
+    public isolated function execute() returns anydata|error;
+};
+
+function f1(objs:Bar bar) {
+    objs:Foo _ = bar;
+}
+
+function f2(objs:Foo foo) {
+    objs:Bar _ = foo;
+    Foo _ = foo;
+}
+
+function f3(objs:Xyz xyz) {
+    objs:Qux _ = xyz;
+}
+
+function f4(objs:Qux qux) {
+    objs:Xyz _ = qux;
+}
+
+function f5(Foo foo) {
+    objs:Foo _ = foo;
+}