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; } }