Skip to content

Commit

Permalink
Fix an incorrect autocorrect for Style/SpecialGlobalVars
Browse files Browse the repository at this point in the history
This PR fixes the following incorrect autocorrect for `Style/SpecialGlobalVars`
when global variable as Perl name is used multiple times.

Example:

```ruby
% cat example.rb
$*[0]
$*[1]

% bundle exec rubocop --only Style/SpecialGlobalVars -A
Inspecting 3 files
.C.

Offenses:

example.rb:1:1: C: [Corrected] Style/SpecialGlobalVars: Prefer $ARGV
from the stdlib 'English' module (don't forget to require it) or ARGV
over $*.
$*[0]
^^
example.rb:2:1: C: [Corrected] Style/SpecialGlobalVars: Prefer $ARGV
from the stdlib 'English' module (don't forget to require it) or ARGV
over $*.
$*[1]
^^

3 files inspected, 2 offenses detected, 2 offenses corrected
```

This auto-correction will add `require 'English'` twice:

```ruby
% cat example.rb
require 'English'
require 'English'
$ARGV[0]
$ARGV[1]
```

This PR ensures that `require 'English'` is added only once:

```ruby
% cat example.rb
require 'English'
$ARGV[0]
$ARGV[1]
```
  • Loading branch information
koic authored and bbatsov committed Apr 20, 2022
1 parent 118fd7d commit a2cfdc1
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#10541](https://github.com/rubocop/rubocop/pull/10541): Fix an incorrect autocorrect for `Style/SpecialGlobalVars` when global variable as Perl name is used multiple times. ([@koic][])
12 changes: 11 additions & 1 deletion lib/rubocop/cop/style/special_global_vars.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ class SpecialGlobalVars < Base

LIBRARY_NAME = 'English'

def on_new_investigation
super
@required_english = false
end

def on_gvar(node)
global_var, = *node

Expand Down Expand Up @@ -172,7 +177,11 @@ def message(global_var)
def autocorrect(corrector, node, global_var)
node = node.parent while node.parent&.begin_type? && node.parent.children.one?

ensure_required(corrector, node, LIBRARY_NAME) if should_require_english?(global_var)
if should_require_english?(global_var)
ensure_required(corrector, node, LIBRARY_NAME)

@required_english = true
end

corrector.replace(node, replacement(node, global_var))
end
Expand Down Expand Up @@ -243,6 +252,7 @@ def add_require_english?
def should_require_english?(global_var)
style == :use_english_names &&
add_require_english? &&
!@required_english &&
!NON_ENGLISH_VARS.include?(preferred_names(global_var).first)
end
end
Expand Down
19 changes: 19 additions & 0 deletions spec/rubocop/cop/style/special_global_vars_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,25 @@
RUBY
end

it 'adds require English for twice `$*` in nested code' do
expect_offense(<<~RUBY)
# frozen_string_literal: true
puts $*[0]
^^ Prefer `$ARGV` from the stdlib 'English' module (don't forget to require it) or `ARGV` over `$*`.
puts $*[1]
^^ Prefer `$ARGV` from the stdlib 'English' module (don't forget to require it) or `ARGV` over `$*`.
RUBY

expect_correction(<<~RUBY)
# frozen_string_literal: true
require 'English'
puts $ARGV[0]
puts $ARGV[1]
RUBY
end

it 'does not add for replacement outside of English lib' do
expect_offense(<<~RUBY)
puts $0
Expand Down

0 comments on commit a2cfdc1

Please sign in to comment.