From 260f5e3ef31dfc08bfa19a6e70967be1b827db6f Mon Sep 17 00:00:00 2001 From: fatkodima Date: Thu, 30 Jul 2020 20:09:42 +0300 Subject: [PATCH] Add new `Style/GlobalStdStream` cop --- CHANGELOG.md | 1 + config/default.yml | 6 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_style.adoc | 44 +++++++++++++ lib/rubocop.rb | 1 + lib/rubocop/cop/style/global_std_stream.rb | 65 +++++++++++++++++++ .../cop/style/global_std_stream_spec.rb | 48 ++++++++++++++ 7 files changed, 166 insertions(+) create mode 100644 lib/rubocop/cop/style/global_std_stream.rb create mode 100644 spec/rubocop/cop/style/global_std_stream_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b2ca8d77ff..cef3f0b8d48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * [#8339](https://github.com/rubocop-hq/rubocop/pull/8339): Add `Config#for_badge` as an efficient way to get a cop's config merged with its department's. ([@marcandre][]) * [#5067](https://github.com/rubocop-hq/rubocop/issues/5067): Add new `Style/StringConcatenation` cop. ([@fatkodima][]) * [#7425](https://github.com/rubocop-hq/rubocop/pull/7425): Add new `Lint/TopLevelReturnWithArgument` cop. ([@iamravitejag][]) +* [#8417](https://github.com/rubocop-hq/rubocop/pull/8417): Add new `Style/GlobalStdStream` cop. ([@fatkodima][]) * [#7949](https://github.com/rubocop-hq/rubocop/issues/7949): Add new `Style/SingleArgumentDig` cop. ([@volfgox][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 3ecbb89c509..fe609f251e4 100644 --- a/config/default.yml +++ b/config/default.yml @@ -2987,6 +2987,12 @@ Style/FrozenStringLiteralComment: - never Safe: false +Style/GlobalStdStream: + Description: 'Enforces the use of `$stdout/$stderr/$stdin` instead of `STDOUT/STDERR/STDIN`.' + StyleGuide: '#global-stdout' + Enabled: pending + VersionAdded: '0.89' + Style/GlobalVars: Description: 'Do not introduce global variables.' StyleGuide: '#instance-vars' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index b19e5b64fce..3a481041de7 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -374,6 +374,7 @@ In the following section you find all available cops: * xref:cops_style.adoc#styleformatstring[Style/FormatString] * xref:cops_style.adoc#styleformatstringtoken[Style/FormatStringToken] * xref:cops_style.adoc#stylefrozenstringliteralcomment[Style/FrozenStringLiteralComment] +* xref:cops_style.adoc#styleglobalstdstream[Style/GlobalStdStream] * xref:cops_style.adoc#styleglobalvars[Style/GlobalVars] * xref:cops_style.adoc#styleguardclause[Style/GuardClause] * xref:cops_style.adoc#stylehashaslastarrayitem[Style/HashAsLastArrayItem] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index b45be28ba06..55099ba14b3 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -3308,6 +3308,50 @@ end | `always`, `always_true`, `never` |=== +== Style/GlobalStdStream + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.89 +| - +|=== + +This cop enforces the use of `$stdout/$stderr/$stdin` instead of `STDOUT/STDERR/STDIN`. +`STDOUT/STDERR/STDIN` are constants, and while you can actually +reassign (possibly to redirect some stream) constants in Ruby, you'll get +an interpreter warning if you do so. + +=== Examples + +[source,ruby] +---- +# bad +STDOUT.puts('hello') + +hash = { out: STDOUT, key: value } + +def m(out = STDOUT) + out.puts('hello') +end + +# good +$stdout.puts('hello') + +hash = { out: $stdout, key: value } + +def m(out = $stdout) + out.puts('hello') +end +---- + +=== References + +* https://rubystyle.guide#global-stdout + == Style/GlobalVars |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 3f78bba413d..e84497070ea 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -418,6 +418,7 @@ require_relative 'rubocop/cop/style/format_string' require_relative 'rubocop/cop/style/format_string_token' require_relative 'rubocop/cop/style/frozen_string_literal_comment' +require_relative 'rubocop/cop/style/global_std_stream' require_relative 'rubocop/cop/style/global_vars' require_relative 'rubocop/cop/style/guard_clause' require_relative 'rubocop/cop/style/hash_as_last_array_item' diff --git a/lib/rubocop/cop/style/global_std_stream.rb b/lib/rubocop/cop/style/global_std_stream.rb new file mode 100644 index 00000000000..f210647e306 --- /dev/null +++ b/lib/rubocop/cop/style/global_std_stream.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop enforces the use of `$stdout/$stderr/$stdin` instead of `STDOUT/STDERR/STDIN`. + # `STDOUT/STDERR/STDIN` are constants, and while you can actually + # reassign (possibly to redirect some stream) constants in Ruby, you'll get + # an interpreter warning if you do so. + # + # @example + # # bad + # STDOUT.puts('hello') + # + # hash = { out: STDOUT, key: value } + # + # def m(out = STDOUT) + # out.puts('hello') + # end + # + # # good + # $stdout.puts('hello') + # + # hash = { out: $stdout, key: value } + # + # def m(out = $stdout) + # out.puts('hello') + # end + # + class GlobalStdStream < Base + extend AutoCorrector + + MSG = 'Use `%s` instead of `%s`.' + + STD_STREAMS = %i[STDIN STDOUT STDERR].to_set.freeze + + def_node_matcher :const_to_gvar_assignment?, <<~PATTERN + (gvasgn %1 (const nil? _)) + PATTERN + + def on_const(node) + const_name = node.children[1] + return unless STD_STREAMS.include?(const_name) + + gvar_name = gvar_name(const_name).to_sym + return if const_to_gvar_assignment?(node.parent, gvar_name) + + add_offense(node, message: message(const_name)) do |corrector| + corrector.replace(node, gvar_name) + end + end + + private + + def message(const_name) + format(MSG, gvar_name: gvar_name(const_name), const_name: const_name) + end + + def gvar_name(const_name) + "$#{const_name.to_s.downcase}" + end + end + end + end +end diff --git a/spec/rubocop/cop/style/global_std_stream_spec.rb b/spec/rubocop/cop/style/global_std_stream_spec.rb new file mode 100644 index 00000000000..ad9a7f61ce6 --- /dev/null +++ b/spec/rubocop/cop/style/global_std_stream_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::GlobalStdStream do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when using std stream as const' do + expect_offense(<<~RUBY) + STDOUT.puts('hello') + ^^^^^^ Use `$stdout` instead of `STDOUT`. + + hash = { out: STDOUT, key: value } + ^^^^^^ Use `$stdout` instead of `STDOUT`. + + def m(out = STDOUT) + ^^^^^^ Use `$stdout` instead of `STDOUT`. + out.puts('hello') + end + RUBY + + expect_correction(<<~RUBY) + $stdout.puts('hello') + + hash = { out: $stdout, key: value } + + def m(out = $stdout) + out.puts('hello') + end + RUBY + end + + it 'does not register an offense when using non std stream const' do + expect_no_offenses(<<~RUBY) + SOME_CONST.puts('hello') + RUBY + end + + it 'does not register an offense when assigning std stream const to std stream gvar' do + expect_no_offenses(<<~RUBY) + $stdin = STDIN + RUBY + end + + it 'does not register an offense when assigning other const to std stream gvar' do + expect_no_offenses(<<~RUBY) + $stdin = SOME_CONST + RUBY + end +end