Skip to content

Conversation

@kevinrobell-st
Copy link
Contributor

@kevinrobell-st kevinrobell-st commented Nov 6, 2025

This is based on the Rails/Output cop with three minor changes.

  1. Autocorrection is removed as the expectation is that the print statement will be removed by the user.
  2. The message is changed.
  3. The cop runs only on spec files.

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.

If you have modified an existing cop's configuration options:

  • Set VersionChanged: "<<next>>" in config/default.yml.

@kevinrobell-st kevinrobell-st requested a review from a team as a code owner November 6, 2025 00:26
@kevinrobell-st
Copy link
Contributor Author

RSpec/Output was chosen as the cop name to align with Rails/Output but potentially it'd be better to have more descriptive name here.

@kevinrobell-st
Copy link
Contributor Author

I did some grepping around the real-world-rspec repo as well and I think this cop should be fine to add. There are some legitimate uses of printing in some integration tests, some examples of print statements that look like they were leftover from debugging and print statements in setup, helper and support logic. By making the cop enabled: pending for now we don't have to confront those changes immediately but there might come a time in the future were we decide to exclude more subdirectories or files from this cop or even mark it as enabled: false if there are too many false positives. I still think that it has value as a cop though because generally printing in specs is not expected.

@kevinrobell-st
Copy link
Contributor Author

This is succeeding for me locally with no file changes. I wonder why.

@ydah
Copy link
Member

ydah commented Nov 24, 2025

Could you please squashed related commits into one?

@r7kamura
Copy link
Contributor

r7kamura commented Dec 4, 2025

I agree with adding this cop with Enabled: pending, as there should be sufficiently beneficial use cases.

Also, I believe the cop name RSpec/Output is appropriate. It should be easily understood by users, especially since there is already a well-known cop named Rails/Output.

As I commented inline in the code, I believe that contextual autocorrect is preferable, but this is a point where opinions may differ. What does everyone else think?

@kevinrobell-st
Copy link
Contributor Author

In general, I like the idea of adding autocorrect and I didn't realize how simple it would be to add. It might be better to make it an unsafe autocorrect though since there are some legitimate but rare use cases for printing in integration specs.

@kevinrobell-st kevinrobell-st force-pushed the add-rspec/output-cop branch 2 times, most recently from d2bc1f0 to 7ad5ba6 Compare December 4, 2025 22:30
@kevinrobell-st
Copy link
Contributor Author

I've updated the code to address comments and add unsafe autocorrect. The autocorrect specs look a bit weird because they're just blank lines but it does show that things are working correctly. Let me know if there's anything else I should address.

This is based on the `Rails/Output` cop with three minor changes.

1. Autocorrection is removed as the expectation is that the print
   statement will be removed by the user.
2. The message is changed.
3. The cop runs only on spec files.

Clean up rubocop:disable

Co-authored-by: Yudai Takada <[email protected]>

Update comment

Apply suggestions from code review

Make the code more maintainable and add autocorrect.

Co-authored-by: Ryo Nakamura <[email protected]>

Update specs

Add AutoCorrect: contextual

Co-authored-by: Ryo Nakamura <[email protected]>

Add @safety comment
@kevinrobell-st
Copy link
Contributor Author

Okay, I added a @safety comment and marked the new cop as AutoCorrect: contextual. For some reason, I thought that happened automatically for unsafe cops so that's why I didn't add it before.

Copy link
Contributor

@r7kamura r7kamura left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@bquorning bquorning 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 for the contribution!

@bquorning bquorning merged commit 6a5bf09 into rubocop:master Dec 8, 2025
27 checks passed
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.

New cop to disallow printing from tests

4 participants