In progress: [Issue 164] Validate that the default value of a field
matches the type of such field.
Added validation for default values of numeric fields.
diff --git a/com.google.eclipse.protobuf.ui/OSGI-INF/l10n/bundle.properties b/com.google.eclipse.protobuf.ui/OSGI-INF/l10n/bundle.properties
index c4df5f0..1817545 100644
--- a/com.google.eclipse.protobuf.ui/OSGI-INF/l10n/bundle.properties
+++ b/com.google.eclipse.protobuf.ui/OSGI-INF/l10n/bundle.properties
@@ -20,5 +20,5 @@
command.description.1 = Insert semicolon.
command.name.1 = Insert semicolon
command.tooltip.1 = Insert semicolon
-protoc.marker.name = Protocol Buffer Compiler
+protoc.marker.name = Protocol Buffer Problem (Compiler)
editor.marker.name = Protocol Buffer Problem
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/protoc/ProtocMarkerFactory.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/protoc/ProtocMarkerFactory.java
index 7403a06..7470db9 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/protoc/ProtocMarkerFactory.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/protoc/ProtocMarkerFactory.java
@@ -8,9 +8,9 @@
*/
package com.google.eclipse.protobuf.ui.builder.protoc;
+import static com.google.eclipse.protobuf.ui.validation.ProtobufResourceUIValidatorExtension.EDITOR_CHECK;
import static org.eclipse.core.resources.IMarker.*;
import static org.eclipse.core.resources.IResource.DEPTH_INFINITE;
-import static org.eclipse.xtext.ui.MarkerTypes.FAST_VALIDATION;
import org.eclipse.core.resources.*;
import org.eclipse.core.runtime.CoreException;
@@ -30,7 +30,7 @@
ProtocMarkerFactory(IFile file) throws CoreException {
this.file = file;
file.deleteMarkers(PROTOC_CHECK, true, DEPTH_INFINITE);
- markers = file.findMarkers(FAST_VALIDATION, true, DEPTH_INFINITE);
+ markers = file.findMarkers(EDITOR_CHECK, true, DEPTH_INFINITE);
}
void createErrorIfNecessary(String fileName, String message, int lineNumber) throws CoreException {
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java
index a0c3d35..b8109f4 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java
@@ -201,7 +201,7 @@
private boolean isNanProposalValid(ContentAssistContext context) {
MessageField field = fieldFrom(context);
- return field != null && messageFields.mayBeNan(field);
+ return field != null && messageFields.isFloatingPointNumber(field);
}
private MessageField fieldFrom(ContentAssistContext context) {
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/validation/ProtobufResourceUIValidatorExtension.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/validation/ProtobufResourceUIValidatorExtension.java
index 18c5e11..f91ffed 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/validation/ProtobufResourceUIValidatorExtension.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/validation/ProtobufResourceUIValidatorExtension.java
@@ -10,8 +10,7 @@
import static org.eclipse.core.resources.IResource.DEPTH_ZERO;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.inject.Inject;
+import java.util.List;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.runtime.*;
@@ -19,17 +18,18 @@
import org.eclipse.xtext.ui.validation.DefaultResourceUIValidatorExtension;
import org.eclipse.xtext.validation.*;
-import java.util.List;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.inject.Inject;
/**
* Creates/deletes markers of type "Protocol Buffer Problem" instead of the default "Xtext Check."
- *
+ *
* @author alruiz@google.com (Alex Ruiz)
*/
public class ProtobufResourceUIValidatorExtension extends DefaultResourceUIValidatorExtension {
- static final String EDITOR_CHECK = "com.google.eclipse.protobuf.ui.editorMarker";
-
+ public static final String EDITOR_CHECK = "com.google.eclipse.protobuf.ui.editorMarker";
+
@VisibleForTesting
@Inject MarkerCreator markerCreator;
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/grammar/CommonKeyword.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/grammar/CommonKeyword.java
index 921af4f..353ea83 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/grammar/CommonKeyword.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/grammar/CommonKeyword.java
@@ -19,9 +19,13 @@
// looking for. The code was too complicated and if the grammar changed for some reason, we had to change our
// implementation anyway.
- BOOL("bool"), TRUE("true"), FALSE("false"), BYTES("bytes"), OPENING_BRACKET("["), CLOSING_BRACKET("]"),
- OPENING_CURLY_BRACKET("{"), CLOSING_CURLY_BRACKET("}"), DEFAULT("default"), EQUAL("="), SEMICOLON(";"),
- STRING("string"), SYNTAX("syntax"), NAN("nan"), FLOAT("float"), DOUBLE("double");
+
+ BOOL("bool"), BYTES("bytes"), CLOSING_BRACKET("]"), CLOSING_CURLY_BRACKET("}"), DEFAULT("default"), DOUBLE("double"),
+ EQUAL("="), FALSE("false"), FIXED32("fixed32"), FIXED64("fixed64"), FLOAT("float"), INT32("int32"), INT64("int64"),
+ NAN("nan"), OPENING_BRACKET("["), OPENING_CURLY_BRACKET("{"), SEMICOLON(";"), SFIXED32("sfixed32"),
+ SFIXED64("sfixed64"), STRING("string"), SYNTAX("syntax"), TRUE("true"), SINT32("sint32"), SINT64("sint64"),
+ UINT32("unit32"), UINT64("uint64");
+
private final String value;
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/MessageFields.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/MessageFields.java
index 1970a4a..ffcd151 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/MessageFields.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/MessageFields.java
@@ -13,7 +13,7 @@
import com.google.eclipse.protobuf.grammar.CommonKeyword;
import com.google.eclipse.protobuf.protobuf.*;
-import com.google.inject.*;
+import com.google.inject.Singleton;
/**
* Utility methods related to <code>{@link MessageField}</code>s.
@@ -56,13 +56,22 @@
}
/**
- * Indicates whether the given field can accept "nan" as its default value.
+ * Indicates whether the given field is of type {@code float} or {@code double}.
* @param field the given field.
- * @return {@code true} if the given field can accept "nan" as its default value, {@code false} otherwise.
+ * @return {@code true} if the given field is a floating point number, {@code false} otherwise.
*/
- public boolean mayBeNan(MessageField field) {
- String typeName = typeNameOf(field);
- return FLOAT.hasValue(typeName) || DOUBLE.hasValue(typeName);
+ public boolean isFloatingPointNumber(MessageField field) {
+ return isScalarType(field, FLOAT, DOUBLE);
+ }
+
+ /**
+ * Indicates whether the given field is of type {@code fixed32}, {@code fixed64}, {@code int32}, {@code int64},
+ * {@code sfixed32}, {@code sfixed64}, {@code sint32}, {@code sint64}, {@code uint32} or {@code uint64}.
+ * @param field the given field.
+ * @return {@code true} if the given field is an integer, {@code false} otherwise.
+ */
+ public boolean isInteger(MessageField field) {
+ return isScalarType(field, FIXED32, FIXED64, INT32, INT64, SFIXED32, SFIXED64, SINT32, SINT64, UINT32, UINT64);
}
/**
@@ -74,8 +83,24 @@
return isScalarType(field, STRING);
}
- private boolean isScalarType(MessageField field, CommonKeyword typeKeyword) {
- return typeKeyword.hasValue(typeNameOf(field));
+ /**
+ * Indicates whether the given field is of type {@code fixed32}, {@code fixed64}, {@code uint32} or {@code uint64}.
+ * @param field the given field.
+ * @return {@code true} if the given field is an unsigned integer, {@code false} otherwise.
+ */
+ public boolean isUnsignedInteger(MessageField field) {
+ return isScalarType(field, FIXED32, FIXED64, UINT32, UINT64);
+ }
+
+ private boolean isScalarType(MessageField field, CommonKeyword...scalarNames) {
+ TypeLink link = field.getType();
+ if (link instanceof ScalarTypeLink) {
+ String typeName = ((ScalarTypeLink) link).getTarget().getName();
+ for (CommonKeyword scalarName : scalarNames) {
+ if (scalarName.hasValue(typeName)) return true;
+ }
+ }
+ return false;
}
/**
diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/DataTypeValidator.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/DataTypeValidator.java
index 606cf00..4e4a82a 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/DataTypeValidator.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/validation/DataTypeValidator.java
@@ -36,37 +36,80 @@
@Check public void checkValueOfDefaultTypeMatchesFieldType(FieldOption option) {
if (fieldOptions.isDefaultValueOption(option)) {
MessageField field = (MessageField) option.eContainer();
- INode nodeForValueFeature = nodes.firstNodeForFeature(option, FIELD_OPTION__VALUE);
- checkValueTypeMatchesFieldType(option.getValue(), nodeForValueFeature, field);
+ checkValueTypeMatchesFieldType(option, field);
}
}
- private void checkValueTypeMatchesFieldType(Value value, INode nodeForValueFeature, MessageField field) {
- if (messageFields.isString(field)) {
- if (!(value instanceof StringLink)) {
- error(expectedString, FIELD_OPTION__VALUE);
- }
- return;
+ private void checkValueTypeMatchesFieldType(FieldOption option, MessageField field) {
+ if (validateBool(option, field)) return;
+ if (validateFloatingPointNumber(option, field)) return;
+ if (validateInteger(option, field)) return;
+ if (validateString(option, field)) return;
+ validateEnumLiteral(option, field);
+ }
+
+ private boolean validateBool(FieldOption option, MessageField field) {
+ if (!messageFields.isBool(field)) return false;
+ Value value = option.getValue();
+ if (!(value instanceof BooleanLink)) {
+ error(expectedTrueOrFalse, FIELD_OPTION__VALUE);
}
- if (messageFields.isBool(field)) {
- if (!(value instanceof BooleanLink)) {
- error(expectedTrueOrFalse, FIELD_OPTION__VALUE);
- }
- return;
+ return true;
+ }
+
+ private boolean validateFloatingPointNumber(FieldOption option, MessageField field) {
+ if (!messageFields.isFloatingPointNumber(field)) return false;
+ Value value = option.getValue();
+ if (!(value instanceof DoubleLink) && !(value instanceof LongLink)) {
+ error(expectedNumber, FIELD_OPTION__VALUE);
}
+ return true;
+ }
+
+ private boolean validateInteger(FieldOption option, MessageField field) {
+ if (!messageFields.isInteger(field)) return false;
+ Value value = option.getValue();
+ if (!(value instanceof LongLink)) {
+ error(expectedInteger, FIELD_OPTION__VALUE);
+ return true;
+ }
+ if (messageFields.isUnsignedInteger(field)) {
+ Long longValue = ((LongLink) value).getTarget();
+ if (longValue < 0) {
+ error(expectedPositiveNumber, FIELD_OPTION__VALUE);
+ }
+ }
+ return true;
+ }
+
+ private boolean validateString(FieldOption option, MessageField field) {
+ if (!messageFields.isString(field)) return false;
+ Value value = option.getValue();
+ if (!(value instanceof StringLink)) {
+ error(expectedString, FIELD_OPTION__VALUE);
+ }
+ return true;
+ }
+
+ private boolean validateEnumLiteral(FieldOption option, MessageField field) {
+ Value value = option.getValue();
Enum anEnum = modelFinder.enumTypeOf(field);
- if (anEnum != null) {
- if (!(value instanceof LiteralLink)) {
- error(expectedIdentifier, FIELD_OPTION__VALUE);
- }
- Literal literal = ((LiteralLink) value).getTarget();
- if (!anEnum.equals(literal.eContainer())) {
- QualifiedName enumFqn = fqnProvider.getFullyQualifiedName(anEnum);
- String literalName = nodes.textOf(nodeForValueFeature);
- String msg = String.format(literalNotInEnum, enumFqn, literalName);
- error(msg, FIELD_OPTION__VALUE);
- }
- return;
+ if (anEnum == null) return false;
+ if (!(value instanceof LiteralLink)) {
+ error(expectedIdentifier, FIELD_OPTION__VALUE);
+ return true;
}
+ Literal literal = ((LiteralLink) value).getTarget();
+ if (!anEnum.equals(literal.eContainer())) {
+ QualifiedName enumFqn = fqnProvider.getFullyQualifiedName(anEnum);
+ String literalName = nodes.textOf(nodeForValueFeatureIn(option));
+ String msg = String.format(literalNotInEnum, enumFqn, literalName);
+ error(msg, FIELD_OPTION__VALUE);
+ }
+ return true;
+ }
+
+ private INode nodeForValueFeatureIn(FieldOption option) {
+ return nodes.firstNodeForFeature(option, FIELD_OPTION__VALUE);
}
}
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 cef1009..ed62dc8 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
@@ -18,6 +18,9 @@
public static String expectedFieldName;
public static String expectedFieldNumber;
public static String expectedIdentifier;
+ public static String expectedInteger;
+ public static String expectedNumber;
+ public static String expectedPositiveNumber;
public static String expectedString;
public static String expectedSyntaxIdentifier;
public static String expectedTrueOrFalse;
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 497a7cd..105a56a 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
@@ -1,6 +1,9 @@
expectedFieldName = Expected field name.
expectedFieldNumber = Expected field number.
expectedIdentifier = Expected identifier.
+expectedInteger = Expected integer.
+expectedNumber = Expected number.
+expectedPositiveNumber = Unsigned field can't have negative default value.
expectedString = Expected string.
expectedSyntaxIdentifier = Expected syntax identifier.
expectedTrueOrFalse = Expected "true" or "false".
