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 28, 2020
1 parent abe73f0 commit 676b2f4
Show file tree
Hide file tree
Showing 15 changed files with 113 additions and 106 deletions.
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
@@ -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
Expand Down
4 changes: 4 additions & 0 deletions 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)
Expand Down
6 changes: 3 additions & 3 deletions spec/rubocop/cop/rspec/file_path_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/rspec/instance_variable_spec.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::InstanceVariable do
RSpec.describe RuboCop::Cop::RSpec::InstanceVariable, :config do
it 'flags an instance variable inside a describe' do
expect_offense(<<-RUBY)
describe MyClass do
Expand Down
20 changes: 20 additions & 0 deletions 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
12 changes: 10 additions & 2 deletions spec/support/expect_offense.rb
Expand Up @@ -4,16 +4,24 @@
#
# This mixin is the same as rubocop's ExpectOffense except the default
# filename ends with `_spec.rb`
#
# Cops assigned to departments may focus on different files, so it is
# possible to override the inspected file name.
module ExpectOffense
include RuboCop::RSpec::ExpectOffense

DEFAULT_FILENAME = 'example_spec.rb'

def expect_offense(source, filename = DEFAULT_FILENAME, *args, **kwargs) # rubocop:disable Lint/UselessMethodDefinition
def expect_offense(source, filename = inspected_source_filename,
*args, **kwargs)
super
end

def expect_no_offenses(source, filename = DEFAULT_FILENAME) # rubocop:disable Lint/UselessMethodDefinition
def expect_no_offenses(source, filename = inspected_source_filename)
super
end

def inspected_source_filename
DEFAULT_FILENAME
end
end

0 comments on commit 676b2f4

Please sign in to comment.