diff options
Diffstat (limited to 'spec/rubocop/cop')
12 files changed, 398 insertions, 33 deletions
diff --git a/spec/rubocop/cop/api/base_spec.rb b/spec/rubocop/cop/api/base_spec.rb new file mode 100644 index 00000000000..893bcf49627 --- /dev/null +++ b/spec/rubocop/cop/api/base_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/api/base' + +RSpec.describe RuboCop::Cop::API::Base, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + let(:corrected) do + <<~CORRECTED + class SomeAPI < ::API::Base + end + CORRECTED + end + + ['Grape::API', '::Grape::API', 'Grape::API::Instance', '::Grape::API::Instance'].each do |offense| + it "adds an offense when inheriting from #{offense}" do + expect_offense(<<~CODE) + class SomeAPI < #{offense} + #{'^' * offense.length} #{described_class::MSG} + end + CODE + + expect_correction(corrected) + end + end + + it 'does not add an offense when inheriting from BaseAPI' do + expect_no_offenses(corrected) + end +end diff --git a/spec/rubocop/cop/api/grape_api_instance_spec.rb b/spec/rubocop/cop/api/grape_api_instance_spec.rb deleted file mode 100644 index 74f175cb707..00000000000 --- a/spec/rubocop/cop/api/grape_api_instance_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' -require 'rubocop' -require_relative '../../../../rubocop/cop/api/grape_api_instance' - -RSpec.describe RuboCop::Cop::API::GrapeAPIInstance do - include CopHelper - - subject(:cop) { described_class.new } - - it 'adds an offense when inheriting from Grape::API' do - inspect_source(<<~CODE) - class SomeAPI < Grape::API - end - CODE - - expect(cop.offenses.size).to eq(1) - end - - it 'does not add an offense when inheriting from Grape::API::Instance' do - inspect_source(<<~CODE) - class SomeAPI < Grape::API::Instance - end - CODE - - expect(cop.offenses.size).to be_zero - end -end diff --git a/spec/rubocop/cop/code_reuse/active_record_spec.rb b/spec/rubocop/cop/code_reuse/active_record_spec.rb index 25eca185f26..e15b9e11aed 100644 --- a/spec/rubocop/cop/code_reuse/active_record_spec.rb +++ b/spec/rubocop/cop/code_reuse/active_record_spec.rb @@ -84,7 +84,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do SOURCE end - it 'autocorrects offenses in instance methods by whitelisting them' do + it 'autocorrects offenses in instance methods by allowing them' do corrected = autocorrect_source(<<~SOURCE) def foo User.where @@ -100,7 +100,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do SOURCE end - it 'autocorrects offenses in class methods by whitelisting them' do + it 'autocorrects offenses in class methods by allowing them' do corrected = autocorrect_source(<<~SOURCE) def self.foo User.where @@ -116,7 +116,7 @@ RSpec.describe RuboCop::Cop::CodeReuse::ActiveRecord, type: :rubocop do SOURCE end - it 'autocorrects offenses in blocks by whitelisting them' do + it 'autocorrects offenses in blocks by allowing them' do corrected = autocorrect_source(<<~SOURCE) get '/' do User.where diff --git a/spec/rubocop/cop/graphql/gid_expected_type_spec.rb b/spec/rubocop/cop/graphql/gid_expected_type_spec.rb new file mode 100644 index 00000000000..a81af2aea5d --- /dev/null +++ b/spec/rubocop/cop/graphql/gid_expected_type_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' + +require_relative '../../../../rubocop/cop/graphql/gid_expected_type' + +RSpec.describe RuboCop::Cop::Graphql::GIDExpectedType, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + it 'adds an offense when there is no expected_type parameter' do + inspect_source(<<~TYPE) + GitlabSchema.object_from_id(received_id) + TYPE + + expect(cop.offenses.size).to eq 1 + end + + it 'does not add an offense for calls that have an expected_type parameter' do + expect_no_offenses(<<~TYPE.strip) + GitlabSchema.object_from_id("some_id", expected_type: SomeClass) + TYPE + end +end diff --git a/spec/rubocop/cop/graphql/id_type_spec.rb b/spec/rubocop/cop/graphql/id_type_spec.rb new file mode 100644 index 00000000000..8767412e282 --- /dev/null +++ b/spec/rubocop/cop/graphql/id_type_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' + +require_relative '../../../../rubocop/cop/graphql/id_type' + +RSpec.describe RuboCop::Cop::Graphql::IDType, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + it 'adds an offense when GraphQL::ID_TYPE is used as a param to #argument' do + inspect_source(<<~TYPE) + argument :some_arg, GraphQL::ID_TYPE, some: other, params: do_not_matter + TYPE + + expect(cop.offenses.size).to eq 1 + end + + context 'whitelisted arguments' do + RuboCop::Cop::Graphql::IDType::WHITELISTED_ARGUMENTS.each do |arg| + it "does not add an offense for calls to #argument with #{arg} as argument name" do + expect_no_offenses(<<~TYPE.strip) + argument #{arg}, GraphQL::ID_TYPE, some: other, params: do_not_matter + TYPE + end + end + end + + it 'does not add an offense for calls to #argument without GraphQL::ID_TYPE' do + expect_no_offenses(<<~TYPE.strip) + argument :some_arg, ::Types::GlobalIDType[::Awardable], some: other, params: do_not_matter + TYPE + end +end diff --git a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb index b43d44dba65..aaf191a1b6b 100644 --- a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb +++ b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb @@ -36,5 +36,15 @@ RSpec.describe RuboCop::Cop::Migration::AddConcurrentForeignKey, type: :rubocop expect(cop.offenses).to be_empty end + + it 'does not register an offense when `add_foreign_key` is within `with_lock_retries`' do + inspect_source <<~RUBY + with_lock_retries do + add_foreign_key :key, :projects, column: :project_id, on_delete: :cascade + end + RUBY + + expect(cop.offenses).to be_empty + end end end diff --git a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb index 5f0ca419548..0bea7bd7a0c 100644 --- a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb +++ b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb @@ -129,6 +129,28 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns, type: :rubocop do end end + context 'when text columns are used for encryption' do + it 'registers no offenses' do + expect_no_offenses(<<~RUBY) + class TestTextLimits < ActiveRecord::Migration[6.0] + DOWNTIME = false + disable_ddl_transaction! + + def up + create_table :test_text_limits, id: false do |t| + t.integer :test_id, null: false + t.text :encrypted_name + end + + add_column :encrypted_test_text_limits, :encrypted_email, :text + add_column_with_default :encrypted_test_text_limits, :encrypted_role, :text, default: 'default' + change_column_type_concurrently :encrypted_test_text_limits, :encrypted_test_id, :text + end + end + RUBY + end + end + context 'on down' do it 'registers no offense' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb index fa4acc62226..93f43b0feb0 100644 --- a/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb +++ b/spec/rubocop/cop/migration/create_table_with_foreign_keys_spec.rb @@ -83,7 +83,7 @@ RSpec.describe RuboCop::Cop::Migration::CreateTableWithForeignKeys, type: :ruboc context 'with more than one foreign keys' do let(:offense) do 'Creating a table with more than one foreign key at once violates our migration style guide. ' \ - 'For more details check the https://docs.gitlab.com/ce/development/migration_style_guide.html#examples' + 'For more details check the https://docs.gitlab.com/ee/development/migration_style_guide.html#examples' end shared_examples 'target to high traffic table' do |dsl_method, table_name| diff --git a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb index 11e4d784617..607daf0c9f0 100644 --- a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb +++ b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb @@ -53,6 +53,22 @@ RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod, type: : expect(cop.offenses.size).to eq(0) end + + describe 'for `add_foreign_key`' do + it 'registers an offense when more than two FKs are added' do + message = described_class::MSG_ONLY_ONE_FK_ALLOWED + + expect_offense <<~RUBY + with_lock_retries do + add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade + ^^^^^^^^^^^^^^^ #{message} + add_column :projects, :name, :text + add_foreign_key :imports, :users, column: :user_id, on_delete: :cascade + ^^^^^^^^^^^^^^^ #{message} + end + RUBY + end + end end context 'outside of migration' do diff --git a/spec/rubocop/cop/rspec/expect_gitlab_tracking_spec.rb b/spec/rubocop/cop/rspec/expect_gitlab_tracking_spec.rb new file mode 100644 index 00000000000..f7adc1373df --- /dev/null +++ b/spec/rubocop/cop/rspec/expect_gitlab_tracking_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/rspec/expect_gitlab_tracking' + +RSpec.describe RuboCop::Cop::RSpec::ExpectGitlabTracking do + include CopHelper + + let(:source_file) { 'spec/foo_spec.rb' } + + subject(:cop) { described_class.new } + + good_samples = [ + 'expect_snowplow_event(category: nil, action: nil)', + 'expect_snowplow_event(category: "EventCategory", action: "event_action")', + 'expect_snowplow_event(category: "EventCategory", action: "event_action", label: "label", property: "property")', + 'expect_no_snowplow_event' + ] + + bad_samples = [ + 'expect(Gitlab::Tracking).to receive(:event)', + 'expect(Gitlab::Tracking).to_not receive(:event)', + 'expect(Gitlab::Tracking).not_to receive(:event)', + 'expect(Gitlab::Tracking).to_not receive(:event).with("EventCategory", "event_action")', + 'expect(Gitlab::Tracking).not_to receive(:event).with("EventCategory", "event_action")', + 'expect(Gitlab::Tracking).to receive(:event).with("EventCategory", "event_action", label: "label", property: "property")', + 'expect(Gitlab::Tracking).to have_received(:event).with("EventCategory", "event_action")', + 'expect(Gitlab::Tracking).to_not have_received(:event).with("EventCategory", "event_action")', + 'expect(Gitlab::Tracking).not_to have_received(:event).with("EventCategory", "event_action")', + 'allow(Gitlab::Tracking).to receive(:event).and_call_original' + ] + + good_samples.each do |good| + context "good: #{good}" do + it 'does not register an offense' do + inspect_source(good) + + expect(cop.offenses).to be_empty + end + end + end + + bad_samples.each do |bad| + context "bad: #{bad}" do + it 'registers an offense', :aggregate_failures do + inspect_source(bad, source_file) + + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + expect(cop.highlights).to eq([bad]) + + msg = cop.offenses.first.message + + expect(msg).to match( + /Do not expect directly on `Gitlab::Tracking#event`/ + ) + expect(msg).to match(/add the `snowplow` annotation/) + expect(msg).to match(/use `expect_snowplow_event` instead/) + end + end + end +end diff --git a/spec/rubocop/cop/rspec/factory_bot/inline_association_spec.rb b/spec/rubocop/cop/rspec/factory_bot/inline_association_spec.rb new file mode 100644 index 00000000000..70dbe086127 --- /dev/null +++ b/spec/rubocop/cop/rspec/factory_bot/inline_association_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' +require 'rubocop' + +require_relative '../../../../../rubocop/cop/rspec/factory_bot/inline_association' + +RSpec.describe RuboCop::Cop::RSpec::FactoryBot::InlineAssociation, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + shared_examples 'offense' do |code_snippet, autocorrected| + # We allow `create` or `FactoryBot.create` or `::FactoryBot.create` + let(:type) { code_snippet[/^(?:::)?(?:FactoryBot\.)?(\w+)/, 1] } + let(:offense_marker) { '^' * code_snippet.size } + let(:offense_msg) { msg(type) } + let(:offense) { "#{offense_marker} #{offense_msg}" } + let(:pristine_source) { source.sub(offense, '') } + let(:source) do + <<~RUBY + FactoryBot.define do + factory :project do + attribute { #{code_snippet} } + #{offense} + end + end + RUBY + end + + it 'registers an offense' do + expect_offense(source) + end + + it 'autocorrects the source' do + corrected = autocorrect_source(pristine_source) + + expect(corrected).not_to include(code_snippet) + expect(corrected).to include(autocorrected) + end + end + + shared_examples 'no offense' do |code_snippet| + first_line = code_snippet.lines.first.chomp + + context "for `#{first_line}`" do + it 'does not register any offenses' do + expect_no_offenses <<~RUBY + FactoryBot.define do + factory :project do + #{code_snippet} + end + end + RUBY + end + end + end + + context 'offenses' do + using RSpec::Parameterized::TableSyntax + + where(:code_snippet, :autocorrected) do + # create + 'create(:user)' | 'association(:user)' + 'FactoryBot.create(:user)' | 'association(:user)' + '::FactoryBot.create(:user)' | 'association(:user)' + 'create(:user, :admin)' | 'association(:user, :admin)' + 'create(:user, name: "any")' | 'association(:user, name: "any")' + # build + 'build(:user)' | 'association(:user)' + 'FactoryBot.build(:user)' | 'association(:user)' + '::FactoryBot.build(:user)' | 'association(:user)' + 'build(:user, :admin)' | 'association(:user, :admin)' + 'build(:user, name: "any")' | 'association(:user, name: "any")' + end + + with_them do + include_examples 'offense', params[:code_snippet], params[:autocorrected] + end + + it 'recognizes `add_attribute`' do + expect_offense <<~RUBY + FactoryBot.define do + factory :project, class: 'Project' do + add_attribute(:method) { create(:user) } + ^^^^^^^^^^^^^ #{msg(:create)} + end + end + RUBY + end + + it 'recognizes `transient` attributes' do + expect_offense <<~RUBY + FactoryBot.define do + factory :project, class: 'Project' do + transient do + creator { create(:user) } + ^^^^^^^^^^^^^ #{msg(:create)} + end + end + end + RUBY + end + end + + context 'no offenses' do + include_examples 'no offense', 'association(:user)' + include_examples 'no offense', 'association(:user, :admin)' + include_examples 'no offense', 'association(:user, name: "any")' + + include_examples 'no offense', <<~RUBY + after(:build) do |object| + object.user = create(:user) + end + RUBY + + include_examples 'no offense', <<~RUBY + initialize_with do + create(:user) + end + RUBY + + include_examples 'no offense', <<~RUBY + user_id { create(:user).id } + RUBY + end + + def msg(type) + format(described_class::MSG, type: type) + end +end diff --git a/spec/rubocop/cop/rspec/timecop_travel_spec.rb b/spec/rubocop/cop/rspec/timecop_travel_spec.rb new file mode 100644 index 00000000000..25a8127d40e --- /dev/null +++ b/spec/rubocop/cop/rspec/timecop_travel_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/rspec/timecop_travel' + +RSpec.describe RuboCop::Cop::RSpec::TimecopTravel, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + context 'when calling Timecop.travel' do + let(:source) do + <<~SRC + Timecop.travel(1.day.ago) { create(:issue) } + SRC + end + + let(:corrected_source) do + <<~SRC + travel_to(1.day.ago) { create(:issue) } + SRC + end + + it 'registers an offence' do + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + end + + it 'can autocorrect the source' do + expect(autocorrect_source(source)).to eq(corrected_source) + end + end + + context 'when calling a different method on Timecop' do + let(:source) do + <<~SRC + Timecop.freeze { create(:issue) } + SRC + end + + it 'does not register an offence' do + inspect_source(source) + + expect(cop.offenses).to be_empty + end + end +end |