diff options
author | Peter Leitzen <pleitzen@gitlab.com> | 2019-08-26 12:24:25 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-08-26 12:24:25 +0000 |
commit | e101a26444f0f02df2c6301f13bc1f3e20781f8b (patch) | |
tree | d745ac1b2f29ae5eb656c5b597e00343d645c5d4 | |
parent | 9e48f7079b648e427d24083af19c3f670cda3206 (diff) | |
download | gitlab-ce-e101a26444f0f02df2c6301f13bc1f3e20781f8b.tar.gz |
Utilize RuboCop's Include/Exclude config
Stop checking the file location programmatically.
-rw-r--r-- | .rubocop.yml | 54 | ||||
-rw-r--r-- | rubocop/cop/gitlab/httparty.rb | 6 | ||||
-rw-r--r-- | rubocop/cop/gitlab/union.rb | 4 | ||||
-rw-r--r-- | rubocop/cop/graphql/authorize_types.rb | 6 | ||||
-rw-r--r-- | rubocop/cop/include_action_view_context.rb | 5 | ||||
-rw-r--r-- | rubocop/cop/include_sidekiq_worker.rb | 5 | ||||
-rw-r--r-- | rubocop/cop/rspec/env_assignment.rb | 5 | ||||
-rw-r--r-- | rubocop/cop/rspec/factories_in_migration_specs.rb | 5 | ||||
-rw-r--r-- | rubocop/cop/sidekiq_options_queue.rb | 5 | ||||
-rw-r--r-- | rubocop/spec_helpers.rb | 30 | ||||
-rw-r--r-- | spec/rubocop/cop/gitlab/union_spec.rb | 6 | ||||
-rw-r--r-- | spec/rubocop/cop/rspec/env_assignment_spec.rb | 26 | ||||
-rw-r--r-- | spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb | 22 |
13 files changed, 67 insertions, 112 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index b75c63e1f58..012f4890c33 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -178,6 +178,9 @@ Gitlab/ModuleWithInstanceVariables: Gitlab/HTTParty: Enabled: true + Exclude: + - 'spec/**/*' + - 'ee/spec/**/*' GitlabSecurity/PublicSend: Enabled: true @@ -211,3 +214,54 @@ ActiveRecordAssociationReload: Exclude: - 'spec/**/*' - 'ee/spec/**/*' + +RSpec/FactoriesInMigrationSpecs: + Enabled: true + Include: + - 'spec/migrations/**/*.rb' + - 'ee/spec/migrations/**/*.rb' + - 'spec/lib/gitlab/background_migration/**/*.rb' + - 'ee/spec/lib/gitlab/background_migration/**/*.rb' + +Cop/IncludeActionViewContext: + Enabled: true + Exclude: + - 'spec/**/*' + - 'ee/spec/**/*' + +Cop/IncludeSidekiqWorker: + Enabled: true + Exclude: + - 'spec/**/*' + - 'ee/spec/**/*' + +Gitlab/Union: + Enabled: true + Exclude: + - 'spec/**/*' + - 'ee/spec/**/*' + +Cop/SidekiqOptionsQueue: + Enabled: true + Exclude: + - 'spec/**/*.rb' + - 'ee/spec/**/*.rb' + +Graphql/AuthorizeTypes: + Enabled: true + Exclude: + - 'spec/**/*.rb' + - 'ee/spec/**/*.rb' + +RSpec/EnvAssignment: + Enable: true + Include: + - 'spec/**/*.rb' + - 'ee/spec/**/*.rb' + Exclude: + - 'spec/**/fast_spec_helper.rb' + - 'ee/spec/**/fast_spec_helper.rb' + - 'spec/**/rails_helper.rb' + - 'ee/spec/**/rails_helper.rb' + - 'spec/**/spec_helper.rb' + - 'ee/spec/**/spec_helper.rb' diff --git a/rubocop/cop/gitlab/httparty.rb b/rubocop/cop/gitlab/httparty.rb index 215f18b6993..8acebff624d 100644 --- a/rubocop/cop/gitlab/httparty.rb +++ b/rubocop/cop/gitlab/httparty.rb @@ -1,11 +1,9 @@ -require_relative '../../spec_helpers' +# frozen_string_literal: true module RuboCop module Cop module Gitlab class HTTParty < RuboCop::Cop::Cop - include SpecHelpers - MSG_SEND = <<~EOL.freeze Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP wrapper. To allow request to localhost or the private network set @@ -27,8 +25,6 @@ module RuboCop PATTERN def on_send(node) - return if in_spec?(node) - add_offense(node, location: :expression, message: MSG_SEND) if httparty_node?(node) add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_httparty?(node) end diff --git a/rubocop/cop/gitlab/union.rb b/rubocop/cop/gitlab/union.rb index 09541d8af3b..c44c847657b 100644 --- a/rubocop/cop/gitlab/union.rb +++ b/rubocop/cop/gitlab/union.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -require_relative '../../spec_helpers' module RuboCop module Cop @@ -7,8 +6,6 @@ module RuboCop # Cop that disallows the use of `Gitlab::SQL::Union`, in favour of using # the `FromUnion` module. class Union < RuboCop::Cop::Cop - include SpecHelpers - MSG = 'Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly' def_node_matcher :raw_union?, <<~PATTERN @@ -17,7 +14,6 @@ module RuboCop def on_send(node) return unless raw_union?(node) - return if in_spec?(node) add_offense(node, location: :expression) end diff --git a/rubocop/cop/graphql/authorize_types.rb b/rubocop/cop/graphql/authorize_types.rb index 93fe80c3edf..cd8bdbaee59 100644 --- a/rubocop/cop/graphql/authorize_types.rb +++ b/rubocop/cop/graphql/authorize_types.rb @@ -1,13 +1,9 @@ # frozen_string_literal: true -require_relative '../../spec_helpers' - module RuboCop module Cop module Graphql class AuthorizeTypes < RuboCop::Cop::Cop - include SpecHelpers - MSG = 'Add an `authorize :ability` call to the type: '\ 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#type-authorization' @@ -32,8 +28,6 @@ module RuboCop private def in_type?(node) - return if in_spec?(node) - path = node.location.expression.source_buffer.name path.include?(TYPES_DIR) diff --git a/rubocop/cop/include_action_view_context.rb b/rubocop/cop/include_action_view_context.rb index 14662a33e95..52c84a711c9 100644 --- a/rubocop/cop/include_action_view_context.rb +++ b/rubocop/cop/include_action_view_context.rb @@ -1,13 +1,9 @@ # frozen_string_literal: true -require_relative '../spec_helpers' - module RuboCop module Cop # Cop that makes sure workers include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`. class IncludeActionViewContext < RuboCop::Cop::Cop - include SpecHelpers - MSG = 'Include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`, for Rails 5.'.freeze def_node_matcher :includes_action_view_context?, <<~PATTERN @@ -15,7 +11,6 @@ module RuboCop PATTERN def on_send(node) - return if in_spec?(node) return unless includes_action_view_context?(node) add_offense(node.arguments.first, location: :expression) diff --git a/rubocop/cop/include_sidekiq_worker.rb b/rubocop/cop/include_sidekiq_worker.rb index 8da4a147219..e69bc018add 100644 --- a/rubocop/cop/include_sidekiq_worker.rb +++ b/rubocop/cop/include_sidekiq_worker.rb @@ -1,11 +1,9 @@ -require_relative '../spec_helpers' +# frozen_string_literal: true module RuboCop module Cop # Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`. class IncludeSidekiqWorker < RuboCop::Cop::Cop - include SpecHelpers - MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze def_node_matcher :includes_sidekiq_worker?, <<~PATTERN @@ -13,7 +11,6 @@ module RuboCop PATTERN def on_send(node) - return if in_spec?(node) return unless includes_sidekiq_worker?(node) add_offense(node.arguments.first, location: :expression) diff --git a/rubocop/cop/rspec/env_assignment.rb b/rubocop/cop/rspec/env_assignment.rb index 8b61fa8e264..73e108c2232 100644 --- a/rubocop/cop/rspec/env_assignment.rb +++ b/rubocop/cop/rspec/env_assignment.rb @@ -1,4 +1,4 @@ -require_relative '../../spec_helpers' +# frozen_string_literal: true module RuboCop module Cop @@ -17,8 +17,6 @@ module RuboCop # stub_env('FOO', 'bar') # end class EnvAssignment < RuboCop::Cop::Cop - include SpecHelpers - MESSAGE = "Don't assign to ENV, use `stub_env` instead.".freeze def_node_search :env_assignment?, <<~PATTERN @@ -28,7 +26,6 @@ module RuboCop # Following is what node.children looks like on a match # [s(:const, nil, :ENV), :[]=, s(:str, "key"), s(:str, "value")] def on_send(node) - return unless in_spec?(node) return unless env_assignment?(node) add_offense(node, location: :expression, message: MESSAGE) diff --git a/rubocop/cop/rspec/factories_in_migration_specs.rb b/rubocop/cop/rspec/factories_in_migration_specs.rb index 0c5aa838a2c..65c7638a0f4 100644 --- a/rubocop/cop/rspec/factories_in_migration_specs.rb +++ b/rubocop/cop/rspec/factories_in_migration_specs.rb @@ -1,4 +1,4 @@ -require_relative '../../spec_helpers' +# frozen_string_literal: true module RuboCop module Cop @@ -14,8 +14,6 @@ module RuboCop # let(:users) { table(:users) } # let(:user) { users.create!(name: 'User 1', username: 'user1') } class FactoriesInMigrationSpecs < RuboCop::Cop::Cop - include SpecHelpers - MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead.".freeze FORBIDDEN_METHODS = %i[build build_list create create_list].freeze @@ -27,7 +25,6 @@ module RuboCop # - Without FactoryBot namespace: [nil, :build, s(:sym, :user)] # - With FactoryBot namespace: [s(:const, nil, :FactoryBot), :build, s(:sym, :user)] def on_send(node) - return unless in_migration_spec?(node) return unless forbidden_factory_usage?(node) method = node.children[1] diff --git a/rubocop/cop/sidekiq_options_queue.rb b/rubocop/cop/sidekiq_options_queue.rb index 253d2958866..499c712175e 100644 --- a/rubocop/cop/sidekiq_options_queue.rb +++ b/rubocop/cop/sidekiq_options_queue.rb @@ -1,11 +1,9 @@ -require_relative '../spec_helpers' +# frozen_string_literal: true module RuboCop module Cop # Cop that prevents manually setting a queue in Sidekiq workers. class SidekiqOptionsQueue < RuboCop::Cop::Cop - include SpecHelpers - MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.'.freeze def_node_matcher :sidekiq_options?, <<~PATTERN @@ -13,7 +11,6 @@ module RuboCop PATTERN def on_send(node) - return if in_spec?(node) return unless sidekiq_options?(node) node.arguments.first.each_node(:pair) do |pair| diff --git a/rubocop/spec_helpers.rb b/rubocop/spec_helpers.rb deleted file mode 100644 index ecd77c4351d..00000000000 --- a/rubocop/spec_helpers.rb +++ /dev/null @@ -1,30 +0,0 @@ -module RuboCop - module SpecHelpers - SPEC_HELPERS = %w[fast_spec_helper.rb rails_helper.rb spec_helper.rb].freeze - MIGRATION_SPEC_DIRECTORIES = ['spec/migrations', 'spec/lib/gitlab/background_migration'].freeze - - # Returns true if the given node originated from the spec directory. - def in_spec?(node) - path = node.location.expression.source_buffer.name - pwd = RuboCop::PathUtil.pwd - - !SPEC_HELPERS.include?(File.basename(path)) && - path.start_with?(File.join(pwd, 'spec'), File.join(pwd, 'ee', 'spec')) - end - - def migration_directories - @migration_directories ||= MIGRATION_SPEC_DIRECTORIES.map do |dir| - pwd = RuboCop::PathUtil.pwd - [File.join(pwd, dir), File.join(pwd, 'ee', dir)] - end.flatten - end - - # Returns true if the given node originated from a migration spec. - def in_migration_spec?(node) - path = node.location.expression.source_buffer.name - - in_spec?(node) && - path.start_with?(*migration_directories) - end - end -end diff --git a/spec/rubocop/cop/gitlab/union_spec.rb b/spec/rubocop/cop/gitlab/union_spec.rb index 5b06f30b25f..f0544fdb66e 100644 --- a/spec/rubocop/cop/gitlab/union_spec.rb +++ b/spec/rubocop/cop/gitlab/union_spec.rb @@ -16,10 +16,4 @@ describe RuboCop::Cop::Gitlab::Union do ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly SOURCE end - - it 'does not flag the use of Gitlab::SQL::Union in a spec' do - allow(cop).to receive(:in_spec?).and_return(true) - - expect_no_offenses('Gitlab::SQL::Union.new([foo])') - end end diff --git a/spec/rubocop/cop/rspec/env_assignment_spec.rb b/spec/rubocop/cop/rspec/env_assignment_spec.rb index 659633f6467..621afbad3ba 100644 --- a/spec/rubocop/cop/rspec/env_assignment_spec.rb +++ b/spec/rubocop/cop/rspec/env_assignment_spec.rb @@ -33,27 +33,13 @@ describe RuboCop::Cop::RSpec::EnvAssignment do end end - context 'in a spec file' do - before do - allow(cop).to receive(:in_spec?).and_return(true) - end - - context 'with a key using single quotes' do - it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY - it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY, %(stub_env('FOO', 'bar')) - end - - context 'with a key using double quotes' do - it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY - it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY, %(stub_env("FOO", 'bar')) - end + context 'with a key using single quotes' do + it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY + it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY, %(stub_env('FOO', 'bar')) end - context 'outside of a spec file' do - it "does not register an offense for `#{OFFENSE_CALL_SINGLE_QUOTES_KEY}` in a non-spec file" do - inspect_source(OFFENSE_CALL_SINGLE_QUOTES_KEY) - - expect(cop.offenses.size).to eq(0) - end + context 'with a key using double quotes' do + it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY + it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY, %(stub_env("FOO", 'bar')) end end diff --git a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb index 2763f2bda21..94324bc615d 100644 --- a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb +++ b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb @@ -8,8 +8,6 @@ require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs' describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do include CopHelper - let(:source_file) { 'spec/migrations/foo_spec.rb' } - subject(:cop) { described_class.new } shared_examples 'an offensive factory call' do |namespace| @@ -27,22 +25,6 @@ describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do end end - context 'in a migration spec file' do - before do - allow(cop).to receive(:in_migration_spec?).and_return(true) - end - - it_behaves_like 'an offensive factory call', '' - it_behaves_like 'an offensive factory call', 'FactoryBot.' - end - - context 'outside of a migration spec file' do - it "does not register an offense" do - expect_no_offenses(<<-RUBY) - describe 'foo' do - let(:user) { create(:user) } - end - RUBY - end - end + it_behaves_like 'an offensive factory call', '' + it_behaves_like 'an offensive factory call', 'FactoryBot.' end |