Add a spec for our custom GemFetcher coprs-gemfetcher-cop-spec

4 files changed, 76 insertions, 19 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index cfff42e5c99..88345373a5b 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -31,8 +31,7 @@ AllCops:
     - 'lib/gitlab/seeder.rb'
     - 'generator_templates/**/*'
 
-
-##################### Style ##################################
+# Style #######################################################################
 
 # Check indentation of private/protected visibility modifiers.
 Style/AccessModifierIndentation:
@@ -471,7 +470,7 @@ Style/WhileUntilModifier:
 Style/WordArray:
   Enabled: false
 
-#################### Metrics ################################
+# Metrics #####################################################################
 
 # A calculated magnitude based on number of assignments,
 # branches, and conditions.
@@ -516,8 +515,7 @@ Metrics/PerceivedComplexity:
   Enabled: true
   Max: 18
 
-
-#################### Lint ################################
+# Lint ########################################################################
 
 # Checks for useless access modifiers.
 Lint/UselessAccessModifier:
@@ -679,8 +677,7 @@ Lint/UselessSetterCall:
 Lint/Void:
   Enabled: true
 
-
-##################### Performance ############################
+# Performance #################################################################
 
 # Use `casecmp` rather than `downcase ==`.
 Performance/Casecmp:
@@ -718,8 +715,7 @@ Performance/StringReplacement:
 Performance/TimesMap:
   Enabled: true
 
-
-##################### Rails ##################################
+# Rails #######################################################################
 
 # Enables Rails cops.
 Rails:
@@ -767,7 +763,7 @@ Rails/ReadWriteAttribute:
 Rails/ScopeArgs:
   Enabled: true
 
-##################### RSpec ##################################
+# RSpec #######################################################################
 
 # Check that instances are not being stubbed globally.
 RSpec/AnyInstance:
@@ -828,3 +824,9 @@ RSpec/NotToNot:
 # Prefer using verifying doubles over normal doubles.
 RSpec/VerifiedDoubles:
   Enabled: false
+
+# Custom ######################################################################
+
+# Disallow the `git` and `github` arguments in the Gemfile.
+GemFetcher:
+  Enabled: true
diff --git a/rubocop/cop/gem_fetcher.rb b/rubocop/cop/gem_fetcher.rb
index 4a63c760744..c199f6acab2 100644
--- a/rubocop/cop/gem_fetcher.rb
+++ b/rubocop/cop/gem_fetcher.rb
@@ -1,17 +1,15 @@
 module RuboCop
   module Cop
-    # Cop that checks for all gems specified in the Gemfile, and will
-    # alert if any gem is to be fetched not from the RubyGems index.
-    # This enforcement is done so as to minimize external build
-    # dependencies and build times.
+    # This cop prevents usage of the `git` and `github` arguments to `gem` in a
+    # `Gemfile` in order to avoid additional points of failure beyond
+    # rubygems.org.
     class GemFetcher < RuboCop::Cop::Cop
       MSG = 'Do not use gems from git repositories, only use gems from RubyGems.'
 
       GIT_KEYS = [:git, :github]
 
       def on_send(node)
-        file_path = node.location.expression.source_buffer.name
-        return unless file_path.end_with?("Gemfile")
+        return unless gemfile?(node)
 
         func_name = node.children[1]
         return unless func_name == :gem
@@ -19,10 +17,21 @@ module RuboCop
         node.children.last.each_node(:pair) do |pair|
           key_name = pair.children[0].children[0].to_sym
           if GIT_KEYS.include?(key_name)
-            add_offense(node, :selector)
+            add_offense(node, pair.source_range, MSG)
           end
         end
       end
+
+      private
+
+      def gemfile?(node)
+        node
+          .location
+          .expression
+          .source_buffer
+          .name
+          .end_with?("Gemfile")
+      end
     end
   end
 end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 3e292a4527c..aa35fb1701c 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -1,4 +1,4 @@
-require_relative 'cop/migration/add_index'
+require_relative 'cop/gem_fetcher'
 require_relative 'cop/migration/add_column'
 require_relative 'cop/migration/add_column_with_default'
-require_relative 'cop/gem_fetcher'
+require_relative 'cop/migration/add_index'
diff --git a/spec/rubocop/cop/gem_fetcher_spec.rb b/spec/rubocop/cop/gem_fetcher_spec.rb
new file mode 100644
index 00000000000..c07f6a831dc
--- /dev/null
+++ b/spec/rubocop/cop/gem_fetcher_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../rubocop/cop/gem_fetcher'
+
+describe RuboCop::Cop::GemFetcher do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'in Gemfile' do
+    before do
+      allow(cop).to receive(:gemfile?).and_return(true)
+    end
+
+    it 'registers an offense when a gem uses `git`' do
+      inspect_source(cop, 'gem "foo", git: "https://gitlab.com/foo/bar.git"')
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+        expect(cop.highlights).to eq(['git: "https://gitlab.com/foo/bar.git"'])
+      end
+    end
+
+    it 'registers an offense when a gem uses `github`' do
+      inspect_source(cop, 'gem "foo", github: "foo/bar.git"')
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+        expect(cop.highlights).to eq(['github: "foo/bar.git"'])
+      end
+    end
+  end
+
+  context 'outside of Gemfile' do
+    it 'registers no offense' do
+      inspect_source(cop, 'gem "foo", git: "https://gitlab.com/foo/bar.git"')
+
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+end