Merge pull request #815 from sleekybadger/hotfix/file-list-return-array

Fix FileList returning arrays

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Add your own contributions to the next release on the line below this, please include your name too. Please don't set a new version if you are the first to make the section for `master`.
 
+* Fix FileList returning arrays - [@sleekybadger](https://github.com/sleekybadger)
 * Fix broken violations order for github - [@sleekybadger](https://github.com/sleekybadger)
 
 ## 5.2.1

appveyor.yml

@@ -5,4 +5,7 @@ install:
 build: off
 
 test_script:
+  # Tests use real git commands
+  - git config --global user.email "danger@example.com"
+  - git config --global user.name "Danger McShane"
   - bundle exec rake specs

lib/danger/core_ext/file_list.rb

@@ -1,8 +1,11 @@
+require "danger/helpers/array_subclass"
+
 module Danger
-  class FileList < Array
+  class FileList
+    include Helpers::ArraySublcass
+
     # Information about pattern: http://ruby-doc.org/core-2.2.0/File.html#method-c-fnmatch
     # e.g. "**/something.*" for any file called something with any extension
-
     def include?(pattern)
       self.each do |current|
         return true if File.fnmatch(pattern, current)

lib/danger/helpers/array_subclass.rb

@@ -0,0 +1,61 @@
+module Danger
+  module Helpers
+    module ArraySublcass
+      include Comparable
+
+      def initialize(array)
+        @__array__ = array
+      end
+
+      def kind_of?(compare_class)
+        return true if compare_class == self.class
+
+        dummy.kind_of?(compare_class)
+      end
+
+      def method_missing(name, *args, &block)
+        super unless __array__.respond_to?(name)
+
+        respond_to_method(name, *args, &block)
+      end
+
+      def respond_to_missing?(name)
+        __array__.respond_to?(name) || super
+      end
+
+      def to_a
+        __array__
+      end
+
+      def to_ary
+        __array__
+      end
+
+      def <=>(other)
+        return unless other.kind_of?(self.class)
+
+        __array__ <=> other.instance_variable_get(:@__array__)
+      end
+
+      private
+
+      attr_accessor :__array__
+
+      def dummy
+        Class.new(Array).new
+      end
+
+      def respond_to_method(name, *args, &block)
+        result = __array__.send(name, *args, &block)
+        return result unless result.kind_of?(Array)
+
+        if name =~ /!/
+          __array__ = result
+          self
+        else
+          self.class.new(result)
+        end
+      end
+    end
+  end
+end

spec/lib/danger/danger_core/plugins/dangerfile_git_plugin_spec.rb

@@ -36,21 +36,21 @@ def run_in_repo_with_diff
       diff = [OpenStruct.new(type: "new", path: "added")]
       allow(@repo).to receive(:diff).and_return(diff)
 
-      expect(@dsl.added_files).to eq(["added"])
+      expect(@dsl.added_files).to eq(Danger::FileList.new(["added"]))
     end
 
     it "gets deleted_files " do
       diff = [OpenStruct.new(type: "deleted", path: "deleted")]
       allow(@repo).to receive(:diff).and_return(diff)
 
-      expect(@dsl.deleted_files).to eq(["deleted"])
+      expect(@dsl.deleted_files).to eq(Danger::FileList.new(["deleted"]))
     end
 
     it "gets modified_files " do
       diff = [OpenStruct.new(type: "modified", path: "my/path/file_name")]
       allow(@repo).to receive(:diff).and_return(diff)
 
-      expect(@dsl.modified_files).to eq(["my/path/file_name"])
+      expect(@dsl.modified_files).to eq(Danger::FileList.new(["my/path/file_name"]))
     end
 
     it "gets lines_of_code" do

spec/lib/danger/helpers/array_subclass_spec.rb

@@ -0,0 +1,49 @@
+RSpec.describe Danger::Helpers::ArraySublcass do
+  class List; include Danger::Helpers::ArraySublcass; end
+  class OtherList; include Danger::Helpers::ArraySublcass; end
+
+  it "acts as array" do
+    first_list = List.new([1, 2, 3])
+    second_list = List.new([4, 5, 6])
+    third_list = List.new([1, 2, 3])
+    fourth_list = List.new([7, 7])
+
+    mapped_list = first_list.map { |item| item + 1 }
+    concated_list = first_list + second_list
+    mapped_mutated_list = third_list.map! { |item| item + 10 }
+    deleted_from_list = fourth_list.delete_at(0)
+    reduced_list = first_list.each_with_index.reduce({}) do |accum, (i, el)|
+      accum.store(i, el)
+      accum
+    end
+
+    expect(first_list.length).to eq(3)
+    expect(mapped_list).to eq(List.new([2, 3, 4]))
+    expect(concated_list).to eq(List.new([1, 2, 3, 4, 5, 6]))
+    expect(third_list).to eq(List.new([11, 12, 13]))
+    expect(mapped_mutated_list).to eq(List.new([11, 12, 13]))
+    expect(deleted_from_list).to eq(7)
+    expect(fourth_list).to eq(List.new([7]))
+    expect(reduced_list).to eq({ 1 => 0, 2 => 1, 3 => 2 })
+  end
+
+  describe "equality" do
+    it "equals with same class same size and same values" do
+      first_list = List.new([1, 2, 3])
+      second_list = List.new([1, 2, 3])
+      third_list = List.new([4, 5, 6])
+
+      expect(first_list).to eq(second_list)
+      expect(first_list).not_to eq(third_list)
+    end
+
+    it "not equals with other classes" do
+      first_list = List.new([1, 2, 3])
+      second_list = OtherList.new([1, 2, 3])
+      third_list = [4, 5, 6]
+
+      expect(first_list).not_to eq(second_list)
+      expect(first_list).not_to eq(third_list)
+    end
+  end
+end

spec/lib/danger/scm_source/git_repo_spec.rb

@@ -64,7 +64,7 @@
         @dm = testing_dangerfile
         @dm.env.scm.diff_for_folder(dir, from: "master", to: "new")
 
-        expect(@dm.git.added_files).to eq(["file2"])
+        expect(@dm.git.added_files).to eq(Danger::FileList.new(["file2"]))
         expect(@dm.git.diff_for_file("file2")).not_to be_nil
       end
     end
@@ -84,7 +84,7 @@
 
           @dm = testing_dangerfile
           @dm.env.scm.diff_for_folder(dir, from: "master", to: "new")
-          expect(@dm.git.deleted_files).to eq(["file"])
+          expect(@dm.git.deleted_files).to eq(Danger::FileList.new(["file"]))
         end
       end
     end
@@ -106,7 +106,7 @@
           @dm.env.scm.diff_for_folder(dir, from: "master", to: "new")
 
           # Need to compact here because c50713a changes make AppVeyor fail
-          expect(@dm.git.modified_files.compact).to eq(["file"])
+          expect(@dm.git.modified_files.compact).to eq(Danger::FileList.new(["file"]))
         end
       end
     end
@@ -131,7 +131,7 @@
           @dm.env.scm.diff_for_folder(dir, from: "master", to: "new")
 
           # Need to compact here because c50713a changes make AppVeyor fail
-          expect(@dm.git.modified_files.compact).to eq(["file"])
+          expect(@dm.git.modified_files.compact).to eq(Danger::FileList.new(["file"]))
           expect(@dm.git.diff_for_file("file")).not_to be_nil
         end
       end