Fix PlotRangeIterator

Core/src/main/java/com/plotsquared/core/plot/Plot.java

@@ -2299,12 +2299,13 @@ public class Plot {
                 continue;
             }
             boolean merge = true;
-            PlotId bot = PlotId.of(current.getId().getX(), current.getId().getY());
+            PlotId bot = current.getId();
-            PlotId top = PlotId.of(current.getId().getX(), current.getId().getY());
+            PlotId top = current.getId();
             while (merge) {
                 merge = false;
-                List<PlotId> ids = Lists.newArrayList((Iterable<? extends PlotId>) PlotId.PlotRangeIterator
+                Iterable<PlotId> ids = PlotId.PlotRangeIterator.range(
-                    .range(PlotId.of(bot.getX(), bot.getY() - 1), PlotId.of(top.getX(), bot.getY() - 1)));
+                        PlotId.of(bot.getX(), bot.getY() - 1),
+                        PlotId.of(top.getX(), bot.getY() - 1));
                 boolean tmp = true;
                 for (PlotId id : ids) {
                     Plot plot = this.area.getPlotAbs(id);
@@ -2316,8 +2317,9 @@ public class Plot {
                     merge = true;
                     bot = PlotId.of(bot.getX(), bot.getY() - 1);
                 }
-                ids = Lists.newArrayList((Iterable<? extends PlotId>) PlotId.PlotRangeIterator
+                ids = PlotId.PlotRangeIterator.range(
-                    .range(PlotId.of(top.getX() + 1, bot.getY()), PlotId.of(top.getX() + 1, top.getY())));
+                        PlotId.of(top.getX() + 1, bot.getY()),
+                        PlotId.of(top.getX() + 1, top.getY()));
                 tmp = true;
                 for (PlotId id : ids) {
                     Plot plot = this.area.getPlotAbs(id);
@@ -2329,8 +2331,9 @@ public class Plot {
                     merge = true;
                     top = PlotId.of(top.getX() + 1, top.getY());
                 }
-                ids = Lists.newArrayList((Iterable<? extends PlotId>) PlotId.PlotRangeIterator
+                ids = PlotId.PlotRangeIterator.range(
-                    .range(PlotId.of(bot.getX(), top.getY() + 1), PlotId.of(top.getX(), top.getY() + 1)));
+                        PlotId.of(bot.getX(), top.getY() + 1),
+                        PlotId.of(top.getX(), top.getY() + 1));
                 tmp = true;
                 for (PlotId id : ids) {
                     Plot plot = this.area.getPlotAbs(id);
@@ -2342,8 +2345,9 @@ public class Plot {
                     merge = true;
                     top = PlotId.of(top.getX(), top.getY() + 1);
                 }
-                ids = Lists.newArrayList((Iterable<? extends PlotId>) PlotId.PlotRangeIterator
+                ids = PlotId.PlotRangeIterator.range(
-                    .range(PlotId.of(bot.getX() - 1, bot.getY()), PlotId.of(bot.getX() - 1, top.getY())));
+                        PlotId.of(bot.getX() - 1, bot.getY()),
+                        PlotId.of(bot.getX() - 1, top.getY()));
                 tmp = true;
                 for (PlotId id : ids) {
                     Plot plot = this.area.getPlotAbs(id);

Core/src/main/java/com/plotsquared/core/plot/PlotId.java

@@ -30,6 +30,7 @@ import com.plotsquared.core.location.Direction;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 /**
  * Plot (X,Y) tuples for plot locations
@@ -265,26 +266,25 @@ public final class PlotId {
         }
 
         @Override public boolean hasNext() {
-            if (this.x < this.end.getX()) {
+            // end is fully included
-                return true;
+            return this.x <= this.end.getX() && this.y <= this.end.getY();
-            } else if (this.x == this.end.getX()) {
-                return this.y < this.end.getY();
-            } else {
-                return false;
-            }
         }
 
         @Override public PlotId next() {
             if (!hasNext()) {
-                throw new IndexOutOfBoundsException("The iterator has no more entries");
+                throw new NoSuchElementException("The iterator has no more entries");
             }
+            // increment *after* getting the result to include the minimum
+            // the id to return
+            PlotId result = PlotId.of(this.x, this.y);
+            // first increase y, then x
             if (this.y == this.end.getY()) {
                 this.x++;
                 this.y = 0;
             } else {
                 this.y++;
             }
-            return PlotId.of(this.start.getX() + this.x, this.start.getY() + this.y);
+            return result;
         }
 
         @Nonnull @Override public Iterator<PlotId> iterator() {

Core/src/test/java/com/plotsquared/core/plot/PlotRangeIteratorTest.java

@@ -0,0 +1,108 @@
+/*
+ *       _____  _       _    _____                                _
+ *      |  __ \| |     | |  / ____|                              | |
+ *      | |__) | | ___ | |_| (___   __ _ _   _  __ _ _ __ ___  __| |
+ *      |  ___/| |/ _ \| __|\___ \ / _` | | | |/ _` | '__/ _ \/ _` |
+ *      | |    | | (_) | |_ ____) | (_| | |_| | (_| | | |  __/ (_| |
+ *      |_|    |_|\___/ \__|_____/ \__, |\__,_|\__,_|_|  \___|\__,_|
+ *                                    | |
+ *                                    |_|
+ *            PlotSquared plot management system for Minecraft
+ *                  Copyright (C) 2020 IntellectualSites
+ *
+ *     This program is free software: you can redistribute it and/or modify
+ *     it under the terms of the GNU General Public License as published by
+ *     the Free Software Foundation, either version 3 of the License, or
+ *     (at your option) any later version.
+ *
+ *     This program is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ *     You should have received a copy of the GNU General Public License
+ *     along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+package com.plotsquared.core.plot;
+
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+public class PlotRangeIteratorTest {
+
+    @Test
+    public void singlePlotIterator() {
+        // an iterator that should only contain the given plot
+        PlotId id = PlotId.of(3, 7);
+        PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id, id);
+        assertTrue(range.hasNext());
+        assertEquals(id, range.next());
+        assertFalse(range.hasNext());
+        assertThrows(NoSuchElementException.class, range::next);
+    }
+
+    // the tests below assume a specific order (first increasing y, then increasing x)
+    // this is not a written requirement but makes testing easier
+
+    @Test
+    public void squareAreaPlotIterator() {
+        PlotId id00 = PlotId.of(0, 0);
+        PlotId id01 = PlotId.of(0, 1);
+        PlotId id10 = PlotId.of(1, 0);
+        PlotId id11 = PlotId.of(1, 1);
+        List<PlotId> all = Arrays.asList(id00, id01, id10, id11);
+        PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id00, id11);
+        for (PlotId id : all) {
+            assertTrue(range.hasNext());
+            assertEquals(id, range.next());
+        }
+        assertFalse(range.hasNext());
+        assertThrows(NoSuchElementException.class, range::next);
+    }
+
+    @Test
+    public void rectangleYAreaPlotIterator() {
+        // max y > max x
+        PlotId id00 = PlotId.of(0, 0);
+        PlotId id01 = PlotId.of(0, 1);
+        PlotId id02 = PlotId.of(0, 2);
+        PlotId id10 = PlotId.of(1, 0);
+        PlotId id11 = PlotId.of(1, 1);
+        PlotId id12 = PlotId.of(1, 2);
+        List<PlotId> all = Arrays.asList(id00, id01, id02, id10, id11, id12);
+        PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id00, id12);
+        for (PlotId id : all) {
+            assertTrue(range.hasNext());
+            assertEquals(id, range.next());
+        }
+        assertFalse(range.hasNext());
+        assertThrows(NoSuchElementException.class, range::next);
+    }
+
+    @Test
+    public void rectangleXAreaPlotIterator() {
+        // max x > max y
+        PlotId id00 = PlotId.of(0, 0);
+        PlotId id01 = PlotId.of(0, 1);
+        PlotId id10 = PlotId.of(1, 0);
+        PlotId id11 = PlotId.of(1, 1);
+        PlotId id20 = PlotId.of(2, 0);
+        PlotId id21 = PlotId.of(2, 1);
+        List<PlotId> all = Arrays.asList(id00, id01, id10, id11, id20, id21);
+        PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id00, id21);
+        for (PlotId id : all) {
+            assertTrue(range.hasNext());
+            assertEquals(id, range.next());
+        }
+        assertFalse(range.hasNext());
+        assertThrows(NoSuchElementException.class, range::next);
+    }
+}