Remove relevance detection code

We have had problems with it: - it was slow - it is a non-standard extension of RuboCop's config - Include option exists - department-specific (e.g. FactoryBot) config was ignored
rubocop · Oct 28, 2020 · 4920879 · 4920879
1 parent d0588f2
commit 4920879
Show file tree

Hide file tree

Showing 14 changed files with 111 additions and 107 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 * Remove deprecated class `::RuboCop::Cop::RSpec::Cop`. ([@bquorning][])
 * Retire `RSpec/InvalidPredicateMatcher` cop. ([@pirj][])
+* Remove the code responsible for filtering files to inspect. ([@pirj][])
 
 ## 2.0.0.pre (2020-10-22)
 

diff --git a/config/default.yml b/config/default.yml
@@ -1,14 +1,8 @@
 ---
-AllCops:
-  RSpec:
-    Patterns:
-    - _spec.rb
-    - "(?:^|/)spec/"
-  RSpec/FactoryBot:
-    Patterns:
-    - spec/factories.rb
-    - spec/factories/**/*.rb
-    - features/support/factories/**/*.rb
+RSpec:
+  Include:
+  - "**/*_spec.rb"
+  - "**/spec/**/*"
 
 RSpec/AlignLeftLetBrace:
   Description: Checks that left braces for adjacent single line lets are aligned.
@@ -241,6 +235,9 @@ RSpec/ExpectOutput:
 RSpec/FilePath:
   Description: Checks that spec file paths are consistent and well-formed.
   Enabled: true
+  Include:
+  - "**/*_spec*rb*"
+  - "**/spec/**/*"
   CustomTransform:
     RuboCop: rubocop
     RSpec: rspec
@@ -639,13 +636,23 @@ RSpec/Capybara/VisibilityMatcher:
 RSpec/FactoryBot/AttributeDefinedStatically:
   Description: Always declare attribute values as blocks.
   Enabled: true
+  Include:
+  - spec/factories.rb
+  - spec/factories/**/*.rb
+  - features/support/factories/**/*.rb
   VersionAdded: '1.28'
   VersionChanged: '2.0'
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/AttributeDefinedStatically
 
 RSpec/FactoryBot/CreateList:
   Description: Checks for create_list usage.
   Enabled: true
+  Include:
+  - "**/*_spec.rb"
+  - "**/spec/**/*"
+  - spec/factories.rb
+  - spec/factories/**/*.rb
+  - features/support/factories/**/*.rb
   EnforcedStyle: create_list
   SupportedStyles:
   - create_list
@@ -657,6 +664,10 @@ RSpec/FactoryBot/CreateList:
 RSpec/FactoryBot/FactoryClassName:
   Description: Use string value when setting the class attribute explicitly.
   Enabled: true
+  Include:
+  - spec/factories.rb
+  - spec/factories/**/*.rb
+  - features/support/factories/**/*.rb
   VersionAdded: '1.37'
   VersionChanged: '2.0'
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/FactoryClassName

diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc
@@ -1536,6 +1536,10 @@ my_class_spec.rb         # describe MyClass, '#method'
 |===
 | Name | Default value | Configurable values
 
+| Include
+| `+**/*_spec*rb*+`, `+**/spec/**/*+`
+| Array
+
 | CustomTransform
 | `{"RuboCop"=>"rubocop", "RSpec"=>"rspec"}`
 | 

diff --git a/docs/modules/ROOT/pages/cops_rspec/factorybot.adoc b/docs/modules/ROOT/pages/cops_rspec/factorybot.adoc
@@ -37,6 +37,16 @@ count 1
 count { 1 }
 ----
 
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| Include
+| `spec/factories.rb`, `spec/factories/**/*.rb`, `features/support/factories/**/*.rb`
+| Array
+|===
+
 === References
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/AttributeDefinedStatically
@@ -89,6 +99,10 @@ create_list :user, 3
 |===
 | Name | Default value | Configurable values
 
+| Include
+| `+**/*_spec.rb+`, `+**/spec/**/*+`, `spec/factories.rb`, `spec/factories/**/*.rb`, `features/support/factories/**/*.rb`
+| Array
+
 | EnforcedStyle
 | `create_list`
 | `create_list`, `n_times`
@@ -130,6 +144,16 @@ factory :foo, class: 'Foo' do
 end
 ----
 
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| Include
+| `spec/factories.rb`, `spec/factories/**/*.rb`, `features/support/factories/**/*.rb`
+| Array
+|===
+
 === References
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/FactoryClassName
diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc
@@ -36,23 +36,20 @@ end
 
 == Inspecting files that don't end with `_spec.rb`
 
-By default, `rubocop-rspec` only inspects code within paths ending in `_spec.rb` or including `spec/`. You can override this setting in your config file by specifying one or more patterns:
+By default, `rubocop-rspec` only inspects code within paths ending in `_spec.rb` or including `spec/`. You can override this setting in your config file by setting `Include`:
 
 [source,yaml]
 ----
-# Inspect all files
-AllCops:
-  RSpec:
-    Patterns:
-    - '.+'
+# Inspect files in `test/` directory
+RSpec:
+  Include:
+    - '**/test/**/*'
 ----
 
-
 [source,yaml]
 ----
 # Inspect only files ending with `_test.rb`
-AllCops:
-  RSpec:
-    Patterns:
-    - '_test.rb$'
+RSpec:
+  Include:
+    - '**/*_test.rb'
 ----
diff --git a/lib/rubocop/cop/rspec/base.rb b/lib/rubocop/cop/rspec/base.rb
@@ -4,72 +4,16 @@ module RuboCop
   module Cop
     module RSpec
       # @abstract parent class to RSpec cops
-      #
-      # The criteria for whether rubocop-rspec analyzes a certain ruby file
-      # is configured via `AllCops/RSpec`. For example, if you want to
-      # customize your project to scan all files within a `test/` directory
-      # then you could add this to your configuration:
-      #
-      # @example configuring analyzed paths
-      #   # .rubocop.yml
-      #   # AllCops:
-      #   #   RSpec:
-      #   #     Patterns:
-      #   #     - '_test.rb$'
-      #   #     - '(?:^|/)test/'
       class Base < ::RuboCop::Cop::Base
         include RuboCop::RSpec::Language
         include RuboCop::RSpec::Language::NodePattern
 
-        DEFAULT_CONFIGURATION =
-          RuboCop::RSpec::CONFIG.fetch('AllCops').fetch('RSpec')
-
-        DEFAULT_PATTERN_RE = Regexp.union(
-          DEFAULT_CONFIGURATION.fetch('Patterns')
-                               .map(&Regexp.public_method(:new))
-        )
-
         exclude_from_registry
 
         # Invoke the original inherited hook so our cops are recognized
         def self.inherited(subclass) # rubocop:disable Lint/MissingSuper
           RuboCop::Cop::Base.inherited(subclass)
         end
-
-        def relevant_file?(file)
-          relevant_rubocop_rspec_file?(file) && super
-        end
-
-        private
-
-        def relevant_rubocop_rspec_file?(file)
-          rspec_pattern.match?(file)
-        end
-
-        def rspec_pattern
-          if rspec_pattern_config?
-            Regexp.union(rspec_pattern_config.map(&Regexp.public_method(:new)))
-          else
-            DEFAULT_PATTERN_RE
-          end
-        end
-
-        def all_cops_config
-          config
-            .for_all_cops
-        end
-
-        def rspec_pattern_config?
-          return unless all_cops_config.key?('RSpec')
-
-          all_cops_config.fetch('RSpec').key?('Patterns')
-        end
-
-        def rspec_pattern_config
-          all_cops_config
-            .fetch('RSpec', DEFAULT_CONFIGURATION)
-            .fetch('Patterns')
-        end
       end
     end
   end

diff --git a/lib/rubocop/rspec/config_formatter.rb b/lib/rubocop/rspec/config_formatter.rb
@@ -6,7 +6,7 @@ module RuboCop
   module RSpec
     # Builds a YAML config file from two config hashes
     class ConfigFormatter
-      NAMESPACES = /^(RSpec|Capybara|FactoryBot|Rails)/.freeze
+      EXTENSION_ROOT_DEPARTMENT = %r{^(RSpec/)}.freeze
       STYLE_GUIDE_BASE_URL = 'https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/'
 
       def initialize(config, descriptions)
@@ -15,7 +15,7 @@ def initialize(config, descriptions)
       end
 
       def dump
-        YAML.dump(unified_config).gsub(NAMESPACES, "\n\\1")
+        YAML.dump(unified_config).gsub(EXTENSION_ROOT_DEPARTMENT, "\n\\1")
       end
 
       private
@@ -29,7 +29,7 @@ def unified_config
       end
 
       def cops
-        (descriptions.keys | config.keys).grep(NAMESPACES)
+        (descriptions.keys | config.keys).grep(EXTENSION_ROOT_DEPARTMENT)
       end
 
       attr_reader :config, :descriptions

diff --git a/spec/project/default_config_spec.rb b/spec/project/default_config_spec.rb
@@ -29,7 +29,7 @@
   end
 
   let(:config_keys) do
-    cop_names + %w[AllCops]
+    cop_names + %w[RSpec]
   end
 
   def cop_configuration(config_key)

diff --git a/spec/rubocop/cop/rspec/base_spec.rb b/spec/rubocop/cop/rspec/base_spec.rb
@@ -1,25 +1,8 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::Cop::RSpec::Base do
-  subject(:cop) { RuboCop::RSpec::FakeCop.new(config) }
-
-  let(:config) do
-    rubocop_config =
-      {
-        'AllCops' => {
-          'RSpec' => {
-            'Patterns' => rspec_patterns
-          }
-        },
-        'RSpec/FakeCop' => {
-          'Exclude' => %w[bar_spec.rb]
-        }
-      }
-
-    RuboCop::Config.new(rubocop_config, 'fake_cop_config.yml')
-  end
-
-  let(:rspec_patterns) { ['_spec.rb$', '(?:^|/)spec/'] }
+  let(:cop_class) { RuboCop::RSpec::FakeCop }
+  let(:cop_config) { { 'Exclude' => %w[bar_spec.rb] } }
 
   before do
     stub_const('RuboCop::RSpec::FakeCop',
@@ -78,8 +61,12 @@ def on_send(node)
   end
 
   context 'when custom patterns are specified' do
-    let(:rspec_patterns) do
-      ['_test\.rb$']
+    let(:other_cops) do
+      {
+        'RSpec' => {
+          'Include' => ['*_test\.rb']
+        }
+      }
     end
 
     it 'registers offenses when the path matches a custom specified pattern' do

diff --git a/spec/rubocop/cop/rspec/factory_bot/attribute_defined_statically_spec.rb b/spec/rubocop/cop/rspec/factory_bot/attribute_defined_statically_spec.rb
@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AttributeDefinedStatically do
+  def inspected_source_filename
+    'spec/factories.rb'
+  end
+
   it 'registers an offense for offending code' do
     expect_offense(<<-RUBY)
       FactoryBot.define do

diff --git a/spec/rubocop/cop/rspec/factory_bot/factory_class_name_spec.rb b/spec/rubocop/cop/rspec/factory_bot/factory_class_name_spec.rb
@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::Cop::RSpec::FactoryBot::FactoryClassName do
+  def inspected_source_filename
+    'spec/factories.rb'
+  end
+
   context 'when passing block' do
     it 'flags passing a class' do
       expect_offense(<<~RUBY)

diff --git a/spec/rubocop/cop/rspec/file_path_spec.rb b/spec/rubocop/cop/rspec/file_path_spec.rb
@@ -58,14 +58,14 @@
   end
 
   it 'registers an offense for a file missing _spec' do
-    expect_offense(<<-RUBY, 'user.rb')
+    expect_offense(<<-RUBY, 'spec/models/user.rb')
       describe User do; end
       ^^^^^^^^^^^^^ Spec path should end with `user*_spec.rb`.
     RUBY
   end
 
   it 'ignores shared examples' do
-    expect_no_offenses(<<-RUBY, 'user.rb')
+    expect_no_offenses(<<-RUBY, 'spec/models/user.rb')
       shared_examples_for 'foo' do; end
     RUBY
   end
@@ -254,7 +254,7 @@ class Foo
     end
 
     it 'registers an offense when _spec.rb suffix is missing' do
-      expect_offense(<<-RUBY, 'whatever.rb')
+      expect_offense(<<-RUBY, 'spec/whatever.rb')
         describe MyClass do; end
         ^^^^^^^^^^^^^^^^ Spec path should end with `*_spec.rb`.
       RUBY

diff --git a/spec/support/config.rb b/spec/support/config.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+RSpec.shared_context 'with RuboCop-RSpec config', :config do
+  # This overrides the config defined in the default shared context since
+  # RuboCop ignores department-level cop configuration in specs.
+  let(:config) do
+    department_name = cop_class.badge.department.to_s
+    # By default, `RSpec/Include: ['**/*_spec.rb', '**/spec/**/*']`
+    department_configuration = RuboCop::ConfigLoader
+      .default_configuration
+      .for_department(department_name)
+
+    hash = { 'AllCops' => all_cops_config,
+             cop_class.cop_name => cur_cop_config,
+             department_name => department_configuration }
+      .merge!(other_cops)
+
+    RuboCop::Config.new(hash, "#{Dir.pwd}/.rubocop.yml")
+  end
+end