From 10c8ec081e731439955c60f4dffe00e67003c155 Mon Sep 17 00:00:00 2001 From: Kevin Robell Date: Wed, 5 Nov 2025 16:02:11 -0800 Subject: [PATCH] Add a new cop `RSpec/Output` 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 Update comment Apply suggestions from code review Make the code more maintainable and add autocorrect. Co-authored-by: Ryo Nakamura Update specs Add AutoCorrect: contextual Co-authored-by: Ryo Nakamura Add @safety comment --- .rubocop.yml | 1 + CHANGELOG.md | 2 + config/default.yml | 8 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rspec.adoc | 38 ++++ lib/rubocop/cop/rspec/output.rb | 78 +++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + spec/rubocop/cop/rspec/output_spec.rb | 221 ++++++++++++++++++++++++ 8 files changed, 350 insertions(+) create mode 100644 lib/rubocop/cop/rspec/output.rb create mode 100644 spec/rubocop/cop/rspec/output_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 4362ff878..59c5d81f4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -294,3 +294,4 @@ Performance/ZipWithoutBlock: {Enabled: true} RSpec/IncludeExamples: {Enabled: true} RSpec/LeakyLocalVariable: {Enabled: true} +RSpec/Output: {Enabled: true} diff --git a/CHANGELOG.md b/CHANGELOG.md index 132bdf93b..8b5e1c09a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Fix detection of nameless doubles with methods in `RSpec/VerifiedDoubles`. ([@ushi-as]) - Improve an offense message for `RSpec/RepeatedExample` cop. ([@ydah]) - Let `RSpec/SpecFilePathFormat` leverage ActiveSupport inflections when configured. ([@corsonknowles], [@bquorning]) +- Add new cop `RSpec/Output`. ([@kevinrobell-st]) ## 3.7.0 (2025-09-01) @@ -1025,6 +1026,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@jtannas]: https://github.com/jtannas [@k-s-a]: https://github.com/K-S-A [@kellysutton]: https://github.com/kellysutton +[@kevinrobell-st]: https://github.com/kevinrobell-st [@koic]: https://github.com/koic [@krororo]: https://github.com/krororo [@kuahyeow]: https://github.com/kuahyeow diff --git a/config/default.yml b/config/default.yml index 537d47768..4fd230d9b 100644 --- a/config/default.yml +++ b/config/default.yml @@ -758,6 +758,14 @@ RSpec/NotToNot: VersionAdded: '1.4' Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NotToNot +RSpec/Output: + Description: Checks for the use of output calls like puts and print in specs. + Enabled: pending + AutoCorrect: contextual + SafeAutoCorrect: false + VersionAdded: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Output + RSpec/OverwritingSetup: Description: Checks if there is a let/subject that overwrites an existing one. Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index fb43db294..9c1e7746f 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -77,6 +77,7 @@ * xref:cops_rspec.adoc#rspecnestedgroups[RSpec/NestedGroups] * xref:cops_rspec.adoc#rspecnoexpectationexample[RSpec/NoExpectationExample] * xref:cops_rspec.adoc#rspecnottonot[RSpec/NotToNot] +* xref:cops_rspec.adoc#rspecoutput[RSpec/Output] * xref:cops_rspec.adoc#rspecoverwritingsetup[RSpec/OverwritingSetup] * xref:cops_rspec.adoc#rspecpending[RSpec/Pending] * xref:cops_rspec.adoc#rspecpendingwithoutreason[RSpec/PendingWithoutReason] diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index c878e60d0..c20743397 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -4725,6 +4725,44 @@ end * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NotToNot +[#rspecoutput] +== RSpec/Output + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Command-line only (Unsafe) +| <> +| - +|=== + +Checks for the use of output calls like puts and print in specs. + +[#safety-rspecoutput] +=== Safety + +This autocorrection is marked as unsafe because, in rare cases, print +statements can be used on purpose for integration testing and deleting +them will cause tests to fail. + +[#examples-rspecoutput] +=== Examples + +[source,ruby] +---- +# bad +puts 'A debug message' +pp 'A debug message' +print 'A debug message' +---- + +[#references-rspecoutput] +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Output + [#rspecoverwritingsetup] == RSpec/OverwritingSetup diff --git a/lib/rubocop/cop/rspec/output.rb b/lib/rubocop/cop/rspec/output.rb new file mode 100644 index 000000000..4fc652921 --- /dev/null +++ b/lib/rubocop/cop/rspec/output.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # NOTE: Originally based on the `Rails/Output` cop. + module RSpec + # Checks for the use of output calls like puts and print in specs. + # + # @safety + # This autocorrection is marked as unsafe because, in rare cases, print + # statements can be used on purpose for integration testing and deleting + # them will cause tests to fail. + # + # @example + # # bad + # puts 'A debug message' + # pp 'A debug message' + # print 'A debug message' + class Output < Base + extend AutoCorrector + + MSG = 'Do not write to stdout in specs.' + + KERNEL_METHODS = %i[ + ap + p + pp + pretty_print + print + puts + ].to_set.freeze + private_constant :KERNEL_METHODS + + IO_METHODS = %i[ + binwrite + syswrite + write + write_nonblock + ].to_set.freeze + private_constant :IO_METHODS + + RESTRICT_ON_SEND = (KERNEL_METHODS + IO_METHODS).to_a.freeze + + # @!method output?(node) + def_node_matcher :output?, <<~PATTERN + (send nil? KERNEL_METHODS ...) + PATTERN + + # @!method io_output?(node) + def_node_matcher :io_output?, <<~PATTERN + (send + { + (gvar #match_gvar?) + (const {nil? cbase} {:STDOUT :STDERR}) + } + IO_METHODS + ...) + PATTERN + + def on_send(node) # rubocop:disable Metrics/CyclomaticComplexity + return if node.parent&.call_type? || node.block_node + return if !output?(node) && !io_output?(node) + return if node.arguments.any? { |arg| arg.type?(:hash, :block_pass) } + + add_offense(node) do |corrector| + corrector.remove(node) + end + end + + private + + def match_gvar?(sym) + %i[$stdout $stderr].include?(sym) + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index d64ca9e51..877d466f2 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -75,6 +75,7 @@ require_relative 'rspec/nested_groups' require_relative 'rspec/no_expectation_example' require_relative 'rspec/not_to_not' +require_relative 'rspec/output' require_relative 'rspec/overwriting_setup' require_relative 'rspec/pending' require_relative 'rspec/pending_without_reason' diff --git a/spec/rubocop/cop/rspec/output_spec.rb b/spec/rubocop/cop/rspec/output_spec.rb new file mode 100644 index 000000000..a6393b360 --- /dev/null +++ b/spec/rubocop/cop/rspec/output_spec.rb @@ -0,0 +1,221 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::Output do + it 'registers an offense for using `p` method without a receiver' do + expect_offense(<<~RUBY) + p "edmond dantes" + ^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense for using `puts` method without a receiver' do + expect_offense(<<~RUBY) + puts "sinbad" + ^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense for using `print` method without a receiver' do + expect_offense(<<~RUBY) + print "abbe busoni" + ^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense for using `pp` method without a receiver' do + expect_offense(<<~RUBY) + pp "monte cristo" + ^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense with `$stdout.write`' do + expect_offense(<<~RUBY) + $stdout.write "lord wilmore" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense with `$stderr.syswrite`' do + expect_offense(<<~RUBY) + $stderr.syswrite "faria" + ^^^^^^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense with `STDOUT.write`' do + expect_offense(<<~RUBY) + STDOUT.write "bertuccio" + ^^^^^^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense with `::STDOUT.write`' do + expect_offense(<<~RUBY) + ::STDOUT.write "bertuccio" + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense with `STDERR.write`' do + expect_offense(<<~RUBY) + STDERR.write "bertuccio" + ^^^^^^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'registers an offense with `::STDERR.write`' do + expect_offense(<<~RUBY) + ::STDERR.write "bertuccio" + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'does not record an offense for methods with a receiver' do + expect_no_offenses(<<~RUBY) + obj.print + something.p + nothing.pp + RUBY + end + + it 'registers an offense for methods without arguments' do + expect_offense(<<~RUBY) + print + ^^^^^ Do not write to stdout in specs. + pp + ^^ Do not write to stdout in specs. + puts + ^^^^ Do not write to stdout in specs. + $stdout.write + ^^^^^^^^^^^^^ Do not write to stdout in specs. + STDERR.write + ^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + + + + + RUBY + end + + it 'registers an offense when `p` method with positional argument' do + expect_offense(<<~RUBY) + p(do_something) + ^^^^^^^^^^^^^^^ Do not write to stdout in specs. + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + + it 'does not register an offense when a method is called ' \ + 'to a local variable with the same name as a print method' do + expect_no_offenses(<<~RUBY) + p.do_something + RUBY + end + + it 'does not register an offense when `p` method with keyword argument' do + expect_no_offenses(<<~RUBY) + p(class: 'this `p` method is a DSL') + RUBY + end + + it 'does not register an offense when `p` method with symbol proc' do + expect_no_offenses(<<~RUBY) + p(&:this_p_method_is_a_dsl) + RUBY + end + + it 'does not register an offense when the `p` method is called ' \ + 'with block argument' do + expect_no_offenses(<<~RUBY) + # phlex-rails gem. + div do + p { 'Some text' } + end + RUBY + end + + it 'does not register an offense when io method is called ' \ + 'with block argument' do + expect_no_offenses(<<~RUBY) + obj.write { do_somethig } + RUBY + end + + it 'does not register an offense when io method is called ' \ + 'with numbered block argument' do + expect_no_offenses(<<~RUBY) + obj.write { do_something(_1) } + RUBY + end + + it 'does not register an offense when io method is called ' \ + 'with `it` parameter', :ruby34, unsupported_on: :parser do + expect_no_offenses(<<~RUBY) + obj.write { do_something(it) } + RUBY + end + + it 'does not register an offense when a method is safe navigation called ' \ + 'to a local variable with the same name as a print method' do + expect_no_offenses(<<~RUBY) + p&.do_something + RUBY + end + + it 'does not record an offense for comments' do + expect_no_offenses(<<~RUBY) + # print "test" + # p + # $stdout.write + # STDERR.binwrite + RUBY + end +end