From 8ebcd998f7df3175ca9bea15708e45102ddf9353 Mon Sep 17 00:00:00 2001 From: Kalaiyarasiganeshalingam Date: Thu, 10 Aug 2023 15:14:17 +0530 Subject: [PATCH 1/4] Fix NPE from the compiler plugin --- .../xmldata/compiler/CompilerPluginTest.java | 5 ++ .../resources/diagnostics/sample11/main.bal | 57 +++++++------------ .../compiler/XmldataRecordFieldValidator.java | 17 +++--- 3 files changed, 34 insertions(+), 45 deletions(-) diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java index 2c8d36fa..b814423f 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java @@ -29,8 +29,10 @@ import org.testng.Assert; import org.testng.annotations.Test; +import java.io.PrintStream; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -165,6 +167,9 @@ public void testInvalidChildAnnotation() { List errorDiagnosticsList = diagnosticResult.diagnostics().stream() .filter(r -> r.diagnosticInfo().severity().equals(DiagnosticSeverity.ERROR)) .collect(Collectors.toList()); + PrintStream asd = System.out; + asd.println(errorDiagnosticsList.size()); + asd.println(Arrays.toString(errorDiagnosticsList.toArray())); Assert.assertEquals(errorDiagnosticsList.size(), 1); Assert.assertEquals(errorDiagnosticsList.get(0).diagnosticInfo().messageFormat(), "invalid annotation attachment: child record does not allow name annotation"); diff --git a/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal b/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal index 7232ae1d..06f39515 100644 --- a/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal +++ b/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal @@ -1,50 +1,31 @@ -// 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 ballerina/xmldata; +import ballerina/io; @xmldata:Name { - value: "Foo1" -} -type Foo record { - Bar foo; -}; - -@xmldata:Name { - value: "Bar1" + value: "appointment" } -type Bar record { - int bar; - Bar2 bar2; +type Appointment record { + string firstName; + string lastName; + string email; + string age; }; @xmldata:Name { - value: "Bar4" -} -@xmldata:Namespace { - prefix: "ns", - uri: "http://sdf.com" + value: "appointments" } -type Bar2 record { - int bar; - string car; +type Appointments record { + Appointment[] appointment; }; public function main() returns error? { - xml x1 = xml `2`; - Foo actual = check xmldata:fromXml(x1); - Bar result = check xmldata:fromXml(x1); + xml xmlPayload = xml `JohnDoejohn.doe@gmail.com28JohnDoejohn.doe@gmail.com28`; + + // Works okay + map result2 = check xmldata:fromXml(xmlPayload); + io:println(result2); + + // Doesn't work. Failed with "error: The record type name: AppointmentPayload mismatch with given XML name: appointments" + Appointments result3 = check xmldata:fromXml(xmlPayload); + io:println(result3); } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/xmldata/compiler/XmldataRecordFieldValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/xmldata/compiler/XmldataRecordFieldValidator.java index 8e761d2b..c9e918d2 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/xmldata/compiler/XmldataRecordFieldValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/xmldata/compiler/XmldataRecordFieldValidator.java @@ -86,7 +86,9 @@ public void perform(SyntaxNodeAnalysisContext ctx) { } } for (String recordName : this.recordNamesUsedInFunction) { - validateRecord(ctx, this.records.get(recordName)); + if (this.records.containsKey(recordName)) { + validateRecord(ctx, this.records.get(recordName)); + } } } @@ -129,7 +131,7 @@ private boolean isValidFunctionName(ExpressionNode expressionNode) { private void addRecordName(TypeDescriptorNode typeDescriptor) { if (typeDescriptor.kind() == SyntaxKind.SIMPLE_NAME_REFERENCE) { - String returnTypeName = typeDescriptor.toString().trim(); + String returnTypeName = ((SimpleNameReferenceNode) typeDescriptor).name().text().trim(); if (!this.recordNamesUsedInFunction.contains(returnTypeName)) { this.recordNamesUsedInFunction.add(returnTypeName); } @@ -140,7 +142,7 @@ private void processTypeDefinitionNode(TypeDefinitionNode typeDefinitionNode) { Node typeDescriptor = typeDefinitionNode.typeDescriptor(); if (typeDescriptor instanceof RecordTypeDescriptorNode) { RecordTypeDescriptorNode recordTypeDescriptorNode = (RecordTypeDescriptorNode) typeDescriptor; - Record record = new Record(typeDefinitionNode.typeName().text(), typeDefinitionNode.location()); + Record record = new Record(typeDefinitionNode.typeName().text().trim(), typeDefinitionNode.location()); typeDefinitionNode.metadata().ifPresent(metadataNode -> { NodeList annotations = metadataNode.annotations(); for (AnnotationNode annotationNode : annotations) { @@ -154,13 +156,14 @@ private void processTypeDefinitionNode(TypeDefinitionNode typeDefinitionNode) { if (field instanceof RecordFieldNode) { RecordFieldNode recordFieldNode = (RecordFieldNode) field; type = recordFieldNode.typeName(); - } else { + processFieldType(type, record); + } else if (field instanceof RecordFieldWithDefaultValueNode) { RecordFieldWithDefaultValueNode recordFieldNode = (RecordFieldWithDefaultValueNode) field; type = recordFieldNode.typeName(); + processFieldType(type, record); } - processFieldType(type, record); } - this.records.put(record.getName(), record); + this.records.put(record.getName().trim(), record); } } @@ -223,7 +226,7 @@ private void validateRecord(SyntaxNodeAnalysisContext ctx, Record record) { if (!this.validatedRecords.contains(childRecordName)) { Record childRecord = this.records.get(childRecordName); validateRecord(ctx, childRecord); - if (childRecord.hasNameAnnotation() && !recordNamesUsedInFunction.contains(childRecordName)) { + if (childRecord.hasNameAnnotation() && !recordNamesUsedInFunction.contains(childRecordName.trim())) { reportDiagnosticInfo(ctx, childRecord.getLocation(), DiagnosticsCodes.XMLDATA_103); } } From e27a022e3ab23953ac925c67eeafb84d3a1dee34 Mon Sep 17 00:00:00 2001 From: Kalaiyarasiganeshalingam Date: Thu, 10 Aug 2023 15:20:11 +0530 Subject: [PATCH 2/4] Add new test case --- .../xmldata/compiler/CompilerPluginTest.java | 16 +++-- .../resources/diagnostics/sample11/main.bal | 61 ++++++++++++------- .../diagnostics/sample12/Ballerina.toml | 4 ++ .../resources/diagnostics/sample12/main.bal | 45 ++++++++++++++ 4 files changed, 100 insertions(+), 26 deletions(-) create mode 100644 compiler-plugin-tests/src/test/resources/diagnostics/sample12/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java index b814423f..44bd2263 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/xmldata/compiler/CompilerPluginTest.java @@ -29,10 +29,8 @@ import org.testng.Assert; import org.testng.annotations.Test; -import java.io.PrintStream; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -167,9 +165,17 @@ public void testInvalidChildAnnotation() { List errorDiagnosticsList = diagnosticResult.diagnostics().stream() .filter(r -> r.diagnosticInfo().severity().equals(DiagnosticSeverity.ERROR)) .collect(Collectors.toList()); - PrintStream asd = System.out; - asd.println(errorDiagnosticsList.size()); - asd.println(Arrays.toString(errorDiagnosticsList.toArray())); + Assert.assertEquals(errorDiagnosticsList.size(), 1); + Assert.assertEquals(errorDiagnosticsList.get(0).diagnosticInfo().messageFormat(), + "invalid annotation attachment: child record does not allow name annotation"); + } + + @Test + public void testInvalidChildAnnotation1() { + DiagnosticResult diagnosticResult = loadPackage("sample12").getCompilation().diagnosticResult(); + List errorDiagnosticsList = diagnosticResult.diagnostics().stream() + .filter(r -> r.diagnosticInfo().severity().equals(DiagnosticSeverity.ERROR)) + .collect(Collectors.toList()); Assert.assertEquals(errorDiagnosticsList.size(), 1); Assert.assertEquals(errorDiagnosticsList.get(0).diagnosticInfo().messageFormat(), "invalid annotation attachment: child record does not allow name annotation"); diff --git a/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal b/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal index 06f39515..5c2ad8ed 100644 --- a/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal +++ b/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal @@ -1,31 +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. + import ballerina/xmldata; -import ballerina/io; -@xmldata:Name { - value: "appointment" +@xmldata:Name{ + value: "Foo1" } -type Appointment record { - string firstName; - string lastName; - string email; - string age; +type Foo record { + Bar foo; }; -@xmldata:Name { - value: "appointments" +@xmldata:Name{ + value: "Bar1" } -type Appointments record { - Appointment[] appointment; +type Bar record { + int bar; + Bar2 bar2; }; -public function main() returns error? { - xml xmlPayload = xml `JohnDoejohn.doe@gmail.com28JohnDoejohn.doe@gmail.com28`; - - // Works okay - map result2 = check xmldata:fromXml(xmlPayload); - io:println(result2); +@xmldata:Name{ + value: "Bar4" +} +@xmldata:Namespace { + prefix: "ns", + uri: "http://sdf.com" +} +type Bar2 record { + int bar; + string car; +}; - // Doesn't work. Failed with "error: The record type name: AppointmentPayload mismatch with given XML name: appointments" - Appointments result3 = check xmldata:fromXml(xmlPayload); - io:println(result3); +public function main() returns error? { + xml x1 = xml `2`; + Foo actual = check xmldata:fromXml(x1); + Bar result = check xmldata:fromXml(x1); } diff --git a/compiler-plugin-tests/src/test/resources/diagnostics/sample12/Ballerina.toml b/compiler-plugin-tests/src/test/resources/diagnostics/sample12/Ballerina.toml new file mode 100644 index 00000000..ed3dd8a3 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/diagnostics/sample12/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "xmldata_test" +name = "sample12" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal b/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal new file mode 100644 index 00000000..a1fc3133 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal @@ -0,0 +1,45 @@ +// 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 ballerina/xmldata; +import ballerina/io; + +@xmldata:Name { + value: "appointment" +} +type Appointment record { + string firstName; + string lastName; + string email; + string age; +}; + +@xmldata:Name { + value: "appointments" +} +type Appointments record { + Appointment[] appointment; +}; + +public function main() returns error? { + xml xmlPayload = xml `JohnDoejohn.doe@gmail.com28JohnDoejohn.doe@gmail.com28`; + + map result2 = check xmldata:fromXml(xmlPayload); + io:println(result2); + + Appointments result3 = check xmldata:fromXml(xmlPayload); + io:println(result3); +} From b647022d518330f34f5f337525b93e0ea5866938 Mon Sep 17 00:00:00 2001 From: Kalaiyarasiganeshalingam Date: Thu, 10 Aug 2023 15:21:42 +0530 Subject: [PATCH 3/4] Undo the format changes --- .../src/test/resources/diagnostics/sample11/main.bal | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal b/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal index 5c2ad8ed..7232ae1d 100644 --- a/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal +++ b/compiler-plugin-tests/src/test/resources/diagnostics/sample11/main.bal @@ -16,14 +16,14 @@ import ballerina/xmldata; -@xmldata:Name{ +@xmldata:Name { value: "Foo1" } type Foo record { Bar foo; }; -@xmldata:Name{ +@xmldata:Name { value: "Bar1" } type Bar record { @@ -31,7 +31,7 @@ type Bar record { Bar2 bar2; }; -@xmldata:Name{ +@xmldata:Name { value: "Bar4" } @xmldata:Namespace { From 150232b0cf4decc3f0cf89b4c976f973fc55b571 Mon Sep 17 00:00:00 2001 From: Kalaiyarasiganeshalingam Date: Thu, 10 Aug 2023 15:27:05 +0530 Subject: [PATCH 4/4] Update the sample --- .../src/test/resources/diagnostics/sample12/main.bal | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal b/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal index a1fc3133..fd255a75 100644 --- a/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal +++ b/compiler-plugin-tests/src/test/resources/diagnostics/sample12/main.bal @@ -40,6 +40,7 @@ public function main() returns error? { map result2 = check xmldata:fromXml(xmlPayload); io:println(result2); + // Sample Appointments result3 = check xmldata:fromXml(xmlPayload); io:println(result3); }