Add new RSpec/EmptyOutput cop

[bquorning]

Instead of calling output with an empty string, you should use the inverse runner and call output without an argument. E.g. instead of

expect { foo }.to output('').to_stdout

you should call

expect { foo }.not_to output.to_stdout

---

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]

cc @pirj

[pirj]

One exception I can think of is a compund expectation like output(‘’).to_stdout.and output(‘’).to_stderr.
We had a cop that suggested using a custom negation matcher for compound expectations. Was that not_change?

Otherwise - looks good, thanks!

[bquorning]

I actually did look into the compound marchers, and I thinkto_stderr and to_stdout both return true so they cannot be chained. I may be wrong of course.

[pirj]

At a glance, it returns self, and the vase class does include Composable that defines and/or.

Not a big deal, just an edge case. But since to_std… is mandatory, it’s not unrealistic.

[bquorning]

I’ll have a look.

Actually what should we recommend if someone matches e.g.

expect {}.to output("foo").to_stdout.and output("").to_stderr

I don’t think there’s a way to do e.g.

expect {}.to output("foo").to_stdout.and not output.to_stderr
# or
expect {}.to_not output.to_stderr.and output("foo").to_stdout

The last code tells me that

expect(...).not_to matcher.and matcher is not supported, since it creates a bit of an ambiguity. Instead, define negated versions of whatever matchers you wish to negate with RSpec::Matchers.define_negated_matcher and use expect(...).to matcher.and matcher.

Maybe in that case it’s fine that we don’t detect an offense on compound expectations.

[pirj]

Looks good, thank you!

[pirj]

For the other cop, we register an offence, but don’t autocorrect, since it’s not guaranteed that the negated matcher (not_change) is defined in the user’s codebase.

expect { :nop }
  .to not_output.to_stdout
  .and not_output.to_stderr

should work, just like a combination of output.to_stderr with eg not_output.to_stdout

[bquorning]

Yeah, I think it’s too much if we ask the user to implement a #not_output method when output("") works fine.

[bquorning]

Just to be sure, the all matcher cannot be used together with output, right? E.g. expect([proc_1, proc_2]).to all(output("").to_stderr)

[ydah]

I checked with real-world-rspec. There seems to be no errors or false positives ✅

Offenses:

/ydah/real-world-rspec/rubocop/spec/rubocop/config_loader_spec.rb:1016:74: C: [Correctable] RSpec/EmptyOutput: Use not_to instead of matching on an empty output.
        expect { expect(configuration_from_file.to_h).to eq(config) }.to output('').to_stderr
                                                                         ^^^^^^^^^^
/ydah/real-world-rspec/rubocop/spec/rubocop/config_loader_spec.rb:1213:18: C: [Correctable] RSpec/EmptyOutput: Use not_to instead of matching on an empty output.
          end.to output('').to_stderr
                 ^^^^^^^^^^
/ydah/real-world-rspec/rubocop/spec/rubocop/config_loader_spec.rb:1249:18: C: [Correctable] RSpec/EmptyOutput: Use not_to instead of matching on an empty output.
          end.to output('').to_stderr
                 ^^^^^^^^^^
/ydah/real-world-rspec/rubocop/spec/rubocop/config_loader_spec.rb:1317:47: C: [Correctable] RSpec/EmptyOutput: Use not_to instead of matching on an empty output.
        expect { configuration_from_file }.to output('').to_stderr
                                              ^^^^^^^^^^

22407 files inspected, 4 offenses detected, 4 offenses autocorrectable

[ydah]

Looks good!