Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove relevance detection code #1063

Merged
merged 1 commit into from Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
31 changes: 21 additions & 10 deletions config/default.yml
@@ -1,14 +1,8 @@
---
AllCops:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I've dropped AllCops is that it's never mentioned in the docs it's a preferred way to configure cops in departments. It works quite fine on the root level. https://docs.rubocop.org/rubocop/1.0/configuration.html
Also, I think there was some complication where for_badge didn't work properly with configuration options defined inside AllCops.

RSpec:
Patterns:
- _spec.rb
- "(?:^|/)spec/"
RSpec/FactoryBot:
Patterns:
- spec/factories.rb
- spec/factories/**/*.rb
- features/support/factories/**/*.rb
RSpec:
Include:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior of Include (overriding default.yml) was introduced in 0.56.0 via rubocop/rubocop#5882. This change allows people to include/exclude precisely what they need to, without the defaults getting in the way.

- "**/*_spec.rb"
- "**/spec/**/*"

RSpec/AlignLeftLetBrace:
Description: Checks that left braces for adjacent single line lets are aligned.
Expand Down Expand Up @@ -241,6 +235,9 @@ RSpec/ExpectOutput:
RSpec/FilePath:
Description: Checks that spec file paths are consistent and well-formed.
Enabled: true
Include:
- "**/*_spec*rb*"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is due to file name detection, e.g. some_spec.rb.rb and some_specorb.

- "**/spec/**/*"
CustomTransform:
RuboCop: rubocop
RSpec: rspec
Expand Down Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cop should only inspect factory definitions.

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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_list on the contrary, should be checked in both specs and factories.

EnforcedStyle: create_list
SupportedStyles:
- create_list
Expand All @@ -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
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/**/*'
bquorning marked this conversation as resolved.
Show resolved Hide resolved
----


[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))
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this to dramatically simplify #956


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
6 changes: 3 additions & 3 deletions lib/rubocop/rspec/config_formatter.rb
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
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
29 changes: 8 additions & 21 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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSpec/Include is taken from config/default.yml.

}
},
'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',
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']
Copy link
Member Author

@pirj pirj Oct 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overrides the setting of RSpec/Include from config/default.yml.

}
}
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our overloaded expect_offense/expect_no_offenses use this to pass the file name to super.

This tells the cop that it inspects a relevant source.

'spec/factories.rb'
end

it 'registers an offense for offending code' do
pirj marked this conversation as resolved.
Show resolved Hide resolved
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
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)
Comment on lines +7 to +11
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope to submit this upstream.


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