rubocop-rspec
rubocop-rspec copied to clipboard
Add `require_implicit` style to `RSpec/ImplicitSubject`
Although the explicit style is recommended by default, some users may prefer the implicit style. For such users, it would be better if there is an option to make the style consistent with the explicit style, rather than asking them to disable this cop.
Before submitting the PR make sure the following are checked:
- [x] Feature branch is up-to-date with
master
(if not - rebase it). - [x] Squashed related commits together.
- [x] Added tests.
- [x] Updated documentation.
- [x] Added an entry to the
CHANGELOG.md
if the new code introduces user-observable changes. - [x] The build (
bundle exec rake
) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).
If you have created a new cop:
- [ ] Added the new cop to
config/default.yml
. - [ ] The cop is configured as
Enabled: pending
inconfig/default.yml
. - [ ] The cop is configured as
Enabled: true
in.rubocop.yml
. - [ ] The cop documents examples of good and bad code.
- [ ] The tests assert both that bad code is reported and that good code is not reported.
- [ ] Set
VersionAdded
indefault/config.yml
to the next minor version.
If you have modified an existing cop's configuration options:
- [x] Set
VersionChanged
inconfig/default.yml
to the next major version.
I think the name of the style "always" is debatable. Since this cop is called ImplicitSubject, I named it "always" as in "always use implicit subject." If you have any other good ideas, please let me know.
I added a multi-line example and then an example about named subject.
I figured that if named subject is used, it would be better to ignore it. Or should we also flag named subject as an offense and prompt to convert to unnamed subject?
This is my opinion, but I personally think it would be better to keep this cop only to decide whether is_expected
should be used or expect(subject)
should be used. Rather than having this cop decide what the named subject should be, it would be better to have a separate cop that decides whether the named subject should be recommended or not, and have it decide there.
I tend to agree that if there is named subject in use, it's better to be left explicit, as you can't use the name with the implicit syntax, and there might be a reason you have chosen a name.
if there is named subject in use, it's better to be left explicit, as you can't use the name with the implicit syntax, and there might be a reason you have chosen a name.
Furthermore, there can be more than one:
subject(:foo) { ... }
context '' do
subject(:bar) { ... }
it do
expect(foo).to ...
end
and we'll end up with a task to properly detect if the used explicit subject is also the right implicit one.
I have renamed some of the required CI checks, which unfortunately means that you will need to rebase this branch to make CI pass.