Check that fields inside a "oneof" declaration do not have modifiers.
Change-Id: Icee28c0ff839abc34b316a2c05ccc6d4d57eab18
Signed-off-by: Harry Terkelsen <het@google.com>
diff --git a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkFieldModifiers_Test.java b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkFieldModifiers_Test.java
index c8c87a7..e620d16 100644
--- a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkFieldModifiers_Test.java
+++ b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkFieldModifiers_Test.java
@@ -13,8 +13,10 @@
import static com.google.eclipse.protobuf.protobuf.ProtobufPackage.Literals.MESSAGE_FIELD__MODIFIER;
import static com.google.eclipse.protobuf.validation.Messages.mapWithModifier;
import static com.google.eclipse.protobuf.validation.Messages.missingModifier;
+import static com.google.eclipse.protobuf.validation.Messages.oneofFieldWithModifier;
import static com.google.eclipse.protobuf.validation.ProtobufJavaValidator.MAP_WITH_MODIFIER_ERROR;
import static com.google.eclipse.protobuf.validation.ProtobufJavaValidator.MISSING_MODIFIER_ERROR;
+import static com.google.eclipse.protobuf.validation.ProtobufJavaValidator.ONEOF_FIELD_WITH_MODIFIER_ERROR;
import static org.eclipse.xtext.validation.ValidationMessageAcceptor.INSIGNIFICANT_INDEX;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -112,4 +114,33 @@
validator.checkFieldModifiers(field);
verifyZeroInteractions(messageAcceptor);
}
+
+ // syntax = "proto2";
+ //
+ // message Foo {
+ // oneof test {
+ // string bar = 1;
+ // string baz = 2;
+ // }
+ // }
+ @Test public void should_not_create_error_if_no_modifier_on_oneof_field() {
+ MessageField field = xtext.find("bar", MessageField.class);
+ validator.checkFieldModifiers(field);
+ verifyZeroInteractions(messageAcceptor);
+ }
+
+ // syntax = "proto2";
+ //
+ // message Foo {
+ // oneof test {
+ // repeated string bar = 1;
+ // string baz = 2;
+ // }
+ // }
+ @Test public void should_create_error_if_modifier_on_oneof_field() {
+ MessageField field = xtext.find("bar", MessageField.class);
+ validator.checkFieldModifiers(field);
+ verify(messageAcceptor).acceptError(oneofFieldWithModifier, field, MESSAGE_FIELD__MODIFIER,
+ INSIGNIFICANT_INDEX, ONEOF_FIELD_WITH_MODIFIER_ERROR);
+ }
}
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/Protobuf.xtext b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/Protobuf.xtext
index 3e0ce2a..6c60868 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/Protobuf.xtext
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/Protobuf.xtext
@@ -81,7 +81,7 @@
'}' (';')?;
OneOfElement returns MessageElement:
- Extensions | Group;
+ Extensions | Group | MessageField;
Extensions:
->'extensions' ranges+=Range (',' ranges+=Range)* (';')+;
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java
index 8b6c645..8f80459 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.java
@@ -34,6 +34,7 @@
public static String missingFieldNumber;
public static String missingModifier;
public static String multiplePackages;
+ public static String oneofFieldWithModifier;
public static String requiredInProto3;
public static String scopingError;
public static String unknownSyntax;
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties
index df4f28f..9c15670 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/Messages.properties
@@ -17,6 +17,7 @@
mapWithModifier = Field labels (required/optional/repeated) are not allowed on map fields.
missingFieldNumber = Missing field number.
missingModifier = Missing modifier: \"required\", \"optional\", or \"repeated\".
+oneofFieldWithModifier = Fields inside "oneof" cannot have a modifier.
multiplePackages = Multiple package definitions.
requiredInProto3 = Required fields are not allowed in proto3.
scopingError = It may be caused by an imported non-proto2 file.
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java
index adbb50d..3ec975b 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator.java
@@ -22,6 +22,7 @@
import static com.google.eclipse.protobuf.validation.Messages.mapWithModifier;
import static com.google.eclipse.protobuf.validation.Messages.missingModifier;
import static com.google.eclipse.protobuf.validation.Messages.multiplePackages;
+import static com.google.eclipse.protobuf.validation.Messages.oneofFieldWithModifier;
import static com.google.eclipse.protobuf.validation.Messages.requiredInProto3;
import static com.google.eclipse.protobuf.validation.Messages.unknownSyntax;
import static com.google.eclipse.protobuf.validation.Messages.unrecognizedSyntaxIdentifier;
@@ -67,6 +68,7 @@
public static final String REQUIRED_IN_PROTO3_ERROR = "requiredInProto3";
public static final String INVALID_MAP_KEY_TYPE_ERROR = "invalidMapKeyType";
public static final String MAP_WITH_MAP_VALUE_TYPE_ERROR = "mapWithMapValueType";
+ public static final String ONEOF_FIELD_WITH_MODIFIER_ERROR = "oneofFieldWithModifier";
@Inject private IndexedElements indexedElements;
@Inject private NameResolver nameResolver;
@@ -109,6 +111,10 @@
checkMapField(field);
return;
}
+ if (field.eContainer() instanceof OneOf) {
+ checkOneOfField(field);
+ return;
+ }
if (field.getModifier() == ModifierEnum.UNSPECIFIED && isProto2Field(field)) {
error(missingModifier, field, MESSAGE_FIELD__MODIFIER, MISSING_MODIFIER_ERROR);
} else if (field.getModifier() == ModifierEnum.REQUIRED && isProto3Field(field)) {
@@ -123,6 +129,13 @@
}
}
+ private void checkOneOfField(MessageField field) {
+ if (field.getModifier() != ModifierEnum.UNSPECIFIED) {
+ error(oneofFieldWithModifier, field, MESSAGE_FIELD__MODIFIER,
+ ONEOF_FIELD_WITH_MODIFIER_ERROR);
+ }
+ }
+
private boolean isProto2Field(MessageField field) {
EObject container = field.eContainer();
if (container != null && !(container instanceof Protobuf)) {
