#39: Validate field labels for proto2 and proto3

Author: kshchepanovskyi

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

@@ -13,6 +13,8 @@
 import io.protostuff.jetbrains.plugin.psi.AntlrParserRuleNode;
 import io.protostuff.jetbrains.plugin.psi.EnumConstantNode;
 import io.protostuff.jetbrains.plugin.psi.EnumNode;
+import io.protostuff.jetbrains.plugin.psi.FieldLabel;
+import io.protostuff.jetbrains.plugin.psi.FieldNode;
 import io.protostuff.jetbrains.plugin.psi.MessageField;
 import io.protostuff.jetbrains.plugin.psi.MessageNode;
 import io.protostuff.jetbrains.plugin.psi.ProtoRootNode;
@@ -24,6 +26,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -58,6 +61,8 @@ public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder hold
                     checkDuplicateFieldNames(fields);
                     checkReservedFieldTags(message, fields);
                     checkReservedFieldNames(message, fields);
+                } else if (element instanceof FieldNode) {
+                    checkFieldLabel((FieldNode) element, syntax);
                 } else if (element instanceof EnumNode) {
                     EnumNode anEnum = (EnumNode) element;
                     List<EnumConstantNode> constants = anEnum.getConstants();
@@ -73,6 +78,38 @@ public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder hold
         }
     }
 
+    private void checkFieldLabel(FieldNode field, Syntax syntax) {
+        switch (syntax) {
+            case PROTO2:
+                checkFieldLabelProto2(field);
+                break;
+            case PROTO3:
+                checkFieldLabelProto3(field);
+                break;
+            default:
+                throw new IllegalStateException(String.valueOf(syntax));
+        }
+    }
+
+    private void checkFieldLabelProto2(FieldNode field) {
+        ASTNode fieldLabelNode = field.getFieldLabelNode();
+        if (fieldLabelNode == null) {
+            String message = message("error.missing.field.label");
+            markError(field.getNode(), null, message);
+        }
+    }
+
+    private void checkFieldLabelProto3(FieldNode field) {
+        Optional<FieldLabel> fieldLabel = field.getFieldLabel();
+        fieldLabel.ifPresent(label -> {
+            if (label == FieldLabel.OPTIONAL
+                    || label == FieldLabel.REQUIRED) {
+                String message = message("error.illegal.field.label", label.getName());
+                markError(field.getFieldLabelNode(), null, message);
+            }
+        });
+    }
+
     private ProtoRootNode getProtoRoot(PsiElement element) {
         PsiElement tmp = element;
         while (tmp != null && !(tmp instanceof ProtoRootNode)) {

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

@@ -0,0 +1,36 @@
+package io.protostuff.jetbrains.plugin.psi;
+
+import java.util.Optional;
+
+/**
+ * Field label.
+ *
+ * @author Kostiantyn Shchepanovskyi
+ */
+public enum FieldLabel {
+    OPTIONAL("optional"),
+    REQUIRED("required"),
+    REPEATED("repeated");
+
+    private final String name;
+
+    FieldLabel(String name) {
+        this.name = name;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    /**
+     * Get field label by name.
+     */
+    public static Optional<FieldLabel> forString(String name) {
+        for (FieldLabel fieldLabel : FieldLabel.values()) {
+            if (fieldLabel.getName().equals(name)) {
+                return Optional.of(fieldLabel);
+            }
+        }
+        return Optional.empty();
+    }
+}

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

@@ -12,6 +12,7 @@
 import io.protostuff.jetbrains.plugin.ProtoParserDefinition;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Optional;
 import org.antlr.jetbrains.adapter.psi.IdentifierDefSubtree;
 import org.jetbrains.annotations.NotNull;
 
@@ -70,6 +71,21 @@ public ASTNode getTagNode() {
         return node.findChildByType(R_TAG);
     }
 
+    @Override
+    public Optional<FieldLabel> getFieldLabel() {
+        ASTNode label = getFieldLabelNode();
+        if (label == null) {
+            return Optional.empty();
+        }
+        return FieldLabel.forString(label.getText());
+    }
+
+    @Override
+    public ASTNode getFieldLabelNode() {
+        ASTNode node = getNode();
+        return node.findChildByType(R_FIELD_MODIFIER);
+    }
+
     @Override
     public String toString() {
         return "FieldNode(" + getFieldName() + "=" + getTag() + ")";

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

@@ -5,6 +5,7 @@
 import static io.protostuff.jetbrains.plugin.psi.Util.decodeIntegerFromText;
 
 import com.intellij.lang.ASTNode;
+import java.util.Optional;
 import org.antlr.jetbrains.adapter.psi.AntlrPsiNode;
 import org.jetbrains.annotations.NotNull;
 
@@ -54,4 +55,14 @@ public TypeReferenceNode getFieldType() {
         return null;
     }
 
+    @Override
+    public Optional<FieldLabel> getFieldLabel() {
+        return Optional.empty();
+    }
+
+    @Override
+    public ASTNode getFieldLabelNode() {
+        return null;
+    }
+
 }

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

@@ -2,6 +2,7 @@
 
 import com.intellij.lang.ASTNode;
 import com.intellij.psi.PsiElement;
+import java.util.Optional;
 
 /**
  * Message field node.
@@ -18,5 +19,9 @@ public interface MessageField extends PsiElement {
 
     ASTNode getTagNode();
 
+    Optional<FieldLabel> getFieldLabel();
+
+    ASTNode getFieldLabelNode();
+
     TypeReferenceNode getFieldType();
 }

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

@@ -12,6 +12,7 @@
 import io.protostuff.jetbrains.plugin.ProtoParserDefinition;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Optional;
 import org.antlr.jetbrains.adapter.psi.IdentifierDefSubtree;
 import org.jetbrains.annotations.NotNull;
 
@@ -66,6 +67,16 @@ public ASTNode getTagNode() {
         return node.findChildByType(R_TAG);
     }
 
+    @Override
+    public Optional<FieldLabel> getFieldLabel() {
+        return Optional.empty();
+    }
+
+    @Override
+    public ASTNode getFieldLabelNode() {
+        return null;
+    }
+
     @Override
     public TypeReferenceNode getFieldType() {
         return findChildByClass(TypeReferenceNode.class);

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

@@ -18,4 +18,6 @@ error.reserved.field.name=Reserved field name: {0}
 error.duplicate.constant.name=Duplicate constant name: {0}
 error.duplicate.constant.value=Duplicate constant value: {0}
 error.duplicate.method.name=Duplicate RPC method name: {0}
+error.missing.field.label=Missing field label
+error.illegal.field.label=Field label "{0}" is not allowed in proto3
 element.context.display={0} in {1}

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

@@ -66,6 +66,18 @@ public void testDuplicateServiceMethodName() {
         check();
     }
 
+    public void testProto2MissingFieldLabel() {
+        check();
+    }
+
+    public void testProto3IllegalOptionalFieldLabel() {
+        check();
+    }
+
+    public void testProto3IllegalRequiredFieldLabel() {
+        check();
+    }
+
     private void check() {
         String file = getTestName(false) + ".proto";
         myFixture.configureByFiles(file);

src/test/resources/annotator/Proto2MissingFieldLabel.proto

@@ -0,0 +1,7 @@
+syntax = "proto2";
+
+package annotator;
+
+message TestMessage {
+    <error descr="Missing field label">int32 field = 1;</error>
+}

src/test/resources/annotator/Proto3IllegalOptionalFieldLabel.proto

@@ -0,0 +1,7 @@
+syntax = "proto3";
+
+package annotator;
+
+message TestMessage {
+    <error descr="Field label \"optional\" is not allowed in proto3">optional</error> int32 field = 1;
+}