Implement PostRewrite#modified_files helper

lib/overcommit/hook_context/base.rb

@@ -94,5 +94,30 @@ def input_string
     def input_lines
       @input_lines ||= input_string.split("\n")
     end
+
+    private
+
+    def filter_modified_files(modified_files)
+      filter_directories(filter_nonexistent(modified_files))
+    end
+
+    # Filter out non-existent files (unless it's a broken symlink, in which case
+    # it's a file that points to a non-existent file). This could happen if a
+    # file was renamed as part of an amendment, leading to the old file no
+    # longer existing.
+    def filter_nonexistent(modified_files)
+      modified_files.select do |file|
+        File.exist?(file) || Overcommit::Utils.broken_symlink?(file)
+      end
+    end
+
+    # Filter out directories. This could happen when changing a symlink to a
+    # directory as part of an amendment, since the symlink will still appear as
+    # a file, but the actual working tree will have a directory.
+    def filter_directories(modified_files)
+      modified_files.reject do |file|
+        File.directory?(file) && !File.symlink?(file)
+      end
+    end
   end
 end

lib/overcommit/hook_context/post_rewrite.rb

@@ -25,6 +25,22 @@ def rewritten_commits
       end
     end
 
+    # Get a list of files that have been added or modified as part of a
+    # rewritten commit. Renames and deletions are ignored, since there should be
+    # nothing to check.
+    def modified_files
+      @modified_files ||= begin
+        @modified_files = []
+
+        rewritten_commits.each do |rewritten_commit|
+          refs = "#{rewritten_commit.old_hash} #{rewritten_commit.new_hash}"
+          @modified_files |= Overcommit::GitRepo.modified_files(refs: refs)
+        end
+
+        filter_modified_files(@modified_files)
+      end
+    end
+
     # Struct encapsulating the old and new SHA1 hashes of a rewritten commit
     RewrittenCommit = Struct.new(:old_hash, :new_hash)
   end

lib/overcommit/hook_context/pre_commit.rb

@@ -95,23 +95,7 @@ def modified_files
         if amendment?
           subcmd = 'show --format=%n'
           previously_modified = Overcommit::GitRepo.modified_files(subcmd: subcmd)
-
-          # Filter out non-existent files (unless it's a broken symlink, in
-          # which case it's a file that points to a non-existent file).
-          # This could happen if a file was renamed as part of the amendment,
-          # leading to the old file no longer existing.
-          previously_modified.select! do |file|
-            File.exist?(file) || Overcommit::Utils.broken_symlink?(file)
-          end
-
-          # Filter out directories. This could happen when changing a symlink to
-          # a directory as part of an amendment, since the symlink will still
-          # appear as a file, but the actual working tree will have a directory.
-          previously_modified.reject! do |file|
-            File.directory?(file) && !File.symlink?(file)
-          end
-
-          @modified_files |= previously_modified
+          @modified_files |= filter_modified_files(previously_modified)
         end
       end
       @modified_files

spec/overcommit/hook_context/post_rewrite_spec.rb

@@ -79,4 +79,144 @@
       end
     end
   end
+
+  describe '#modified_files' do
+    subject { context.modified_files }
+
+    before do
+      context.stub(:rewritten_commits).and_return(rewritten_commits)
+    end
+
+    context 'when rewrite was triggered by amend' do
+      let(:args) { ['amend'] }
+      let(:rewritten_commits) { [double(old_hash: 'HEAD@{1}', new_hash: 'HEAD')] }
+
+      it 'does not include submodules' do
+        submodule = repo do
+          FileUtils.touch 'foo'
+          `git add foo`
+          `git commit -m "Initial commit"`
+        end
+
+        repo do
+          `git commit --allow-empty -m "Initial commit"`
+          `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+          `git commit --amend -m "Add submodule"`
+          expect(subject).to_not include File.expand_path('test-sub')
+        end
+      end
+
+      context 'when no files were modified' do
+        around do |example|
+          repo do
+            `git commit --allow-empty -m "Initial commit"`
+            `git commit --amend --allow-empty -m "Another commit"`
+            example.run
+          end
+        end
+
+        it { should be_empty }
+      end
+
+      context 'when files were added' do
+        around do |example|
+          repo do
+            `git commit --allow-empty -m "Initial commit"`
+            FileUtils.touch('some-file')
+            `git add some-file`
+            `git commit --amend -m "Add file"`
+            example.run
+          end
+        end
+
+        it { should == [File.expand_path('some-file')] }
+      end
+
+      context 'when files were modified' do
+        around do |example|
+          repo do
+            FileUtils.touch('some-file')
+            `git add some-file`
+            `git commit -m "Initial commit"`
+            echo('Hello', 'some-file')
+            `git add some-file`
+            `git commit --amend -m "Modify file"`
+            example.run
+          end
+        end
+
+        it { should == [File.expand_path('some-file')] }
+      end
+
+      context 'when files were deleted' do
+        around do |example|
+          repo do
+            FileUtils.touch('some-file')
+            `git add some-file`
+            `git commit -m "Initial commit"`
+            `git rm some-file`
+            `git commit --amend --allow-empty -m "Delete file"`
+            example.run
+          end
+        end
+
+        it { should be_empty }
+      end
+
+      context 'when files were renamed' do
+        around do |example|
+          repo do
+            FileUtils.touch 'some-file'
+            `git add some-file`
+            `git commit -m "Add file"`
+            `git mv some-file renamed-file`
+            `git commit --amend -m "Rename file"`
+            example.run
+          end
+        end
+
+        it { should == [File.expand_path('renamed-file')] }
+      end
+
+      context 'when changing a symlink to a directory during an amendment' do
+        around do |example|
+          repo do
+            FileUtils.mkdir 'some-directory'
+            FileUtils.ln_s 'some-directory', 'some-symlink'
+            `git add some-symlink some-directory`
+            `git commit -m "Add file"`
+            `git rm some-symlink`
+            FileUtils.mkdir 'some-symlink'
+            FileUtils.touch File.join('some-symlink', 'another-file')
+            `git add some-symlink`
+            `git commit --amend -m "Change symlink to directory"`
+            example.run
+          end
+        end
+
+        it 'does not include the directory in the list of modified files' do
+          subject.should_not include File.expand_path('some-symlink')
+        end
+      end
+
+      context 'when breaking a symlink during an amendment' do
+        around do |example|
+          repo do
+            FileUtils.mkdir 'some-directory'
+            FileUtils.touch File.join('some-directory', 'some-file')
+            FileUtils.ln_s 'some-directory', 'some-symlink'
+            `git add some-symlink some-directory`
+            `git commit -m "Add file"`
+            `git rm -rf some-directory`
+            `git commit --amend -m "Remove directory to break symlink"`
+            example.run
+          end
+        end
+
+        it 'does not include the broken symlink in the list of modified files' do
+          subject.should_not include File.expand_path('some-symlink')
+        end
+      end
+    end
+  end
 end