Merge pull request #29 from prakanth97/issue_6705

Handle attribute and element with same name in the same scope

ballerina/Ballerina.toml

@@ -1,7 +1,7 @@
 [package]
 org = "ballerina"
 name = "data.xmldata"
-version = "0.1.2"
+version = "0.1.3"
 authors = ["Ballerina"]
 keywords = ["xml"]
 repository = "https://github.com/ballerina-platform/module-ballerina-data-xmldata"
@@ -12,5 +12,5 @@ export = ["data.xmldata"]
 [[platform.java17.dependency]]
 groupId = "io.ballerina.lib"
 artifactId = "data-native"
-version = "0.1.2"
-path = "../native/build/libs/data.xmldata-native-0.1.2.jar"
+version = "0.1.3"
+path = "../native/build/libs/data.xmldata-native-0.1.3-SNAPSHOT.jar"

ballerina/CompilerPlugin.toml

@@ -3,4 +3,4 @@ id = "constraint-compiler-plugin"
 class = "io.ballerina.lib.data.xmldata.compiler.XmldataCompilerPlugin"
 
 [[dependency]]
-path = "../compiler-plugin/build/libs/data.xmldata-compiler-plugin-0.1.2.jar"
+path = "../compiler-plugin/build/libs/data.xmldata-compiler-plugin-0.1.3-SNAPSHOT.jar"

ballerina/Dependencies.toml

@@ -10,7 +10,7 @@ distribution-version = "2201.9.0"
 [[package]]
 org = "ballerina"
 name = "data.xmldata"
-version = "0.1.2"
+version = "0.1.3"
 dependencies = [
 	{org = "ballerina", name = "io"},
 	{org = "ballerina", name = "jballerina.java"},

ballerina/tests/fromXml_test.bal

@@ -2812,6 +2812,94 @@ function testProjectionWithXmlAttributeForParseAsType() returns error? {
     test:assertEquals(rec.A, "2");
 }
 
+type RecType1 record {
+    string name;
+    @Name {
+        value: "name"
+    }
+    @Attribute
+    string duplicateName;
+};
+
+type RecType2 record {
+    record {|
+        string \#content;
+    |} name;
+    @Name {
+        value: "name"
+    }
+    @Attribute
+    string duplicateName;
+};
+
+@test:Config
+isolated function testElementAndAttributeInSameScopeHaveSameName() returns error? {
+    string xmlStr = string `
+    <Data name="Kevin">
+        <name>Kanth</name>
+    </Data>
+    `; 
+    RecType1 rec11 = check parseString(xmlStr);
+    test:assertEquals(rec11.name, "Kanth");
+    test:assertEquals(rec11.duplicateName, "Kevin");
+
+    RecType2 rec12 = check parseString(xmlStr);
+    test:assertEquals(rec12.name.\#content, "Kanth");
+    test:assertEquals(rec12.duplicateName, "Kevin");  
+
+    xml xmlVal = xml `
+    <Data name="Kevin">
+        <name>Kanth</name>
+    </Data>
+    `;
+    RecType1 rec21 = check parseAsType(xmlVal);
+    test:assertEquals(rec21.name, "Kanth");
+    test:assertEquals(rec21.duplicateName, "Kevin");
+
+    RecType2 rec22 = check parseAsType(xmlVal);
+    test:assertEquals(rec22.name.\#content, "Kanth");
+    test:assertEquals(rec22.duplicateName, "Kevin");  
+}
+
+type RecNs3 record {|
+    @Namespace {
+        prefix: "ns1",
+        uri: "example1.com"
+    }
+    string name;
+    @Name {
+        value: "name"
+    }
+    @Namespace {
+        prefix: "ns2",
+        uri: "example2.com"
+    }
+    string duplicateName;
+|};
+
+@test:Config
+isolated function testElementWithDifferentNamespace() returns error? {
+    string xmlStr = string `
+    <Data xmlns:ns1="example1.com" xmlns:ns2="example2.com">
+        <ns1:name>Kevin</ns1:name>
+        <ns2:name>Kanth</ns2:name>
+    </Data>
+    `;
+    RecNs3 rec = check parseString(xmlStr);
+    test:assertEquals(rec.name, "Kevin");
+    test:assertEquals(rec.duplicateName, "Kanth");
+
+    xml xmlVal = xml `
+    <Data xmlns:ns1="example1.com" xmlns:ns2="example2.com">
+        <ns1:name>Kevin</ns1:name>
+        <ns2:name>Kanth</ns2:name>
+    </Data>
+    `;
+    RecNs3 rec2 = check parseAsType(xmlVal);
+    test:assertEquals(rec2.name, "Kevin");
+    test:assertEquals(rec2.duplicateName, "Kanth");
+}
+
 // Negative cases
 type DataN1 record {|
     int A;
@@ -3240,3 +3328,62 @@ function testInvalidNamespaceInOpenRecordForParseAsType2() {
     test:assertTrue(err is error);
     test:assertEquals((<error>err).message(), "undefined field 'name' in record 'data.xmldata:AuthorOpen'");
 }
+
+type RecTypeDup1 record {
+    string name;
+    @Name {
+        value: "name"
+    }
+    string duplicateName;
+};
+
+type RecTypeDup2 record {
+    @Namespace {
+        prefix: "ns",
+        uri: "example.com"
+    }
+    string name;
+    @Name {
+        value: "name"
+    }
+    string duplicateName;
+};
+
+@test:Config
+isolated function testDuplicateField() {
+    string xmlStr = string `
+    <Data name="Kevin">
+        <name>Kanth</name>
+    </Data>
+    `;
+    RecTypeDup1|Error err = parseString(xmlStr);
+    test:assertTrue(err is Error);
+    test:assertEquals((<Error> err).message(), "duplicate field 'name'");
+
+    xml xmlVal = xml `
+    <Data name="Kevin">
+        <name>Kanth</name>
+    </Data>
+    `;
+    RecTypeDup1|Error err2 = parseAsType(xmlVal);
+    test:assertTrue(err2 is Error);
+    test:assertEquals((<Error> err2).message(), "duplicate field 'name'");
+
+    string xmlStr2 = string `
+    <Data name="Kevin">
+        <name>Kanth</name>
+    </Data>
+    `;
+    RecTypeDup2|Error err3 = parseString(xmlStr2);
+    test:assertTrue(err3 is Error);
+    test:assertEquals((<Error> err3).message(), "duplicate field 'name'");
+
+    xml xmlVal2 = xml `
+    <Data name="Kevin">
+        <name>Kanth</name>
+    </Data>
+    `;
+    RecTypeDup2|Error err4 = parseAsType(xmlVal2);
+    test:assertTrue(err4 is Error);
+    test:assertEquals((<Error> err4).message(), "duplicate field 'name'");
+}

ballerina/tests/toXml_test.bal

@@ -1191,7 +1191,7 @@ type Soap1 record {
 };
 
 @test:Config {
-    groups: ["toXml", "testFail"]
+    groups: ["toXml"]
 }
 isolated function testXmlToRecordWithNamespaceAttachedToFields() returns error? {
     Soap1 val = {

compiler-plugin/src/main/java/io/ballerina/lib/data/xmldata/compiler/Constants.java

@@ -13,6 +13,7 @@ public class Constants {
     static final String TO_XML = "toXml";
     static final String NAME = "Name";
     static final String NAMESPACE = "Namespace";
+    static final String ATTRIBUTE = "Attribute";
     static final String XMLDATA = "xmldata";
     static final String BALLERINA = "ballerina";
     static final String DATA_XMLDATA = "data.xmldata";

compiler-plugin/src/main/java/io/ballerina/lib/data/xmldata/compiler/XmldataRecordFieldValidator.java

@@ -83,7 +83,7 @@ public void perform(SyntaxNodeAnalysisContext ctx) {
         boolean erroneousCompilation = diagnostics.stream()
                 .anyMatch(d -> d.diagnosticInfo().severity().equals(DiagnosticSeverity.ERROR));
         if (erroneousCompilation) {
-            rest();
+            reset();
             return;
         }
 
@@ -99,10 +99,10 @@ public void perform(SyntaxNodeAnalysisContext ctx) {
             }
         }
 
-        rest();
+        reset();
     }
 
-    private void rest() {
+    private void reset() {
         semanticModel = null;
         allDiagnosticInfo.clear();
         modulePrefix = Constants.XMLDATA;
@@ -376,6 +376,7 @@ private QualifiedName getQNameFromAnnotation(String fieldName,
         String uri = "";
         String name = fieldName;
         String prefix = "";
+        boolean isAttribute = false;
         for (AnnotationAttachmentSymbol annotAttSymbol : annotationAttachments) {
             AnnotationSymbol annotation = annotAttSymbol.typeDescriptor();
             if (!getAnnotModuleName(annotation).contains(Constants.XMLDATA)) {
@@ -397,9 +398,11 @@ private QualifiedName getQNameFromAnnotation(String fieldName,
                 }
                 uri = ((LinkedHashMap<?, ?>) annotAttSymbol.attachmentValue().orElseThrow().value())
                         .get("uri").toString();
+            } else if (value.equals(Constants.ATTRIBUTE)) {
+                isAttribute = true;
             }
         }
-        return new QualifiedName(uri, name, prefix);
+        return new QualifiedName(uri, name, prefix, isAttribute);
     }
 
     private String getAnnotModuleName(AnnotationSymbol annotation) {

compiler-plugin/src/main/java/io/ballerina/lib/data/xmldata/compiler/objects/QualifiedName.java

@@ -22,11 +22,13 @@ public class QualifiedName {
     private final String localPart;
     private final String namespaceURI;
     private final String prefix;
+    private boolean isAttributeDefined;
 
-    public QualifiedName(String namespaceURI, String localPart, String prefix) {
+    public QualifiedName(String namespaceURI, String localPart, String prefix, boolean isAttributeDefined) {
         this.localPart = localPart;
         this.namespaceURI = namespaceURI;
         this.prefix = prefix;
+        this.isAttributeDefined = isAttributeDefined;
     }
 
     @Override
@@ -46,6 +48,6 @@ public boolean equals(Object objectToTest) {
 
         QualifiedName qName = (QualifiedName) objectToTest;
         return localPart.equals(qName.localPart) && namespaceURI.equals(qName.namespaceURI) &&
-                prefix.equals(qName.prefix);
+                prefix.equals(qName.prefix) && (isAttributeDefined == qName.isAttributeDefined);
     }
 }

native/src/main/java/io/ballerina/lib/data/xmldata/utils/DataUtils.java

@@ -50,6 +50,10 @@
 
 import javax.xml.namespace.QName;
 
+import static io.ballerina.lib.data.xmldata.xml.QualifiedName.AttributeState.ATTRIBUTE;
+import static io.ballerina.lib.data.xmldata.xml.QualifiedName.AttributeState.ELEMENT;
+import static io.ballerina.lib.data.xmldata.xml.QualifiedName.AttributeState.NOT_DEFINED;
+
 /**
  * A util class for the Data package's native implementation.
  *
@@ -137,24 +141,29 @@ public static Map<QualifiedName, Field> getAllFieldsInRecordType(RecordType reco
         Map<String, List<QualifiedName>> fieldNames = new HashMap<>();
         Map<String, Field> recordFields = recordType.getFields();
         for (String key : recordFields.keySet()) {
+            QualifiedNameMap<Field> attributeMap = analyzerData.attributeHierarchy.peek();
             QualifiedName modifiedQName =
                     modifiedNames.getOrDefault(key, new QualifiedName(Constants.NS_ANNOT_NOT_DEFINED, key, ""));
             String localName = modifiedQName.getLocalPart();
-            if (fieldMap.containsKey(modifiedQName)) {
+            if (attributeMap.contains(modifiedQName) && modifiedQName.getAttributeState() == NOT_DEFINED) {
+                if (!key.equals(attributeMap.get(modifiedQName).getFieldName())) {
+                    modifiedQName.setAttributeState(ELEMENT);
+                    fieldMap.put(modifiedQName, recordFields.get(key));
+                    fieldNames.put(localName, new ArrayList<>(List.of(modifiedQName)));
+                }
+            } else if (fieldMap.containsKey(modifiedQName)) {
                 throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, localName);
             } else if (fieldNames.containsKey(localName)) {
-                if (modifiedQName.getNamespaceURI().equals(Constants.NS_ANNOT_NOT_DEFINED)) {
-                    throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, localName);
-                }
                 List<QualifiedName> qNames = fieldNames.get(localName);
                 qNames.forEach(qName -> {
-                    if (qName.getNamespaceURI().equals(Constants.NS_ANNOT_NOT_DEFINED)) {
+                    if (DataUtils.isSameAttributeFlag(qName.getAttributeState(), modifiedQName.getAttributeState())
+                            && DataUtils.isSameNamespace(qName, modifiedQName)) {
                         throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, localName);
                     }
                 });
                 fieldMap.put(modifiedQName, recordFields.get(key));
                 fieldNames.get(localName).add(modifiedQName);
-            } else if (!analyzerData.attributeHierarchy.peek().contains(modifiedQName)) {
+            } else if (!attributeMap.contains(modifiedQName)) {
                 fieldMap.put(modifiedQName, recordFields.get(key));
                 fieldNames.put(localName, new ArrayList<>(List.of(modifiedQName)));
             }
@@ -172,6 +181,7 @@ public static Map<QualifiedName, Field> getAllAttributesInRecordType(RecordType
                 String attributeName = keyStr.split(Constants.FIELD_REGEX)[1].replaceAll("\\\\", "");
                 Map<BString, Object> fieldAnnotation = (Map<BString, Object>) annotations.get(annotationKey);
                 QualifiedName fieldQName = getFieldNameFromRecord(fieldAnnotation, attributeName);
+                fieldQName.setAttributeState(ATTRIBUTE);
                 fieldQName.setLocalPart(getModifiedName(fieldAnnotation, attributeName));
                 attributes.put(fieldQName, recordType.getFields().get(attributeName));
             }
@@ -365,6 +375,17 @@ public static void logArrayMismatchErrorIfProjectionNotAllowed(boolean allowData
         throw DiagnosticLog.error(DiagnosticErrorCode.ARRAY_SIZE_MISMATCH);
     }
 
+    public static boolean isSameNamespace(QualifiedName q1, QualifiedName q2) {
+        String ns1 = q1.getNamespaceURI();
+        String ns2 = q2.getNamespaceURI();
+        return  (ns1.equals(ns2) && q1.getPrefix().equals(q2.getPrefix()))
+                || ns1.equals(Constants.NS_ANNOT_NOT_DEFINED) || ns2.equals(Constants.NS_ANNOT_NOT_DEFINED);
+    }
+
+    public static boolean isSameAttributeFlag(QualifiedName.AttributeState flag1, QualifiedName.AttributeState flag2) {
+        return flag1 == NOT_DEFINED || flag2 == NOT_DEFINED || flag1.equals(flag2);
+    }
+
     @SuppressWarnings("unchecked")
     public static Object getModifiedRecord(BMap<BString, Object> input, BString textFieldName, BTypedesc type) {
         Type describingType = type.getDescribingType();

native/src/main/java/io/ballerina/lib/data/xmldata/xml/QualifiedName.java

@@ -27,6 +27,20 @@ public class QualifiedName {
     private String localPart;
     private String namespaceURI;
     private String prefix;
+    private AttributeState attributeState = AttributeState.NOT_DEFINED;
+
+    public enum AttributeState {
+        ATTRIBUTE,
+        ELEMENT,
+        NOT_DEFINED
+    }
+
+    public QualifiedName(String namespaceURI, String localPart, String prefix, AttributeState attributeState) {
+        this.localPart = localPart;
+        this.namespaceURI = namespaceURI;
+        this.prefix = prefix;
+        this.attributeState = attributeState;
+    }
 
     public QualifiedName(String namespaceURI, String localPart, String prefix) {
         this.localPart = localPart;
@@ -56,9 +70,17 @@ public String getPrefix() {
         return prefix;
     }
 
+    public void setAttributeState(AttributeState attributeState) {
+        this.attributeState = attributeState;
+    }
+
+    public AttributeState getAttributeState() {
+        return this.attributeState;
+    }
+
     @Override
     public int hashCode() {
-        return prefix.hashCode() ^ namespaceURI.hashCode() ^ localPart.hashCode();
+        return prefix.hashCode() ^ namespaceURI.hashCode() ^ localPart.hashCode() ^ attributeState.hashCode();
     }
 
     @Override
@@ -72,6 +94,6 @@ public boolean equals(Object objectToTest) {
         }
 
         return localPart.equals(qName.localPart) && namespaceURI.equals(qName.namespaceURI) &&
-                prefix.equals(qName.prefix);
+                prefix.equals(qName.prefix) && attributeState.equals(qName.attributeState);
     }
 }