Validate that the start and end numbers of reserved and of extensions are positive and that end >= start. Change-Id: I94377f31f35a9fd15500bda93b2af1badd8efb39

diff --git a/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkIndexRangeBounds.java b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkIndexRangeBounds.java
new file mode 100644
index 0000000..a04cd27
--- /dev/null
+++ b/com.google.eclipse.protobuf.test/src/com/google/eclipse/protobuf/validation/ProtobufJavaValidator_checkIndexRangeBounds.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2015 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.validation;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static com.google.eclipse.protobuf.junit.core.UnitTestModule.unitTestModule;
+import static com.google.eclipse.protobuf.junit.core.XtextRule.overrideRuntimeModuleWith;
+
+import java.util.List;
+
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EStructuralFeature;
+import org.eclipse.xtext.validation.ValidationMessageAcceptor;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import com.google.eclipse.protobuf.junit.core.XtextRule;
+import com.google.eclipse.protobuf.protobuf.IndexRange;
+import com.google.eclipse.protobuf.protobuf.ProtobufPackage;
+import com.google.inject.Inject;
+
+public class ProtobufJavaValidator_checkIndexRangeBounds {
+  @Rule public XtextRule xtext = overrideRuntimeModuleWith(unitTestModule());
+
+  @Inject private ProtobufJavaValidator validator;
+  private ValidationMessageAcceptor messageAcceptor;
+
+  @Before public void setUp() {
+    messageAcceptor = mock(ValidationMessageAcceptor.class);
+    validator.setMessageAcceptor(messageAcceptor);
+  }
+
+  // syntax = "proto2";
+  //
+  // message Person {
+  //   reserved -2 to 2;
+  // }
+  @Test public void should_error_on_negative_bounds() {
+    List<IndexRange> indexRanges = xtext.findAll(IndexRange.class);
+    validator.checkIndexRangeBounds(indexRanges.get(0));
+    verifyError(
+        "Extensions and reserved numbers must be positive.",
+        indexRanges.get(0),
+        ProtobufPackage.Literals.INDEX_RANGE__FROM);
+    verifyNoMoreInteractions(messageAcceptor);
+  }
+
+  // syntax = "proto2";
+  //
+  // message Person {
+  //   reserved 3 to 1;
+  //   reserved 4 to 4;
+  // }
+  @Test public void should_error_on_end_less_than_start() {
+    List<IndexRange> indexRanges = xtext.findAll(IndexRange.class);
+    validator.checkIndexRangeBounds(indexRanges.get(0));
+    validator.checkIndexRangeBounds(indexRanges.get(1));
+    verifyError("End number must be greater than or equal to start number.", indexRanges.get(0));
+    verifyNoMoreInteractions(messageAcceptor);
+  }
+
+  private void verifyError(String message, EObject errorSource) {
+    verifyError(message, errorSource, null);
+  }
+
+  private void verifyError(String message, EObject errorSource, EStructuralFeature errorFeature) {
+    verify(messageAcceptor).acceptError(message, errorSource, errorFeature, -1, null);
+  }
+}

diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java
index 47aee62..7eb1a56 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexRanges.java

@@ -20,6 +20,13 @@
  * @author jogl@google.com (John Glassmyer)
  */
 @Singleton public class IndexRanges {
+  /**
+   * Thrown to indicate that a range's end number is less than its start number.
+   */
+  public static class BackwardsRangeException extends Exception {
+    private static final long serialVersionUID = 1L;
+  }
+
   private ProtobufGrammarAccess protobufGrammarAccess;
 
   @Inject
@@ -27,7 +34,10 @@
     this.protobufGrammarAccess = protobufGrammarAccess;
   }
 
-  public Range<Long> toLongRange(IndexRange indexRange) {
+  /**
+   * @throws BackwardsRangeException if the end number is less than the start number
+   */
+  public Range<Long> toLongRange(IndexRange indexRange) throws BackwardsRangeException {
     long from = indexRange.getFrom();
 
     Range<Long> range;
@@ -37,7 +47,13 @@
     } else if (toString.equals(getMaxKeyword())) {
       range = Range.atLeast(from);
     } else {
-      range = Range.closed(from, Long.valueOf(toString));
+      Long to = Long.valueOf(toString);
+
+      if (to < from) {
+        throw new BackwardsRangeException();
+      }
+
+      range = Range.closed(from, to);
     }
 
     return range;

diff --git a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java
index 1b995d4..f0cd083 100644
--- a/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java
+++ b/com.google.eclipse.protobuf/src/com/google/eclipse/protobuf/model/util/IndexedElements.java

@@ -21,6 +21,7 @@
 
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Range;
+import com.google.eclipse.protobuf.model.util.IndexRanges.BackwardsRangeException;
 import com.google.eclipse.protobuf.protobuf.FieldOption;
 import com.google.eclipse.protobuf.protobuf.Group;
 import com.google.eclipse.protobuf.protobuf.IndexRange;
@@ -67,20 +68,24 @@
   private long findMaxIndex(Iterable<? extends EObject> elements) {
     long maxIndex = 0;
 
-    for (EObject e : elements) {
-      if (e instanceof OneOf) {
-        maxIndex = max(maxIndex, findMaxIndex(((OneOf) e).getElements()));
-      } else if (e instanceof IndexedElement) {
-        maxIndex  = max(maxIndex, indexOf((IndexedElement) e));
-        if (e instanceof Group) {
-          maxIndex = max(maxIndex, findMaxIndex(((Group) e).getElements()));
+    for (EObject element : elements) {
+      if (element instanceof OneOf) {
+        maxIndex = max(maxIndex, findMaxIndex(((OneOf) element).getElements()));
+      } else if (element instanceof IndexedElement) {
+        maxIndex  = max(maxIndex, indexOf((IndexedElement) element));
+        if (element instanceof Group) {
+          maxIndex = max(maxIndex, findMaxIndex(((Group) element).getElements()));
         }
-      } else if (e instanceof Reserved) {
+      } else if (element instanceof Reserved) {
         for (IndexRange indexRange :
-            Iterables.filter(((Reserved) e).getReservations(), IndexRange.class)) {
-          Range<Long> range = indexRanges.toLongRange(indexRange);
-          if (range.hasUpperBound()) {
-            maxIndex = max(maxIndex, range.upperEndpoint());
+            Iterables.filter(((Reserved) element).getReservations(), IndexRange.class)) {
+          try {
+            Range<Long> range = indexRanges.toLongRange(indexRange);
+            if (range.hasUpperBound()) {
+              maxIndex = max(maxIndex, range.upperEndpoint());
+            }
+          } catch (BackwardsRangeException e) {
+            // Do not consider reserved ranges that are invalid.
           }
         }
       }

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 85c2f9d..bda1d9f 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

@@ -31,6 +31,8 @@
   public static String fieldNumbersMustBePositive;
   public static String importingUnsupportedSyntax;
   public static String importNotFound;
+  public static String indexRangeEndLessThanStart;
+  public static String indexRangeNonPositive;
   public static String invalidMapKeyType;
   public static String invalidMapValueType;
   public static String literalNotInEnum;

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 15aaf6a..60934f8 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

@@ -15,6 +15,8 @@
 fieldNumbersMustBePositive = Field numbers must be positive integers.
 importingUnsupportedSyntax = Importing unsupported file (directly or indirectly.) This may cause errors related to unresolved references.
 importNotFound = Import \"%s\" was not found.
+indexRangeEndLessThanStart = End number must be greater than or equal to start number.
+indexRangeNonPositive = Extensions and reserved numbers must be positive.
 invalidMapKeyType = Key in a map field must be of integral or string type.
 invalidMapValueType = Value in a map field cannot be a map.
 literalNotInEnum = Enum type \"%s\" has no value named \"%s\".

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 d37b92d..3c6e173 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

@@ -16,6 +16,8 @@
 import static com.google.eclipse.protobuf.validation.Messages.expectedFieldNumber;
 import static com.google.eclipse.protobuf.validation.Messages.expectedSyntaxIdentifier;
 import static com.google.eclipse.protobuf.validation.Messages.fieldNumbersMustBePositive;
+import static com.google.eclipse.protobuf.validation.Messages.indexRangeEndLessThanStart;
+import static com.google.eclipse.protobuf.validation.Messages.indexRangeNonPositive;
 import static com.google.eclipse.protobuf.validation.Messages.invalidMapKeyType;
 import static com.google.eclipse.protobuf.validation.Messages.invalidMapValueType;
 import static com.google.eclipse.protobuf.validation.Messages.mapWithModifier;
@@ -49,6 +51,7 @@
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Range;
 import com.google.eclipse.protobuf.model.util.IndexRanges;
+import com.google.eclipse.protobuf.model.util.IndexRanges.BackwardsRangeException;
 import com.google.eclipse.protobuf.model.util.IndexedElements;
 import com.google.eclipse.protobuf.model.util.Protobufs;
 import com.google.eclipse.protobuf.model.util.StringLiterals;
@@ -124,22 +127,44 @@
     error(msg, syntax, SYNTAX__NAME, SYNTAX_IS_NOT_KNOWN_ERROR);
   }
 
+  @Check public void checkIndexRangeBounds(IndexRange indexRange) {
+    Range<Long> range;
+    try {
+      range = indexRanges.toLongRange(indexRange);
+    } catch (BackwardsRangeException e) {
+      error(indexRangeEndLessThanStart, indexRange, null);
+      return;
+    }
+
+    if (range.lowerEndpoint() <= 0) {
+      error(indexRangeNonPositive, indexRange, ProtobufPackage.Literals.INDEX_RANGE__FROM);
+    }
+  }
+
   @Check public void checkForIndexConflicts(Message message) {
     Multimap<EObject, Range<Long>> rangeUsages = LinkedHashMultimap.create();
 
     for (Reserved reserved : getOwnedElements(message, Reserved.class)) {
       for (IndexRange indexRange : Iterables.filter(reserved.getReservations(), IndexRange.class)) {
-        Range<Long> range = indexRanges.toLongRange(indexRange);
-        errorOnConflicts(range, rangeUsages, indexRange, null);
-        rangeUsages.put(reserved, range);
+        try {
+          Range<Long> range = indexRanges.toLongRange(indexRange);
+          errorOnConflicts(range, rangeUsages, indexRange, null);
+          rangeUsages.put(reserved, range);
+        } catch (BackwardsRangeException e) {
+          // Do not try to find conflicts with invalid ranges.
+        }
       }
     }
 
     for (Extensions extensions : getOwnedElements(message, Extensions.class)) {
       for (IndexRange indexRange : extensions.getRanges()) {
-        Range<Long> range = indexRanges.toLongRange(indexRange);
-        errorOnConflicts(range, rangeUsages, indexRange, null);
-        rangeUsages.put(extensions, range);
+        try {
+          Range<Long> range = indexRanges.toLongRange(indexRange);
+          errorOnConflicts(range, rangeUsages, indexRange, null);
+          rangeUsages.put(extensions,  range);
+        } catch (BackwardsRangeException e) {
+          // Do not try to find conflicts with invalid ranges.
+        }
       }
     }