Fixed: [ Issue 57 ] Groups are not taken into account for property index validation/calculation
https://code.google.com/p/protobuf-dt/issues/detail?id=57
diff --git a/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Properties_calculateTagNumberOf_Test.java b/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Fields_calculateTagNumberOf_Test.java
similarity index 83%
rename from com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Properties_calculateTagNumberOf_Test.java
rename to com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Fields_calculateTagNumberOf_Test.java
index a62b373..032d125 100644
--- a/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Properties_calculateTagNumberOf_Test.java
+++ b/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Fields_calculateTagNumberOf_Test.java
@@ -19,18 +19,18 @@
import com.google.eclipse.protobuf.protobuf.Protobuf;
/**
- * Tests for <code>{@link Properties#calculateTagNumberOf(Property)}</code>.
+ * Tests for <code>{@link Fields#calculateTagNumberOf(Property)}</code>.
*
* @author alruiz@google.com (Alex Ruiz)
*/
-public class Properties_calculateTagNumberOf_Test {
+public class Fields_calculateTagNumberOf_Test {
@Rule public XtextRule xtext = new XtextRule();
- private Properties properties;
+ private Fields fields;
@Before public void setUp() {
- properties = xtext.getInstanceOf(Properties.class);
+ fields = xtext.getInstanceOf(Fields.class);
}
@Test public void should_return_one_for_first_and_only_property() {
@@ -40,7 +40,7 @@
.append("} ");
Protobuf root = xtext.parse(proto);
Property name = findProperty("name", root);
- int index = properties.calculateTagNumberOf(name);
+ int index = fields.calculateTagNumberOf(name);
assertThat(index, equalTo(1));
}
@@ -52,7 +52,7 @@
.append("} ");
Protobuf root = xtext.parse(proto);
Property id = findProperty("id", root);
- int index = properties.calculateTagNumberOf(id);
+ int index = fields.calculateTagNumberOf(id);
assertThat(index, equalTo(7));
}
}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/ProtocMarkerFactory.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/ProtocMarkerFactory.java
index a1eb6c9..1188d40 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/ProtocMarkerFactory.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/builder/ProtocMarkerFactory.java
@@ -33,7 +33,7 @@
file.deleteMarkers(PROTOC, true, DEPTH_INFINITE);
markers = file.findMarkers(FAST_VALIDATION, true, DEPTH_INFINITE);
}
-
+
void createErrorIfNecessary(String message, int lineNumber) throws CoreException {
if (containsMarker(message, lineNumber)) return;
IMarker marker = file.createMarker(PROTOC);
@@ -41,11 +41,13 @@
marker.setAttribute(MESSAGE, message);
marker.setAttribute(LINE_NUMBER, lineNumber);
}
-
+
private boolean containsMarker(String description, int lineNumber) throws CoreException {
- for (IMarker marker : markers)
- if (marker.getAttribute(MESSAGE).equals(description) && marker.getAttribute(LINE_NUMBER).equals(lineNumber))
+ for (IMarker marker : markers) {
+ String markerMessage = (String) marker.getAttribute(MESSAGE);
+ if (markerMessage.equalsIgnoreCase(description) && marker.getAttribute(LINE_NUMBER).equals(lineNumber))
return true;
+ }
return false;
}
}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java
index 3216b10..6f52c7e 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java
@@ -20,8 +20,10 @@
import org.eclipse.xtext.ui.editor.contentassist.antlr.ParserBasedContentAssistContextFactory;
import org.eclipse.xtext.util.concurrent.IUnitOfWork;
-import com.google.eclipse.protobuf.protobuf.*;
-import com.google.eclipse.protobuf.ui.util.*;
+import com.google.eclipse.protobuf.protobuf.Literal;
+import com.google.eclipse.protobuf.protobuf.Property;
+import com.google.eclipse.protobuf.ui.util.Fields;
+import com.google.eclipse.protobuf.ui.util.Literals;
import com.google.inject.Inject;
/**
@@ -39,8 +41,8 @@
Pattern.compile("[\\s]+(.*)[\\s]+(.*)[\\s]+(.*)[\\s]+=[\\s]+[\\d]+(.*)");
@Inject private ParserBasedContentAssistContextFactory contextFactory;
+ @Inject private Fields fields;
@Inject private Literals literals;
- @Inject private Properties properties;
private final String semicolon = SEMICOLON.toString();
@@ -94,7 +96,7 @@
private String contentToInsert(String line, Property property) {
boolean hasIndexAlready = PROPERTY_WITH_INDEX.matcher(line).matches();
if (hasIndexAlready) return semicolon;
- int index = properties.calculateTagNumberOf(property);
+ int index = fields.calculateTagNumberOf(property);
return defaultIndexAndSemicolonToInsert(line, index);
}
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 072435b..c6342f0 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
@@ -50,7 +50,9 @@
@Inject private ProtobufElementFinder finder;
@Inject private DescriptorProvider descriptorProvider;
@Inject private PluginImageHelper imageHelper;
- @Inject private Images imageRegistry;
+
+ @Inject private Fields fields;
+ @Inject private Images images;
@Inject private Literals literals;
@Inject private Properties properties;
@Inject private Strings strings;
@@ -68,7 +70,7 @@
@Override public void complete_Syntax(EObject model, RuleCall ruleCall, ContentAssistContext context,
ICompletionProposalAcceptor acceptor) {
String proposal = SYNTAX + SPACE + EQUAL_PROTO2_IN_QUOTES;
- proposeAndAccept(proposal, imageHelper.getImage(imageRegistry.imageFor(Syntax.class)), context, acceptor);
+ proposeAndAccept(proposal, imageHelper.getImage(images.imageFor(Syntax.class)), context, acceptor);
}
@Override public void completeOption_Name(EObject model, Assignment assignment, ContentAssistContext context,
@@ -285,7 +287,7 @@
@Override public void completeProperty_Index(EObject model, Assignment assignment, ContentAssistContext context,
ICompletionProposalAcceptor acceptor) {
- int index = properties.calculateTagNumberOf((Property) model);
+ int index = fields.calculateTagNumberOf((Property) model);
proposeIndex(index, context, acceptor);
}
@@ -322,14 +324,14 @@
}
private Image defaultImage() {
- return imageHelper.getImage(imageRegistry.defaultImage());
+ return imageHelper.getImage(images.defaultImage());
}
-
+
@Override public void complete_FieldOption(EObject model, RuleCall ruleCall, ContentAssistContext context,
ICompletionProposalAcceptor acceptor) {
System.out.println("complete_FieldOption");
}
-
+
@Override public void completeFieldOption_Name(EObject model, Assignment assignment, ContentAssistContext context,
ICompletionProposalAcceptor acceptor) {
Property property = extractPropertyFrom(context);
@@ -349,7 +351,7 @@
acceptor.accept(proposal);
}
}
-
+
private List<String> existingFieldOptionNames(Property property) {
List<FieldOption> options = property.getFieldOptions();
if (options.isEmpty()) return emptyList();
@@ -361,7 +363,7 @@
private boolean canBePacked(Property property) {
return properties.isPrimitive(property) && REPEATED.equals(property.getModifier());
}
-
+
@Override public void completeFieldOption_Value(EObject model, Assignment assignment, ContentAssistContext context,
ICompletionProposalAcceptor acceptor) {
FieldOption option = (FieldOption) model;
@@ -389,7 +391,7 @@
}
private void proposeAndAccept(Enum enumType, ContentAssistContext context, ICompletionProposalAcceptor acceptor) {
- Image image = imageHelper.getImage(imageRegistry.imageFor(Literal.class));
+ Image image = imageHelper.getImage(images.imageFor(Literal.class));
for (Literal literal : enumType.getLiterals())
proposeAndAccept(literal.getName(), image, context, acceptor);
}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Fields.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Fields.java
new file mode 100644
index 0000000..a9e8d40
--- /dev/null
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Fields.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2011 Google Inc.
+ *
+ * All rights reserved. This program and the accompanying materials are made available under the terms of the Eclipse
+ * Public License v1.0 which accompanies this distribution, and is available at
+ *
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package com.google.eclipse.protobuf.ui.util;
+
+import static java.lang.Math.max;
+
+import org.eclipse.emf.ecore.EObject;
+
+import com.google.eclipse.protobuf.protobuf.Field;
+import com.google.inject.Singleton;
+
+/**
+ * Utility methods re <code>{@link Field}</code>.
+ *
+ * @author alruiz@google.com (Alex Ruiz)
+ */
+@Singleton
+public class Fields {
+
+ /**
+ * Calculates the tag number value for the given field. The calculated tag number value is the maximum of all the
+ * tag number values of the given field's siblings, plus one. The minimum tag number value is 1.
+ * <p>
+ * For example, in the following message:
+ *
+ * <pre>
+ * message Person {
+ * required string name = 1;
+ * optional string email = 2;
+ * optional PhoneNumber phone =
+ * </pre>
+ *
+ * The calculated tag number value for the field {@code PhoneNumber} will be 3.
+ * </p>
+ * @param p the given field.
+ * @return the calculated value for the tag number of the given field.
+ */
+ public int calculateTagNumberOf(Field f) {
+ int index = 0;
+ for (EObject o : f.eContainer().eContents()) {
+ if (o == f || !(o instanceof Field)) continue;
+ index = max(index, ((Field) o).getIndex());
+ }
+ return ++index;
+ }
+}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Properties.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Properties.java
index b9dd4f1..f9668cd 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Properties.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Properties.java
@@ -9,10 +9,6 @@
package com.google.eclipse.protobuf.ui.util;
import static com.google.eclipse.protobuf.ui.grammar.CommonKeyword.*;
-import static java.lang.Math.max;
-import static org.eclipse.xtext.EcoreUtil2.getAllContentsOfType;
-
-import java.util.List;
import com.google.eclipse.protobuf.protobuf.*;
import com.google.eclipse.protobuf.ui.grammar.CommonKeyword;
@@ -76,32 +72,4 @@
}
return r.toString();
}
-
- /**
- * Calculates the tag number value for the given property. The calculated tag number value is the maximum of all the
- * tag number values of the given property's siblings, plus one. The minimum tag number value is 1.
- * <p>
- * For example, in the following message:
- *
- * <pre>
- * message Person {
- * required string name = 1;
- * optional string email = 2;
- * optional PhoneNumber phone =
- * </pre>
- *
- * The calculated tag number value for the property {@code PhoneNumber} will be 3.
- * </p>
- * @param p the given property.
- * @return the calculated value for the tag number of the given property.
- */
- public int calculateTagNumberOf(Property p) {
- int index = 0;
- List<Property> allProperties = getAllContentsOfType(p.eContainer(), Property.class);
- for (Property c : allProperties) {
- if (c == p) continue;
- index = max(index, c.getIndex());
- }
- return ++index;
- }
}
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 9cc6fd4..9926ea1 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
@@ -47,7 +47,10 @@
'}'(';')?;
MessageElement:
- Type | Property | Group | ExtendMessage | Option;
+ Type | Field | ExtendMessage | Option;
+
+Field:
+ Property | Group;
Group:
modifier=Modifier 'group' name=ID '=' index=INT
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 eb49e53..baca753 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
@@ -15,7 +15,8 @@
import org.eclipse.emf.common.util.URI;
import org.eclipse.emf.ecore.EObject;
-import org.eclipse.xtext.naming.*;
+import org.eclipse.xtext.naming.IQualifiedNameProvider;
+import org.eclipse.xtext.naming.QualifiedName;
import org.eclipse.xtext.validation.Check;
import com.google.eclipse.protobuf.protobuf.*;
@@ -45,34 +46,34 @@
error(msg, SYNTAX__NAME);
}
- @Check public void checkTagNumberIsUnique(Property property) {
- if (isNameNull(property)) return; // we already show an error if name is null, no need to go further.
- int index = property.getIndex();
- EObject container = property.eContainer();
+ @Check public void checkTagNumberIsUnique(Field field) {
+ if (isNameNull(field)) return; // we already show an error if name is null, no need to go further.
+ int index = field.getIndex();
+ EObject container = field.eContainer();
if (container instanceof Message) {
Message message = (Message) container;
for (MessageElement element : message.getElements()) {
- if (!(element instanceof Property)) continue;
- Property p = (Property) element;
- if (p == property) break;
- if (p.getIndex() != index) continue;
+ if (!(element instanceof Field)) continue;
+ Field other = (Field) element;
+ if (other == field) break;
+ if (other.getIndex() != index) continue;
QualifiedName messageName = qualifiedNameProvider.getFullyQualifiedName(message);
- String msg = format(fieldNumberAlreadyUsed, index, messageName.toString(), p.getName());
- error(msg, PROPERTY__INDEX);
+ String msg = format(fieldNumberAlreadyUsed, index, messageName.toString(), other.getName());
+ error(msg, FIELD__INDEX);
break;
}
}
}
- @Check public void checkTagNumberIsGreaterThanZero(Property property) {
- if (isNameNull(property)) return; // we already show an error if name is null, no need to go further.
- int index = property.getIndex();
+ @Check public void checkTagNumberIsGreaterThanZero(Field field) {
+ if (isNameNull(field)) return; // we already show an error if name is null, no need to go further.
+ int index = field.getIndex();
if (index > 0) return;
String msg = (index == 0) ? fieldNumbersMustBePositive : expectedFieldNumber;
- error(msg, PROPERTY__INDEX);
+ error(msg, FIELD__INDEX);
}
- private boolean isNameNull(Property property) {
- return property.getName() == null;
+ private boolean isNameNull(Field field) {
+ return field.getName() == null;
}
}
