From e936233fa21fbf80e1c5df2ddbd7d2a755859530 Mon Sep 17 00:00:00 2001
From: Max Gurela <maxpowa1@gmail.com>
Date: Fri, 20 Oct 2017 11:40:31 -0700
Subject: [PATCH] Allow chaining Array methods

Modifies Array based on discussion in #4994. `Array#reject!`, `Array#select!`, and `Array#compact!` now return self in all scenarios.
---
 spec/std/array_spec.cr | 33 +++++----------------------------
 src/array.cr           | 11 ++++++-----
 2 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/spec/std/array_spec.cr b/spec/std/array_spec.cr
index 513f86e94877..11de096cbf8a 100644
--- a/spec/std/array_spec.cr
+++ b/spec/std/array_spec.cr
@@ -354,18 +354,11 @@ describe "Array" do
     a.should eq([1, nil, 2, nil, 3])
   end
 
-  describe "compact!" do
-    it "returns true if removed" do
-      a = [1, nil, 2, nil, 3]
-      b = a.compact!.should be_true
-      a.should eq([1, 2, 3])
-    end
-
-    it "returns false if not removed" do
-      a = [1]
-      b = a.compact!.should be_false
-      a.should eq([1])
-    end
+  it "does compact!" do
+    a = [1, nil, 2, nil, 3]
+    b = a.compact!
+    b.should eq([1, 2, 3])
+    b.should be(a)
   end
 
   describe "concat" do
@@ -1307,14 +1300,6 @@ describe "Array" do
     ary2.should be(ary1)
   end
 
-  it "returns nil when using select! and no changes were made" do
-    ary1 = [1, 2, 3, 4, 5]
-
-    ary2 = ary1.select! { true }
-    ary2.should eq(nil)
-    ary1.should eq([1, 2, 3, 4, 5])
-  end
-
   it "rejects!" do
     ary1 = [1, 2, 3, 4, 5]
 
@@ -1323,14 +1308,6 @@ describe "Array" do
     ary2.should be(ary1)
   end
 
-  it "returns nil when using reject! and no changes were made" do
-    ary1 = [1, 2, 3, 4, 5]
-
-    ary2 = ary1.reject! { false }
-    ary2.should eq(nil)
-    ary1.should eq([1, 2, 3, 4, 5])
-  end
-
   it "does map_with_index" do
     ary = [1, 1, 2, 2]
     ary2 = ary.map_with_index { |e, i| e + i }
diff --git a/src/array.cr b/src/array.cr
index 5ad65f8b220d..08d3c9edcad2 100644
--- a/src/array.cr
+++ b/src/array.cr
@@ -541,7 +541,7 @@ class Array(T)
     compact_map &.itself
   end
 
-  # Removes all `nil` elements from `self`.
+  # Removes all `nil` elements from `self` and returns `self`.
   #
   # ```
   # ary = ["a", nil, "b", nil, "c"]
@@ -549,7 +549,7 @@ class Array(T)
   # ary # => ["a", "b", "c"]
   # ```
   def compact!
-    !!(reject! &.nil?)
+    reject! &.nil?
   end
 
   # Appends the elements of *other* to `self`, and returns `self`.
@@ -888,7 +888,7 @@ class Array(T)
   end
 
   # Modifies `self`, keeping only the elements in the collection for which the
-  # passed block returns `true`. Returns `nil` if no changes were made.
+  # passed block returns `true`. Returns `self`.
   #
   # See also: `Array#select`.
   def select!
@@ -896,11 +896,12 @@ class Array(T)
   end
 
   # Modifies `self`, deleting the elements in the collection for which the
-  # passed block returns `true`. Returns `nil` if no changes were made.
+  # passed block returns `true`. Returns `self`.
   #
   # See also: `Array#reject`.
   def reject!
-    internal_delete { |e| yield e }[0]
+    internal_delete { |e| yield e }
+    self
   end
 
   # `reject!` and `delete` implementation: returns a tuple {x, y}