From 4740346ed8766549f0f948a4396954227f2494bb Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Wed, 3 Oct 2018 16:24:02 +1300
Subject: [PATCH] FIX: Make ArrayList::limit() consistent with
 DataList::limit()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This makes it easier to swap one fo the other without code breaking.
Since it’s strictly a removed API, I’ve opted to throw a deprecation
note in SS4 rather than throwing an InvalidArgumentException.

Fixes #2949
---
 src/ORM/ArrayList.php           | 23 +++++++++++++++++++++++
 tests/php/ORM/ArrayListTest.php | 14 ++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/src/ORM/ArrayList.php b/src/ORM/ArrayList.php
index 9ac0558e2..612347fe2 100644
--- a/src/ORM/ArrayList.php
+++ b/src/ORM/ArrayList.php
@@ -8,6 +8,7 @@ use SilverStripe\View\ViewableData;
 use ArrayIterator;
 use InvalidArgumentException;
 use LogicException;
+use SilverStripe\Dev\Deprecation;
 
 /**
  * A list object that wraps around an array of objects or arrays.
@@ -160,7 +161,29 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      */
     public function limit($length, $offset = 0)
     {
+        // Type checking: designed for consistency with DataList::limit()
+        if (!is_numeric($length) || !is_numeric($offset)) {
+            Deprecation::notice(
+                '4.3',
+                'Arguments to ArrayList::limit() should be numeric'
+            );
+        }
+
+        if ($length < 0 || $offset < 0) {
+            Deprecation::notice(
+                '4.3',
+                'Arguments to ArrayList::limit() should be positive'
+            );
+        }
+
         if (!$length) {
+            if ($length === 0) {
+                Deprecation::notice(
+                    '4.3',
+                    "limit(0) is deprecated in SS4. In SS5 a limit of 0 will instead return no records."
+                );
+            }
+
             $length = count($this->items);
         }
 
diff --git a/tests/php/ORM/ArrayListTest.php b/tests/php/ORM/ArrayListTest.php
index 513b5d79b..3d6ae6250 100644
--- a/tests/php/ORM/ArrayListTest.php
+++ b/tests/php/ORM/ArrayListTest.php
@@ -7,6 +7,7 @@ use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Filterable;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\View\ArrayData;
+use SilverStripe\Dev\Deprecation;
 use stdClass;
 
 class ArrayListTest extends SapphireTest
@@ -133,6 +134,19 @@ class ArrayListTest extends SapphireTest
         );
     }
 
+    /**
+     * @expectedException PHPUnit_Framework_Error
+     */
+    public function testZeroLimit()
+    {
+        Deprecation::notification_version('4.3.0');
+        $list = new ArrayList([
+            ['Key' => 1],
+            ['Key' => 2],
+        ]);
+        $list->limit(0);
+    }
+
     public function testAddRemove()
     {
         $list = new ArrayList(