Commit 519a130c authored by Sean McGivern's avatar Sean McGivern

Merge branch 'pl-rubocop-include-exclude' into 'master'

Utilize RuboCop's Include/Exclude config

See merge request gitlab-org/gitlab-ce!32098
parents 9e48f707 e101a264
...@@ -178,6 +178,9 @@ Gitlab/ModuleWithInstanceVariables: ...@@ -178,6 +178,9 @@ Gitlab/ModuleWithInstanceVariables:
Gitlab/HTTParty: Gitlab/HTTParty:
Enabled: true Enabled: true
Exclude:
- 'spec/**/*'
- 'ee/spec/**/*'
GitlabSecurity/PublicSend: GitlabSecurity/PublicSend:
Enabled: true Enabled: true
...@@ -211,3 +214,54 @@ ActiveRecordAssociationReload: ...@@ -211,3 +214,54 @@ ActiveRecordAssociationReload:
Exclude: Exclude:
- 'spec/**/*' - 'spec/**/*'
- 'ee/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'
require_relative '../../spec_helpers' # frozen_string_literal: true
module RuboCop module RuboCop
module Cop module Cop
module Gitlab module Gitlab
class HTTParty < RuboCop::Cop::Cop class HTTParty < RuboCop::Cop::Cop
include SpecHelpers
MSG_SEND = <<~EOL.freeze MSG_SEND = <<~EOL.freeze
Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP
wrapper. To allow request to localhost or the private network set wrapper. To allow request to localhost or the private network set
...@@ -27,8 +25,6 @@ module RuboCop ...@@ -27,8 +25,6 @@ module RuboCop
PATTERN PATTERN
def on_send(node) 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_SEND) if httparty_node?(node)
add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_httparty?(node) add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_httparty?(node)
end end
......
# frozen_string_literal: true # frozen_string_literal: true
require_relative '../../spec_helpers'
module RuboCop module RuboCop
module Cop module Cop
...@@ -7,8 +6,6 @@ module RuboCop ...@@ -7,8 +6,6 @@ module RuboCop
# Cop that disallows the use of `Gitlab::SQL::Union`, in favour of using # Cop that disallows the use of `Gitlab::SQL::Union`, in favour of using
# the `FromUnion` module. # the `FromUnion` module.
class Union < RuboCop::Cop::Cop class Union < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly' MSG = 'Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly'
def_node_matcher :raw_union?, <<~PATTERN def_node_matcher :raw_union?, <<~PATTERN
...@@ -17,7 +14,6 @@ module RuboCop ...@@ -17,7 +14,6 @@ module RuboCop
def on_send(node) def on_send(node)
return unless raw_union?(node) return unless raw_union?(node)
return if in_spec?(node)
add_offense(node, location: :expression) add_offense(node, location: :expression)
end end
......
# frozen_string_literal: true # frozen_string_literal: true
require_relative '../../spec_helpers'
module RuboCop module RuboCop
module Cop module Cop
module Graphql module Graphql
class AuthorizeTypes < RuboCop::Cop::Cop class AuthorizeTypes < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Add an `authorize :ability` call to the type: '\ MSG = 'Add an `authorize :ability` call to the type: '\
'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#type-authorization' 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#type-authorization'
...@@ -32,8 +28,6 @@ module RuboCop ...@@ -32,8 +28,6 @@ module RuboCop
private private
def in_type?(node) def in_type?(node)
return if in_spec?(node)
path = node.location.expression.source_buffer.name path = node.location.expression.source_buffer.name
path.include?(TYPES_DIR) path.include?(TYPES_DIR)
......
# frozen_string_literal: true # frozen_string_literal: true
require_relative '../spec_helpers'
module RuboCop module RuboCop
module Cop module Cop
# Cop that makes sure workers include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`. # Cop that makes sure workers include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`.
class IncludeActionViewContext < RuboCop::Cop::Cop class IncludeActionViewContext < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`, for Rails 5.'.freeze MSG = 'Include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`, for Rails 5.'.freeze
def_node_matcher :includes_action_view_context?, <<~PATTERN def_node_matcher :includes_action_view_context?, <<~PATTERN
...@@ -15,7 +11,6 @@ module RuboCop ...@@ -15,7 +11,6 @@ module RuboCop
PATTERN PATTERN
def on_send(node) def on_send(node)
return if in_spec?(node)
return unless includes_action_view_context?(node) return unless includes_action_view_context?(node)
add_offense(node.arguments.first, location: :expression) add_offense(node.arguments.first, location: :expression)
......
require_relative '../spec_helpers' # frozen_string_literal: true
module RuboCop module RuboCop
module Cop module Cop
# Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`. # Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`.
class IncludeSidekiqWorker < RuboCop::Cop::Cop class IncludeSidekiqWorker < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze
def_node_matcher :includes_sidekiq_worker?, <<~PATTERN def_node_matcher :includes_sidekiq_worker?, <<~PATTERN
...@@ -13,7 +11,6 @@ module RuboCop ...@@ -13,7 +11,6 @@ module RuboCop
PATTERN PATTERN
def on_send(node) def on_send(node)
return if in_spec?(node)
return unless includes_sidekiq_worker?(node) return unless includes_sidekiq_worker?(node)
add_offense(node.arguments.first, location: :expression) add_offense(node.arguments.first, location: :expression)
......
require_relative '../../spec_helpers' # frozen_string_literal: true
module RuboCop module RuboCop
module Cop module Cop
...@@ -17,8 +17,6 @@ module RuboCop ...@@ -17,8 +17,6 @@ module RuboCop
# stub_env('FOO', 'bar') # stub_env('FOO', 'bar')
# end # end
class EnvAssignment < RuboCop::Cop::Cop class EnvAssignment < RuboCop::Cop::Cop
include SpecHelpers
MESSAGE = "Don't assign to ENV, use `stub_env` instead.".freeze MESSAGE = "Don't assign to ENV, use `stub_env` instead.".freeze
def_node_search :env_assignment?, <<~PATTERN def_node_search :env_assignment?, <<~PATTERN
...@@ -28,7 +26,6 @@ module RuboCop ...@@ -28,7 +26,6 @@ module RuboCop
# Following is what node.children looks like on a match # Following is what node.children looks like on a match
# [s(:const, nil, :ENV), :[]=, s(:str, "key"), s(:str, "value")] # [s(:const, nil, :ENV), :[]=, s(:str, "key"), s(:str, "value")]
def on_send(node) def on_send(node)
return unless in_spec?(node)
return unless env_assignment?(node) return unless env_assignment?(node)
add_offense(node, location: :expression, message: MESSAGE) add_offense(node, location: :expression, message: MESSAGE)
......
require_relative '../../spec_helpers' # frozen_string_literal: true
module RuboCop module RuboCop
module Cop module Cop
...@@ -14,8 +14,6 @@ module RuboCop ...@@ -14,8 +14,6 @@ module RuboCop
# let(:users) { table(:users) } # let(:users) { table(:users) }
# let(:user) { users.create!(name: 'User 1', username: 'user1') } # let(:user) { users.create!(name: 'User 1', username: 'user1') }
class FactoriesInMigrationSpecs < RuboCop::Cop::Cop class FactoriesInMigrationSpecs < RuboCop::Cop::Cop
include SpecHelpers
MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead.".freeze MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead.".freeze
FORBIDDEN_METHODS = %i[build build_list create create_list].freeze FORBIDDEN_METHODS = %i[build build_list create create_list].freeze
...@@ -27,7 +25,6 @@ module RuboCop ...@@ -27,7 +25,6 @@ module RuboCop
# - Without FactoryBot namespace: [nil, :build, s(:sym, :user)] # - Without FactoryBot namespace: [nil, :build, s(:sym, :user)]
# - With FactoryBot namespace: [s(:const, nil, :FactoryBot), :build, s(:sym, :user)] # - With FactoryBot namespace: [s(:const, nil, :FactoryBot), :build, s(:sym, :user)]
def on_send(node) def on_send(node)
return unless in_migration_spec?(node)
return unless forbidden_factory_usage?(node) return unless forbidden_factory_usage?(node)
method = node.children[1] method = node.children[1]
......
require_relative '../spec_helpers' # frozen_string_literal: true
module RuboCop module RuboCop
module Cop module Cop
# Cop that prevents manually setting a queue in Sidekiq workers. # Cop that prevents manually setting a queue in Sidekiq workers.
class SidekiqOptionsQueue < RuboCop::Cop::Cop class SidekiqOptionsQueue < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.'.freeze MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.'.freeze
def_node_matcher :sidekiq_options?, <<~PATTERN def_node_matcher :sidekiq_options?, <<~PATTERN
...@@ -13,7 +11,6 @@ module RuboCop ...@@ -13,7 +11,6 @@ module RuboCop
PATTERN PATTERN
def on_send(node) def on_send(node)
return if in_spec?(node)
return unless sidekiq_options?(node) return unless sidekiq_options?(node)
node.arguments.first.each_node(:pair) do |pair| node.arguments.first.each_node(:pair) do |pair|
......
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
...@@ -16,10 +16,4 @@ describe RuboCop::Cop::Gitlab::Union do ...@@ -16,10 +16,4 @@ describe RuboCop::Cop::Gitlab::Union do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly
SOURCE SOURCE
end 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 end
...@@ -33,11 +33,6 @@ describe RuboCop::Cop::RSpec::EnvAssignment do ...@@ -33,11 +33,6 @@ describe RuboCop::Cop::RSpec::EnvAssignment do
end end
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 context 'with a key using single quotes' do
it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY 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')) it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_SINGLE_QUOTES_KEY, %(stub_env('FOO', 'bar'))
...@@ -47,13 +42,4 @@ describe RuboCop::Cop::RSpec::EnvAssignment do ...@@ -47,13 +42,4 @@ describe RuboCop::Cop::RSpec::EnvAssignment do
it_behaves_like 'an offensive ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY 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')) it_behaves_like 'an autocorrected ENV#[]= call', OFFENSE_CALL_DOUBLE_QUOTES_KEY, %(stub_env("FOO", 'bar'))
end end
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
end
end end
...@@ -8,8 +8,6 @@ require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs' ...@@ -8,8 +8,6 @@ require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs'
describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do
include CopHelper include CopHelper
let(:source_file) { 'spec/migrations/foo_spec.rb' }
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
shared_examples 'an offensive factory call' do |namespace| shared_examples 'an offensive factory call' do |namespace|
...@@ -27,22 +25,6 @@ describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do ...@@ -27,22 +25,6 @@ describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do
end end
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', ''
it_behaves_like 'an offensive factory call', 'FactoryBot.' 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment