Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Style/GlobalStdStream cop #8417

Merged
merged 1 commit into from Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -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]
Expand Down
44 changes: 44 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -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

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
65 changes: 65 additions & 0 deletions 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 `%<gvar_name>s` instead of `%<const_name>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
48 changes: 48 additions & 0 deletions 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