Skip to content

Commit

Permalink
Remove relevance detection code
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pirj committed Oct 27, 2020
1 parent 5e23ee2 commit 561d04e
Show file tree
Hide file tree
Showing 16 changed files with 194 additions and 162 deletions.
4 changes: 4 additions & 0 deletions .rubocop_todo.yml
Expand Up @@ -10,3 +10,7 @@
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 106

RSpec/ExampleLength:
Exclude:
- spec/rubocop/cop/rspec/factory_bot/attribute_defined_statically_spec.rb
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down
32 changes: 22 additions & 10 deletions config/default.yml
@@ -1,14 +1,9 @@
---
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.
Expand Down Expand Up @@ -241,6 +236,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
Expand Down Expand Up @@ -639,13 +637,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
Expand All @@ -657,6 +665,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
Expand Down
4 changes: 4 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Expand Up @@ -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"}`
|
Expand Down
24 changes: 24 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec/factorybot.adoc
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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
19 changes: 8 additions & 11 deletions docs/modules/ROOT/pages/usage.adoc
Expand Up @@ -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'
----
56 changes: 0 additions & 56 deletions lib/rubocop/cop/rspec/base.rb
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/rspec/config_formatter.rb
Expand Up @@ -22,6 +22,8 @@ def dump

def unified_config
cops.each_with_object(config.dup) do |cop, unified|
next if cop == 'RSpec' # Skip department-level configuration

unified[cop] = config.fetch(cop)
.merge(descriptions.fetch(cop))
.merge('StyleGuide' => STYLE_GUIDE_BASE_URL + cop.sub('RSpec/', ''))
Expand Down
2 changes: 1 addition & 1 deletion spec/project/default_config_spec.rb
Expand Up @@ -29,7 +29,7 @@
end

let(:config_keys) do
cop_names + %w[AllCops]
cop_names + %w[RSpec]
end

def cop_configuration(config_key)
Expand Down
31 changes: 9 additions & 22 deletions 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/'] }
RSpec.describe RuboCop::Cop::RSpec::Base, :config do
let(:cop_class) { RuboCop::RSpec::FakeCop }
let(:cop_config) { { 'Exclude' => %w[bar_spec.rb] } }

before do
stub_const('RuboCop::RSpec::FakeCop',
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 561d04e

Please sign in to comment.