Remove redundant helper that was making a wrong impression we cover Include/Exclude in specs

[pirj]

We were overriding the inspected file name to be _spec.rb, however, RuboCop seems to inspect offences passed to expect_offense with no regards to the file name.

All specs are green, and with that removal, I lose some certainty in our specs that the commissioner would consider our Include.

If you remove Include, the spec that fails is:

RuboCop::Cop::RSpec::Base when the file is a source file without "spec" in the name ignores the source when the path is not a specified pattern

This behaviour remains the same on master and with the removal of our ExpectOffence/inspected_source_filename.

Thoughts?

Before submitting the PR make sure the following are checked:

• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• [-] Added tests.
• [-] Updated documentation.
• [-] Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
• The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

[pirj]

@bquorning @Darhazer @ydah @r7kamura thoughts?

[pirj]

I've tracked it down to this commit, and nothing fails if I change:

module ExpectViolation
-  DEFAULT_FILENAME = 'example_spec.rb'.freeze
+  DEFAULT_FILENAME = 'example.rb'.freeze

We seem to need to have another way of covering cop's Include/Exclude configuration.

[pirj]

@koic Need your opinion here, as there are quite a lot of Include:/Exclude: directives in rubocop-rails's default config. How do you make sure only those patterns are inspected, and that they are inspected?

[ydah]

I think it would be possible to add a test for behavior when an Include:/Exclude: path is specified below, so that the test is aware of the file path.

• https://github.com/rubocop/rubocop-rspec/blob/master/spec/rubocop/cli/autocorrect_spec.rb

[ydah]

It seems to be possible to inspect here that the behavior is different between the case of paths to be Include: and other paths:

rubocop-rspec/spec/rubocop/cli/autocorrect_spec.rb

Line 22 in d2bc8e7

create_file('spec/example.rb', <<-RUBY)

[r7kamura]

I agree with the change policy of this removing redundant expectation helpers. At least, providing a helper with the same name and different behavior (e.g. #expect_offense) should be avoided in terms of cognitive load.

Perhaps you mentioned me because of my use of #inspected_source_filename in this pull request:

• Add RSpec/FactoryBot/AssociationStyle cop #1414

This in itself is not a problem for me, as all I need to do is to change as follows:

Details

diff --git a/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb b/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb
index b4f3a68..87f10af 100644
--- a/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb
+++ b/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb
@@ -1,20 +1,20 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
-  def inspected_source_filename
-    'spec/factories.rb'
-  end
-
   let(:cop_config) do
     { 'EnforcedStyle' => enforced_style }
   end
 
+  let(:inspected_file_path) do
+    'spec/factories.rb'
+  end
+
   context 'when EnforcedStyle is :implicit' do
     let(:enforced_style) { :implicit }
 
     context 'when factory block is empty' do
       it 'does not register an offense' do
-        expect_no_offenses(<<~RUBY)
+        expect_no_offenses(<<~RUBY, inspected_file_path)
           factory :user do
           end
         RUBY
@@ -23,7 +23,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'with when factory has no block' do
       it 'does not register an offense' do
-        expect_no_offenses(<<~RUBY)
+        expect_no_offenses(<<~RUBY, inspected_file_path)
           factory :user
         RUBY
       end
@@ -31,7 +31,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when implicit style is used' do
       it 'does not register an offense' do
-        expect_no_offenses(<<~RUBY)
+        expect_no_offenses(<<~RUBY, inspected_file_path)
           factory :article do
             user
           end
@@ -41,7 +41,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called in attribute block' do
       it 'does not register an offense' do
-        expect_no_offenses(<<~RUBY)
+        expect_no_offenses(<<~RUBY, inspected_file_path)
           factory :article do
             author do
               association :user
@@ -53,7 +53,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` has only 1 argument' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :user
             ^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -70,7 +70,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called in trait block' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             trait :with_user do
               association :user
@@ -91,7 +91,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called with trait' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :user, :admin
             ^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -108,7 +108,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called with factory option' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :author, factory: :user
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -125,7 +125,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called with array factory option' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :author, factory: %i[user]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -143,7 +143,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
     context 'when `association` is called with trait arguments and factory' \
             'option' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :author, :admin, factory: :user
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -160,7 +160,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called with traits option' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :author, traits: %i[admin]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -177,7 +177,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when `association` is called with factory and traits options' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :author, factory: :user, traits: [:admin]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -195,7 +195,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
     context 'when `association` is called with trait arguments and factory' \
             'and traits options' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             association :author, :active, factory: :user, traits: [:admin]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations.
@@ -216,7 +216,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when explicit style is used' do
       it 'does not register an offense' do
-        expect_no_offenses(<<~RUBY)
+        expect_no_offenses(<<~RUBY, inspected_file_path)
           factory :article do
             association :user
           end
@@ -226,7 +226,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when implicit association is used without any arguments' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             user
             ^^^^ Use explicit style to define associations.
@@ -243,7 +243,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when implicit association is used with arguments' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             author factory: :user
             ^^^^^^^^^^^^^^^^^^^^^ Use explicit style to define associations.
@@ -260,7 +260,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when one of NonImplicitAssociationMethodNames is used' do
       it 'does not register an offense' do
-        expect_no_offenses(<<~RUBY)
+        expect_no_offenses(<<~RUBY, inspected_file_path)
           factory :article do
             skip_create
           end
@@ -270,7 +270,7 @@ RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AssociationStyle do
 
     context 'when implicit association is called in trait block' do
       it 'registers and corrects an offense' do
-        expect_offense(<<~RUBY)
+        expect_offense(<<~RUBY, inspected_file_path)
           factory :article do
             trait :with_user do
               user

I don't know if I'm getting the background problem right, but I don't have a good answer to the concern about wanting a comprehensive inspection to see if we are really following the policy of Include in this gem. I also maintain rubocop-migration gem, and I always write Include: ["db/migrate/**/*.rb"] in its config/default.yml, but sometimes I forget to write it and it causes problems 😇

[pirj]

@r7kamura Thanks for the valuable input!
It made me realize that we do indeed cover that the file name matching to Include/Exclude is done. But it only works if we have those overridden helpers that set the filename.