Rubocop-rspec: Name `subject` when referenced explicitly

[backus]

When referencing subject explicitly is necessary it should be given a name

good

RSpec.describe UserCreator do
  subject(:create) { described_class.new(path) }

  it 'creates a user' do
    expect { create }.to change(User, :count).by(1)
  end
end

bad

RSpec.describe UserCreator do
  subject { described_class.new(path) }

  it 'creates a user' do
    expect { subject }.to change(User, :count).by(1)
  end
end

thoughts? Happy to do a PR

[cgriego]

:+1: Was surprised this wasn't already included.

[wilsonsilva]

Yes, please! :metal:

[backus]

It doesn't look like there is any opposition to this, but just as an extra point of reference here is an article by @dchelimsky making the same point about explicit subject being a smell: http://blog.davidchelimsky.net/blog/2012/05/13/spec-smell-explicit-use-of-subject/

[andyw8]

This should be easy to implement. Any suggestions for what to call the cop? AnonymousSubject?

[backus]

So I forgot I've actually implemented this in my own rubocop extensions library: https://github.com/backus/rubocop-devtools/blob/master/lib/rubocop/cop/devtools/named_subject.rb. I named it NamedSubject. I should do a PR this week

[andyw8]

Cool. I like some of the other cops you have there:

• https://github.com/backus/rubocop-devtools/blob/master/lib/rubocop/cop/devtools/specify_it.rb
• https://github.com/backus/rubocop-devtools/blob/master/lib/rubocop/cop/devtools/group_single_line_let.rb

[backus]

Thanks! I want to revisit that project when I have more time. I should also extract them for rubocop-rspec if the maintainers of this project are interested

[backus]

@cgriego @wilsonsilva @danhawkins @andyw8 this is released in 1.5.3. Check it out!

[wilsonsilva]

@backus awesome! :metal:

[cdmo]

I know this issue is closed and done, but, can someone explain why this is a rule? Is it due to ambiguity? Why explicitly define a subject if you're aren't going to reference it? Thanks

[pirj]

@cdmo Below are some links and quotes to shed some light on this topic.

http://lelylan.github.io/betterspecs/#subject (aka http://www.betterspecs.org/#subject), https://github.com/rubocop-hq/rspec-style-guide/issues/29, and https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NamedSubject, most importantly last one:

Your test subjects should be the most important object in your tests so they deserve a descriptive name.

http://rspec.info/documentation/3.9/rspec-core/RSpec/Core/MemoizedHelpers.html#subject-instance_method:

Note: subject was contributed by Joe Ferris to support the one-liner syntax embraced by shoulda matchers:

RSpec.describe Widget do
  it { is_expected.to validate_presence_of(:name) }

hinting that subject { ... } was never supposed to be used directly.

Also https://relishapp.com/rspec/rspec-core/v/3-9/docs/subject/explicit-subject:

Note that while the examples below demonstrate how the subject helper can be used
as a user-facing concept, we recommend that you reserve it for support of custom
matchers and/or extension libraries that hide its use from examples.

and:

A named subject improves on the explicit subject by assigning it a contextually
semantic name. Since a named subject is an explicit subject, it still defines the value
that is returned by the subject method in the example scope.

[cdmo]

Thank you @pirj for the quick and thorough reply. I think I still struggle a bit with this one, but the rule does prevent ambiguity, making it clearer in each individual test what is being tested. Also important is that it's a ruby convention and standard of the community, which makes it an expectation of ruby programmers. Thanks again.

[pirj]

You are always welcome!