GH-129: Validation for duplicate oneof node names

Author: kshchepanovskyi

src/main/java/io/protostuff/jetbrains/plugin/annotator/ProtoErrorsAnnotator.java

@@ -82,6 +82,9 @@ public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder hold
                     checkInvalidFieldTags(fields);
                     checkDuplicateFieldTags(fields);
                     checkDuplicateFieldNames(fields);
+                    Collection<OneOfNode> oneOfNodes = message.getOneOfNodes();
+                    checkDuplicateOneofNames(oneOfNodes);
+                    checkDuplicateFieldOneofNames(oneOfNodes, fields);
                     checkReservedFieldTags(message, fields);
                     checkReservedFieldNames(message, fields);
                 }
@@ -387,7 +390,7 @@ private void checkDuplicateFieldTags(Collection<MessageField> fields) {
             int tag = field.getTag();
             if (fieldByTag.containsKey(tag)) {
                 String message = message("error.duplicate.field.tag", tag);
-                markError(field.getNode(), fieldByTag.get(tag).getTagNode(), message);
+                markError(fieldByTag.get(tag).getNode(), fieldByTag.get(tag).getTagNode(), message);
                 markError(field.getNode(), field.getTagNode(), message);
             }
             fieldByTag.put(tag, field);
@@ -400,10 +403,39 @@ private void checkDuplicateFieldNames(Collection<MessageField> fields) {
             String name = field.getFieldName();
             if (fieldByName.containsKey(name)) {
                 String message = message("error.duplicate.field.name", name);
-                markError(field.getNode(), fieldByName.get(name).getFieldNameNode(), message);
+                markError(fieldByName.get(name).getNode(), fieldByName.get(name).getFieldNameNode(), message);
                 markError(field.getNode(), field.getFieldNameNode(), message);
             }
             fieldByName.put(name, field);
         }
     }
+
+    private void checkDuplicateOneofNames(Collection<OneOfNode> fields) {
+        Map<String, OneOfNode> fieldByName = new HashMap<>();
+        for (OneOfNode oneOfNode : fields) {
+            String name = oneOfNode.getName();
+            if (fieldByName.containsKey(name)) {
+                String message = message("error.duplicate.name", name);
+                markError(fieldByName.get(name).getNode(), fieldByName.get(name).getOneofNameNode(), message);
+                markError(oneOfNode.getNode(), oneOfNode.getOneofNameNode(), message);
+            }
+            fieldByName.put(name, oneOfNode);
+        }
+    }
+
+    private void checkDuplicateFieldOneofNames(Collection<OneOfNode> oneOfNodes, Collection<MessageField> fields) {
+        Map<String, MessageField> fieldByName = new HashMap<>();
+        for (MessageField field : fields) {
+            String name = field.getFieldName();
+            fieldByName.put(name, field);
+        }
+        for (OneOfNode oneOfNode : oneOfNodes) {
+            String name = oneOfNode.getName();
+            if (fieldByName.containsKey(name)) {
+                String message = message("error.duplicate.name", name);
+                markError(fieldByName.get(name).getNode(), fieldByName.get(name).getFieldNameNode(), message);
+                markError(oneOfNode.getNode(), oneOfNode.getOneofNameNode(), message);
+            }
+        }
+    }
 }

src/main/java/io/protostuff/jetbrains/plugin/psi/MessageNode.java

@@ -110,4 +110,8 @@ public boolean hasSyntaxErrors() {
         return syntaxErrors;
     }
 
+    public Collection<OneOfNode> getOneOfNodes() {
+        OneOfNode[] oneOfs = findChildrenByClass(OneOfNode.class);
+        return Arrays.asList(oneOfs);
+    }
 }

src/main/java/io/protostuff/jetbrains/plugin/psi/OneOfNode.java

@@ -25,4 +25,15 @@ public Collection<MessageField> getFields() {
     public DataType[] getDeclaredDataTypes() {
         return findChildrenByClass(DataType.class);
     }
+
+    /**
+     * Returns name node, if present, null otherwise.
+     */
+    public ASTNode getOneofNameNode() {
+        GenericNameNode nameIdentifier = getNameIdentifier();
+        if (nameIdentifier == null) {
+            return null;
+        }
+        return nameIdentifier.getNode();
+    }
 }

src/main/resources/io/protostuff/protobuf-jetbrains-plugin/messages/ProtostuffBundle.properties

@@ -33,3 +33,5 @@ error.illegal.oneof.field.label=Oneof field cannot have label
 error.unresolved.reference=Unresolved reference
 error.file.not.found=File not found
 error.expected.identifier=Identifier expected
+error.duplicate.name=Duplicate name: {0}
+error.duplicate.value=Duplicate value: {0}

src/test/java/io/protostuff/jetbrains/plugin/annotator/ProtoErrorsAnnotatorTest.java

@@ -30,6 +30,14 @@ public void testDuplicateFieldName() {
         check();
     }
 
+    public void testDuplicateFieldName_asOneofName() {
+        check();
+    }
+
+    public void testDuplicateOneofName() {
+        check();
+    }
+
     public void testDuplicateTagValue() {
         check();
     }

src/test/resources/annotator/DuplicateFieldName_asOneofName.proto

@@ -0,0 +1,9 @@
+syntax = "proto3";
+
+package annotator;
+
+message TestMessage {
+    oneof <error descr="Duplicate name: value">value</error> {
+        int32 <error descr="Duplicate name: value">value</error> = 1;
+    }
+}

src/test/resources/annotator/DuplicateOneofName.proto

@@ -0,0 +1,10 @@
+syntax = "proto3";
+
+package annotator;
+
+message TestMessage {
+    oneof <error descr="Duplicate name: value">value</error> {
+    }
+    oneof <error descr="Duplicate name: value">value</error> {
+    }
+}