Add new RSpec/UndescriptiveLiteralsDescription cop

[ydah]

Fix: #1754

---

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

If you have created a new cop:

• Added the new cop to config/default.yml.
• The cop is configured as Enabled: pending in config/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: "<<next>>" in default/config.yml.

[bquorning]

Is it ok to have a cop that singles out the xstr type. You can pass an Integer or a Regexp to it and it’ll still run – should we allow that? What if e.g. we only allowed str and dstr nodes instead?

[ydah]

we only allowed str and dstr nodes instead?

For example, description specifies the class to be tested, so I don't think it is just string and dstr. Regex and Integer have added support and renamed Cop.

[ydah]

I changed this cop name from UnclearDescription to UndescriptiveLiteralsDescription.

[pirj]

The code looks good. Thank you!

i’d love to run this on real-world-rspec but lack capacity to do this myself

[pirj]

You’ve added a test for foo + time, but only when all? is false.

If your experience suggests that we need to dive as deep as handling +, I think that if any of parts is an xstr, the whole thing is non-deterministic.
Is sometimh like “‘time’ + ‘whoami’” real? Or just as mistyped quotes like “‘when it is’ + time”?

Again, no objections on merging.

[ydah]

I re-think it might be better to always allow for concatenation by +. Because it seems difficult to understand the offense in case of offenses, and it seems to be outside the scope of this cop.

  it 'registers an offense when using `describe` with `time`+"foo"' do
    expect_offense(<<~RUBY)
      describe `time` + 'foo' do
               ^^^^^^^^^^^^^^ Description should be descriptive.
      end
    RUBY
  end

  it 'registers an offense when using `describe` with "foo"+`time`' do
    expect_offense(<<~RUBY)
      describe 'foo' + `time` do
               ^^^^^^^^^^^^^^ Description should be descriptive.
      end
    RUBY
  end

  it 'registers an offense when using `describe` with "foo+(`time`+"bar")' do
    expect_offense(<<~RUBY)
      describe 'foo' + (`time` + 'bar') do
               ^^^^^^^^^^^^^^^^^^^^^^^^ Description should be descriptive.
      end
    RUBY
  end

[ydah]

Looks fine:

Offenses:

/ydah/real-world-rspec/gitlabhq/spec/services/merge_requests/base_service_spec.rb:83:12: C: RSpec/UndescriptiveLiteralsDescription: Description should be descriptive.
  describe `#create_pipeline_for` do
           ^^^^^^^^^^^^^^^^^^^^^^

22772 files inspected, 1 offense detected

[ydah]

@pirj Do you have any concerns?

[pirj]

@ydah The code looks good.
As usual, I’d prefer to run new cops against real-world-rspec, as a preventive measure to user-reported errors. But if you lack capacity to do so, too, I’m fine with releasing this.
My main concern with user-reported errors is that the first thing they suggest is to mark cops as unsafe. And we end up with a bunch of effectively retired cops.

[ydah]

@pirj I'm sorry. It seems that I didn't explain it enough, but the following comment shows that after doing it with real-wolrld-rspec, I was able to see the offense I expected to detect without generating any false positives or errors 🙇🏻

• Add new RSpec/UndescriptiveLiteralsDescription cop #1796 (comment)

[pirj]

@ydah my apologies, this slipped my attention.
Thanks a lot for the cop, it looks great!