From f57989c2ed15c5d84f198ca334686f5bf7207f8e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 8 Feb 2017 16:22:36 -0500 Subject: [PATCH] Add a spec for our custom GemFetcher cop --- .rubocop.yml | 22 +++++++------ rubocop/cop/gem_fetcher.rb | 23 +++++++++----- rubocop/rubocop.rb | 4 +-- spec/rubocop/cop/gem_fetcher_spec.rb | 46 ++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 spec/rubocop/cop/gem_fetcher_spec.rb 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