Add -diff cli option for running precommit hooks against diffs (#860)

For example, running `overcommit --diff main` from a feature branch will
run pre-commit hooks against the diff between the two branches.

I was able to very easily leverage existing code for the bulk of the
feature - this is mainly just adding the cli option, a hook context to
do the execution and some tests based on the existing `--run-all` test.

---

For background, my team is responsible for a couple of really old,
really large rails apps. Getting them completely in compliance with our
various linters is a huge task that isn't getting done anytime soon
(things are funky to the point that we've even observed breakages with
"safe" auto-correct functions).

I introduced/started heavily encouraging overcommit so that we at least
don't add _new_ linting offenses and things will naturally improve over
time. It's been great, but offenses still slip through though here and
there, especially with juniors who might be getting away with not having
a local install (and/or abusing `OVERCOMMIT_DISABLE=1`).

An option like this would allow me to leverage the very useful "only
apply to changed lines" logic within a ci environment and help enforce
my desired "no new linting offenses" policy.

Author: benmelz

lib/overcommit/cli.rb

@@ -9,6 +9,7 @@ module Overcommit
   class CLI # rubocop:disable Metrics/ClassLength
     def initialize(arguments, input, logger)
       @arguments = arguments
+      @cli_options = {}
       @input     = input
       @log       = logger
       @options   = {}
@@ -28,6 +29,8 @@ def run
         sign
       when :run_all
         run_all
+      when :diff
+        diff
       end
     rescue Overcommit::Exceptions::ConfigurationSignatureChanged => e
       puts e
@@ -45,7 +48,7 @@ def parse_arguments
       @parser = create_option_parser
 
       begin
-        @parser.parse!(@arguments)
+        @parser.parse!(@arguments, into: @cli_options)
 
         # Default action is to install
         @options[:action] ||= :install
@@ -98,6 +101,11 @@ def add_installation_options(opts)
         @options[:action] = :run_all
         @options[:hook_to_run] = arg ? arg.to_s : 'run-all'
       end
+
+      opts.on('--diff [ref]', 'Run pre_commit hooks against the diff between a given ref. Defaults to `main`.') do |arg| # rubocop:disable Layout/LineLength
+        @options[:action] = :diff
+        arg
+      end
     end
 
     def add_other_options(opts)
@@ -209,6 +217,19 @@ def run_all
       halt(status ? 0 : 65)
     end
 
+    def diff
+      empty_stdin = File.open(File::NULL) # pre-commit hooks don't take input
+      context = Overcommit::HookContext.create('diff', config, @arguments, empty_stdin, **@cli_options) # rubocop:disable Layout/LineLength
+      config.apply_environment!(context, ENV)
+
+      printer = Overcommit::Printer.new(config, log, context)
+      runner  = Overcommit::HookRunner.new(config, log, context, printer)
+
+      status = runner.run
+
+      halt(status ? 0 : 65)
+    end
+
     # Used for ease of stubbing in tests
     def halt(status = 0)
       exit status

lib/overcommit/hook_context.rb

@@ -2,13 +2,13 @@
 
 # Utility module which manages the creation of {HookContext}s.
 module Overcommit::HookContext
-  def self.create(hook_type, config, args, input)
+  def self.create(hook_type, config, args, input, **cli_options)
     hook_type_class = Overcommit::Utils.camel_case(hook_type)
     underscored_hook_type = Overcommit::Utils.snake_case(hook_type)
 
     require "overcommit/hook_context/#{underscored_hook_type}"
 
-    Overcommit::HookContext.const_get(hook_type_class).new(config, args, input)
+    Overcommit::HookContext.const_get(hook_type_class).new(config, args, input, **cli_options)
   rescue LoadError, NameError => e
     # Could happen when a symlink was created for a hook type Overcommit does
     # not yet support.

lib/overcommit/hook_context/base.rb

@@ -18,10 +18,12 @@ class Base
     # @param config [Overcommit::Configuration]
     # @param args [Array<String>]
     # @param input [IO] standard input stream
-    def initialize(config, args, input)
+    # @param options [Hash] cli options
+    def initialize(config, args, input, **options)
       @config = config
       @args = args
       @input = input
+      @options = options
     end
 
     # Executes a command as if it were a regular git hook, passing all

lib/overcommit/hook_context/diff.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'overcommit/git_repo'
+
+module Overcommit::HookContext
+  # Simulates a pre-commit context based on the diff with another git ref.
+  #
+  # This results in pre-commit hooks running against the changes between the current
+  # and another ref, which is useful for automated CI scripts.
+  class Diff < Base
+    def modified_files
+      @modified_files ||= Overcommit::GitRepo.modified_files(refs: @options[:diff])
+    end
+
+    def modified_lines_in_file(file)
+      @modified_lines ||= {}
+      @modified_lines[file] ||= Overcommit::GitRepo.extract_modified_lines(file,
+                                                                           refs: @options[:diff])
+    end
+
+    def hook_class_name
+      'PreCommit'
+    end
+
+    def hook_type_name
+      'pre_commit'
+    end
+
+    def hook_script_name
+      'pre-commit'
+    end
+
+    def initial_commit?
+      @initial_commit ||= Overcommit::GitRepo.initial_commit?
+    end
+  end
+end

spec/integration/diff_flag_spec.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'overcommit --diff' do
+  subject { shell(%w[overcommit --diff main]) }
+
+  context 'when using an existing pre-commit hook script' do
+    let(:script_name) { 'test-script' }
+    let(:script_contents) { "#!/bin/bash\nexit 0" }
+    let(:script_path) { ".#{Overcommit::OS::SEPARATOR}#{script_name}" }
+
+    let(:config) do
+      {
+        'PreCommit' => {
+          'MyHook' => {
+            'enabled' => true,
+            'required_executable' => script_path,
+          }
+        }
+      }
+    end
+
+    around do |example|
+      repo do
+        File.open('.overcommit.yml', 'w') { |f| f.puts(config.to_yaml) }
+        echo(script_contents, script_path)
+        `git add #{script_path}`
+        FileUtils.chmod(0o755, script_path)
+        example.run
+      end
+    end
+
+    it 'completes successfully without blocking' do
+      wait_until(timeout: 10) { subject } # Need to wait long time for JRuby startup
+      subject.status.should == 0
+    end
+  end
+end

spec/overcommit/cli_spec.rb

@@ -2,6 +2,7 @@
 
 require 'spec_helper'
 require 'overcommit/cli'
+require 'overcommit/hook_context/diff'
 require 'overcommit/hook_context/run_all'
 
 describe Overcommit::CLI do
@@ -125,5 +126,30 @@
         subject
       end
     end
+
+    context 'with the diff switch specified' do
+      let(:arguments) { ['--diff=some-ref'] }
+      let(:config) { Overcommit::ConfigurationLoader.default_configuration }
+
+      before do
+        cli.stub(:halt)
+        Overcommit::HookRunner.any_instance.stub(:run)
+      end
+
+      it 'creates a HookRunner with the diff context' do
+        Overcommit::HookRunner.should_receive(:new).
+          with(config,
+               logger,
+               instance_of(Overcommit::HookContext::Diff),
+               instance_of(Overcommit::Printer)).
+          and_call_original
+        subject
+      end
+
+      it 'runs the HookRunner' do
+        Overcommit::HookRunner.any_instance.should_receive(:run)
+        subject
+      end
+    end
   end
 end

spec/overcommit/hook_context/diff_spec.rb

@@ -0,0 +1,143 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'overcommit/hook_context/diff'
+
+describe Overcommit::HookContext::Diff do
+  let(:config) { double('config') }
+  let(:args) { [] }
+  let(:input) { double('input') }
+  let(:context) { described_class.new(config, args, input, diff: 'master') }
+
+  describe '#modified_files' do
+    subject { context.modified_files }
+
+    context 'when repo contains no files' do
+      around do |example|
+        repo do
+          `git commit --allow-empty -m "Initial commit"`
+          `git checkout -b other-branch 2>&1`
+          example.run
+        end
+      end
+
+      it { should be_empty }
+    end
+
+    context 'when the repo contains files that are unchanged from the ref' do
+      around do |example|
+        repo do
+          touch('some-file')
+          `git add some-file`
+          touch('some-other-file')
+          `git add some-other-file`
+          `git commit -m "Add files"`
+          `git checkout -b other-branch 2>&1`
+          example.run
+        end
+      end
+
+      it { should be_empty }
+    end
+
+    context 'when repo contains files that have been changed from the ref' do
+      around do |example|
+        repo do
+          touch('some-file')
+          `git add some-file`
+          touch('some-other-file')
+          `git add some-other-file`
+          `git commit -m "Add files"`
+          `git checkout -b other-branch 2>&1`
+          File.open('some-file', 'w') { |f| f.write("hello\n") }
+          `git add some-file`
+          `git commit -m "Edit file"`
+          example.run
+        end
+      end
+
+      it { should == %w[some-file].map { |file| File.expand_path(file) } }
+    end
+
+    context 'when repo contains submodules' do
+      around do |example|
+        submodule = repo do
+          touch 'foo'
+          `git add foo`
+          `git commit -m "Initial commit"`
+        end
+
+        repo do
+          `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+          `git commit --allow-empty -m "Initial commit"`
+          `git checkout -b other-branch 2>&1`
+          example.run
+        end
+      end
+
+      it { should_not include File.expand_path('test-sub') }
+    end
+  end
+
+  describe '#modified_lines_in_file' do
+    let(:modified_file) { 'some-file' }
+    subject { context.modified_lines_in_file(modified_file) }
+
+    context 'when file contains a trailing newline' do
+      around do |example|
+        repo do
+          touch(modified_file)
+          `git add #{modified_file}`
+          `git commit -m "Add file"`
+          `git checkout -b other-branch 2>&1`
+          File.open(modified_file, 'w') { |f| (1..3).each { |i| f.write("#{i}\n") } }
+          `git add #{modified_file}`
+          `git commit -m "Edit file"`
+          example.run
+        end
+      end
+
+      it { should == Set.new(1..3) }
+    end
+
+    context 'when file does not contain a trailing newline' do
+      around do |example|
+        repo do
+          touch(modified_file)
+          `git add #{modified_file}`
+          `git commit -m "Add file"`
+          `git checkout -b other-branch 2>&1`
+          File.open(modified_file, 'w') do |f|
+            (1..2).each { |i| f.write("#{i}\n") }
+            f.write(3)
+          end
+          `git add #{modified_file}`
+          `git commit -m "Edit file"`
+          example.run
+        end
+      end
+
+      it { should == Set.new(1..3) }
+    end
+  end
+
+  describe '#hook_type_name' do
+    subject { context.hook_type_name }
+
+    it { should == 'pre_commit' }
+  end
+
+  describe '#hook_script_name' do
+    subject { context.hook_script_name }
+
+    it { should == 'pre-commit' }
+  end
+
+  describe '#initial_commit?' do
+    subject { context.initial_commit? }
+
+    before { Overcommit::GitRepo.stub(:initial_commit?).and_return(true) }
+
+    it { should == true }
+  end
+end