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

Fix inherit_mode for third-party gems #1192

Merged
merged 1 commit into from Oct 5, 2021

Conversation

pirj
Copy link
Member

@pirj pirj commented Oct 3, 2021

fixes #1126

See palkan/action_policy#103 (comment)

If a third-party gem, e.g. action_policy, defines their config/default.yml and supplements to the default RSpec DSL that we provide in our config/default.yml, rubocop's behaviour is to actually override the configuration, as inherit_mode is override for lists by default.

RuboCop has recently fixed the problem with ignored inherit_mode and we've bumped to use rubocop version that includes the fix, but we haven't adjusted our config/default.yml to merge by default.

This is causing both user project RSpec DSL configuration and third-party gem RSpec DSL configuration to override our default, rendering our cops blind.

A little more context and reasoning for this fix palkan/action_policy#103 (comment)

Example

A new project

# Gemfile
source 'https://rubygems.org'

gem 'action_policy'
gem 'rubocop-rspec'
# .rubocop.yml
require:
  - rubocop-rspec

inherit_gem:
  action_policy: config/rubocop-rspec.yml
# spec/a_spec.rb
RSpec.describe 'A' do
  it_behaves_like 'a'
  it_behaves_like 'a'

  describe_rule :show? do
    succeed 'when post is published'
    succeed 'when post is published'
  end
end

Ideally, both the duplicated it_behaves_like and succeed should be detected. However, action_policy's Includes/Examples setting overrides ours, and it_behaves_like disappears from this list. As a result, rubocop only detects the duplication of succeed, but not of it_behaves_like.

References

RSpec DSL configuration ticket #333
RSpec DSL configuration PR #956
Problem with merging #1126
Fix for ignored inherit_mode rubocop/rubocop#9952
#1181
https://docs.rubocop.org/rubocop/1.0/configuration.html#merging-arrays-using-inherit_mode
https://docs.rubocop.org/rubocop/1.9/configuration.html#unusual-files-that-would-not-be-included-by-default
Documentation for third-party gems to work with rubocop-rspec https://docs.rubocop.org/rubocop-rspec/third_party_rspec_syntax_extensions.html
#1077
Example third-party gem RSpec DSL configuration palkan/action_policy#138


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).

fixes #1126

See palkan/action_policy#103 (comment)

If a third-party gem, e.g. `action_policy`, defines their
`config/default.yml` and supplements to the default RSpec DSL that we
provide in our `config/default.yml`, `rubocop`'s behaviour is to
actually override the configuration, as `inherit_mode` is `override` for
lists by default.

RuboCop has recently [fixed the problem with ignored
`inherit_mode`](rubocop/rubocop#9952) and we've
[bumped to use `rubocop` version that includes the
fix](#1181), but we haven't
adjusted our `config/default.yml` to merge by default.

This is causing both user project RSpec DSL configuration and
third-party gem RSpec DSL configuration to override our default,
rendering our cops blind.

### Example

A new project

```ruby
# Gemfile
source 'https://rubygems.org'

gem 'action_policy'
gem 'rubocop-rspec'
```

```yml
# .rubocop.yml
require:
  - rubocop-rspec

inherit_gem:
  action_policy: config/rubocop-rspec.yml
```

```ruby
# spec/a_spec.rb
RSpec.describe 'A' do
  it_behaves_like 'a'
  it_behaves_like 'a'

  describe_rule :show? do
    succeed 'when post is published'
    succeed 'when post is published'
  end
end
```

Ideally, both the duplicated `it_behaves_like` and `succeed` should be
detected. However, `action_policy`'s `Includes/Examples` setting
overrides ours, and `it_behaves_like` disappears from this list. As a
result, `rubocop` only detects the duplication of `succeed`, but not of
`it_behaves_like`.
@pirj pirj force-pushed the fix-rspec-dsl-for-third-party-gems branch from 17e4728 to f663fdc Compare October 5, 2021 08:01
Copy link
Member

@Darhazer Darhazer left a comment

Choose a reason for hiding this comment

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

Thank you 🙇 Perhaps release 2.5.1 with the fix?

@pirj pirj merged commit 5f21458 into master Oct 5, 2021
@pirj pirj deleted the fix-rspec-dsl-for-third-party-gems branch October 5, 2021 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RSpec/Language isn't merging properly Make RSpec describe_rule DRY
2 participants