diff --git a/changelog/fix_add_require_english_for_special_globals.md b/changelog/fix_add_require_english_for_special_globals.md new file mode 100644 index 00000000000..a36ecb14dc7 --- /dev/null +++ b/changelog/fix_add_require_english_for_special_globals.md @@ -0,0 +1 @@ +* [#4097](https://github.com/rubocop/rubocop/issues/4097): Add require English for special globals. ([@biinari][]) diff --git a/config/default.yml b/config/default.yml index 5fbba4556d7..a750e241315 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4575,6 +4575,7 @@ Style/SpecialGlobalVars: VersionAdded: '0.13' VersionChanged: '0.36' SafeAutoCorrect: false + RequireEnglish: true EnforcedStyle: use_english_names SupportedStyles: - use_perl_names diff --git a/lib/rubocop.rb b/lib/rubocop.rb index c9a937ea142..4b475009723 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -111,6 +111,7 @@ require_relative 'rubocop/cop/mixin/preceding_following_alignment' require_relative 'rubocop/cop/mixin/preferred_delimiters' require_relative 'rubocop/cop/mixin/rational_literal' +require_relative 'rubocop/cop/mixin/require_library' require_relative 'rubocop/cop/mixin/rescue_node' require_relative 'rubocop/cop/mixin/safe_assignment' require_relative 'rubocop/cop/mixin/space_after_punctuation' @@ -144,6 +145,7 @@ require_relative 'rubocop/cop/correctors/parentheses_corrector' require_relative 'rubocop/cop/correctors/percent_literal_corrector' require_relative 'rubocop/cop/correctors/punctuation_corrector' +require_relative 'rubocop/cop/correctors/require_library_corrector' require_relative 'rubocop/cop/correctors/space_corrector' require_relative 'rubocop/cop/correctors/string_literal_corrector' require_relative 'rubocop/cop/correctors/unused_arg_corrector' diff --git a/lib/rubocop/cop/correctors/require_library_corrector.rb b/lib/rubocop/cop/correctors/require_library_corrector.rb new file mode 100644 index 00000000000..8a8b7a83607 --- /dev/null +++ b/lib/rubocop/cop/correctors/require_library_corrector.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # This class ensures a require statement is present for a standard library + # determined by the variable library_name + class RequireLibraryCorrector + extend RangeHelp + + class << self + def correct(corrector, node, library_name) + node = node.parent while node.parent? + node = node.children.first if node.begin_type? + corrector.insert_before(node, require_statement(library_name)) + end + + def require_statement(library_name) + "require '#{library_name}'\n" + end + end + end + end +end diff --git a/lib/rubocop/cop/mixin/require_library.rb b/lib/rubocop/cop/mixin/require_library.rb new file mode 100644 index 00000000000..ec1fd338d8f --- /dev/null +++ b/lib/rubocop/cop/mixin/require_library.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Ensure a require statement is present for a standard library determined + # by variable library_name + module RequireLibrary + extend NodePattern::Macros + + def ensure_required(corrector, node, library_name) + node = node.parent while node.parent&.parent? + + if node.parent&.begin_type? + return if @required_libs.include?(library_name) + + remove_subsequent_requires(corrector, node, library_name) + end + + RequireLibraryCorrector.correct(corrector, node, library_name) + end + + def remove_subsequent_requires(corrector, node, library_name) + node.right_siblings.each do |sibling| + next unless require_library_name?(sibling, library_name) + + range = range_by_whole_lines(sibling.source_range, include_final_newline: true) + corrector.remove(range) + end + end + + def on_send(node) + return if node.parent&.parent? + + name = require_any_library?(node) + return if name.nil? + + @required_libs.add(name) + end + + private + + def on_new_investigation + # Holds the required files at top-level + @required_libs = Set.new + super + end + + # @!method require_any_library?(node) + def_node_matcher :require_any_library?, <<~PATTERN + (send {(const {nil? cbase} :Kernel) nil?} :require (str $_)) + PATTERN + + # @!method require_library_name?(node, library_name) + def_node_matcher :require_library_name?, <<~PATTERN + (send {(const {nil? cbase} :Kernel) nil?} :require (str %1)) + PATTERN + end + end +end diff --git a/lib/rubocop/cop/style/special_global_vars.rb b/lib/rubocop/cop/style/special_global_vars.rb index 3c7dff432aa..5e54f44f9e7 100644 --- a/lib/rubocop/cop/style/special_global_vars.rb +++ b/lib/rubocop/cop/style/special_global_vars.rb @@ -5,9 +5,14 @@ module Cop module Style # # This cop looks for uses of Perl-style global variables. + # Correcting to global variables in the 'English' library + # will add a require statement to the top of the file if + # enabled by RequireEnglish config. # # @example EnforcedStyle: use_english_names (default) # # good + # require 'English' # or this could be in another file. + # # puts $LOAD_PATH # puts $LOADED_FEATURES # puts $PROGRAM_NAME @@ -50,6 +55,8 @@ module Style # class SpecialGlobalVars < Base include ConfigurableEnforcedStyle + include RangeHelp + include RequireLibrary extend AutoCorrector MSG_BOTH = 'Prefer `%s` from the stdlib \'English\' ' \ @@ -90,6 +97,8 @@ class SpecialGlobalVars < Base # Anything *not* in this set is provided by the English library. NON_ENGLISH_VARS = Set.new(%i[$LOAD_PATH $LOADED_FEATURES $PROGRAM_NAME ARGV]).freeze + LIBRARY_NAME = 'English' + def on_gvar(node) global_var, = *node @@ -117,6 +126,8 @@ 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) + corrector.replace(node, replacement(node, global_var)) end @@ -172,6 +183,16 @@ def english_name_replacement(preferred_name, node) "{#{preferred_name}}" end + + def add_require_english? + cop_config['RequireEnglish'] + end + + def should_require_english?(global_var) + style == :use_english_names && + add_require_english? && + !NON_ENGLISH_VARS.include?(preferred_names(global_var).first) + end end end end diff --git a/spec/rubocop/cli/auto_gen_config_spec.rb b/spec/rubocop/cli/auto_gen_config_spec.rb index 78bb5b2659d..d366161830a 100644 --- a/spec/rubocop/cli/auto_gen_config_spec.rb +++ b/spec/rubocop/cli/auto_gen_config_spec.rb @@ -1140,7 +1140,7 @@ def function(arg1, arg2, arg3, arg4, arg5, arg6, arg7) # Offense count: 2 # Cop supports --auto-correct. - # Configuration parameters: EnforcedStyle. + # Configuration parameters: RequireEnglish, EnforcedStyle. # SupportedStyles: use_perl_names, use_english_names Style/SpecialGlobalVars: Enabled: false @@ -1165,7 +1165,7 @@ def function(arg1, arg2, arg3, arg4, arg5, arg6, arg7) # Offense count: 3 # Cop supports --auto-correct. - # Configuration parameters: EnforcedStyle. + # Configuration parameters: RequireEnglish, EnforcedStyle. # SupportedStyles: use_perl_names, use_english_names Style/SpecialGlobalVars: Exclude: diff --git a/spec/rubocop/cli/options_spec.rb b/spec/rubocop/cli/options_spec.rb index bed23fbe71d..d5596debb9e 100644 --- a/spec/rubocop/cli/options_spec.rb +++ b/spec/rubocop/cli/options_spec.rb @@ -1800,6 +1800,7 @@ def f 1 file inspected, 1 offense detected, 1 offense corrected ==================== + require 'English' p $INPUT_RECORD_SEPARATOR RESULT ensure @@ -1823,6 +1824,7 @@ def f ==================== RESULT expect($stdout.string).to eq(<<~RESULT.chomp) + require 'English' p $INPUT_RECORD_SEPARATOR RESULT ensure diff --git a/spec/rubocop/cop/style/special_global_vars_spec.rb b/spec/rubocop/cop/style/special_global_vars_spec.rb index 25a12db6d83..d2303dc612d 100644 --- a/spec/rubocop/cop/style/special_global_vars_spec.rb +++ b/spec/rubocop/cop/style/special_global_vars_spec.rb @@ -2,122 +2,213 @@ RSpec.describe RuboCop::Cop::Style::SpecialGlobalVars, :config do context 'when style is use_english_names' do - let(:cop_config) { { 'EnforcedStyle' => 'use_english_names' } } - - it 'registers an offense for $:' do - expect_offense(<<~RUBY) - puts $: - ^^ Prefer `$LOAD_PATH` over `$:`. - RUBY - - expect_correction(<<~RUBY) - puts $LOAD_PATH - RUBY - end - - it 'registers an offense for $"' do - expect_offense(<<~RUBY) - puts $" - ^^ Prefer `$LOADED_FEATURES` over `$"`. - RUBY - - expect_correction(<<~RUBY) - puts $LOADED_FEATURES - RUBY - end - - it 'registers an offense for $0' do - expect_offense(<<~RUBY) - puts $0 - ^^ Prefer `$PROGRAM_NAME` over `$0`. - RUBY - - expect_correction(<<~RUBY) - puts $PROGRAM_NAME - RUBY - end - - it 'registers an offense for $$' do - expect_offense(<<~RUBY) - puts $$ - ^^ Prefer `$PROCESS_ID` or `$PID` from the stdlib 'English' module (don't forget to require it) over `$$`. - RUBY - - expect_correction(<<~RUBY) - puts $PROCESS_ID - RUBY - end - - it 'is clear about variables from the English library vs those not' do - expect_offense(<<~RUBY) - puts $* - ^^ Prefer `$ARGV` from the stdlib 'English' module (don't forget to require it) or `ARGV` over `$*`. - RUBY - - expect_correction(<<~RUBY) - puts $ARGV - RUBY + context 'when add require English is disabled' do + let(:cop_config) do + { + 'EnforcedStyle' => 'use_english_names', + 'RequireEnglish' => false + } + end + + it 'registers an offense for $:' do + expect_offense(<<~RUBY) + puts $: + ^^ Prefer `$LOAD_PATH` over `$:`. + RUBY + + expect_correction(<<~RUBY) + puts $LOAD_PATH + RUBY + end + + it 'registers an offense for $"' do + expect_offense(<<~RUBY) + puts $" + ^^ Prefer `$LOADED_FEATURES` over `$"`. + RUBY + + expect_correction(<<~RUBY) + puts $LOADED_FEATURES + RUBY + end + + it 'registers an offense for $0' do + expect_offense(<<~RUBY) + puts $0 + ^^ Prefer `$PROGRAM_NAME` over `$0`. + RUBY + + expect_correction(<<~RUBY) + puts $PROGRAM_NAME + RUBY + end + + it 'registers an offense for $$' do + expect_offense(<<~RUBY) + puts $$ + ^^ Prefer `$PROCESS_ID` or `$PID` from the stdlib 'English' module (don't forget to require it) over `$$`. + RUBY + + expect_correction(<<~RUBY) + puts $PROCESS_ID + RUBY + end + + it 'is clear about variables from the English library vs those not' do + expect_offense(<<~RUBY) + puts $* + ^^ Prefer `$ARGV` from the stdlib 'English' module (don't forget to require it) or `ARGV` over `$*`. + RUBY + + expect_correction(<<~RUBY) + puts $ARGV + RUBY + end + + it 'does not register an offense for backrefs like $1' do + expect_no_offenses('puts $1') + end + + it 'auto-corrects $/ to $INPUT_RECORD_SEPARATOR' do + expect_offense(<<~RUBY) + $/ + ^^ Prefer `$INPUT_RECORD_SEPARATOR` or `$RS` from the stdlib 'English' module (don't forget to require it) over `$/`. + RUBY + + expect_correction(<<~RUBY) + $INPUT_RECORD_SEPARATOR + RUBY + end + + it 'auto-corrects #$: to #{$LOAD_PATH}' do + expect_offense(<<~'RUBY') + "#$:" + ^^ Prefer `$LOAD_PATH` over `$:`. + RUBY + + expect_correction(<<~'RUBY') + "#{$LOAD_PATH}" + RUBY + end + + it 'auto-corrects #{$!} to #{$ERROR_INFO}' do + expect_offense(<<~'RUBY') + "#{$!}" + ^^ Prefer `$ERROR_INFO` from the stdlib 'English' module (don't forget to require it) over `$!`. + RUBY + + expect_correction(<<~'RUBY') + "#{$ERROR_INFO}" + RUBY + end + + it 'generates correct auto-config when Perl variable names are used' do + expect_offense(<<~RUBY) + $0 + ^^ Prefer `$PROGRAM_NAME` over `$0`. + RUBY + expect(cop.config_to_allow_offenses).to eq('EnforcedStyle' => 'use_perl_names') + + expect_correction(<<~RUBY) + $PROGRAM_NAME + RUBY + end + + it 'generates correct auto-config when mixed styles are used' do + expect_offense(<<~RUBY) + $!; $ERROR_INFO + ^^ Prefer `$ERROR_INFO` from the stdlib 'English' module (don't forget to require it) over `$!`. + RUBY + expect(cop.config_to_allow_offenses).to eq('Enabled' => false) + + expect_correction(<<~RUBY) + $ERROR_INFO; $ERROR_INFO + RUBY + end end - it 'does not register an offense for backrefs like $1' do - expect_no_offenses('puts $1') - end - - it 'auto-corrects $/ to $INPUT_RECORD_SEPARATOR' do - expect_offense(<<~RUBY) - $/ - ^^ Prefer `$INPUT_RECORD_SEPARATOR` or `$RS` from the stdlib 'English' module (don't forget to require it) over `$/`. - RUBY - - expect_correction(<<~RUBY) - $INPUT_RECORD_SEPARATOR - RUBY - end - - it 'auto-corrects #$: to #{$LOAD_PATH}' do - expect_offense(<<~'RUBY') - "#$:" - ^^ Prefer `$LOAD_PATH` over `$:`. - RUBY - - expect_correction(<<~'RUBY') - "#{$LOAD_PATH}" - RUBY - end - - it 'auto-corrects #{$!} to #{$ERROR_INFO}' do - expect_offense(<<~'RUBY') - "#{$!}" - ^^ Prefer `$ERROR_INFO` from the stdlib 'English' module (don't forget to require it) over `$!`. - RUBY - - expect_correction(<<~'RUBY') - "#{$ERROR_INFO}" - RUBY - end - - it 'generates correct auto-config when Perl variable names are used' do - expect_offense(<<~RUBY) - $0 - ^^ Prefer `$PROGRAM_NAME` over `$0`. - RUBY - expect(cop.config_to_allow_offenses).to eq('EnforcedStyle' => 'use_perl_names') - - expect_correction(<<~RUBY) - $PROGRAM_NAME - RUBY - end - - it 'generates correct auto-config when mixed styles are used' do - expect_offense(<<~RUBY) - $!; $ERROR_INFO - ^^ Prefer `$ERROR_INFO` from the stdlib 'English' module (don't forget to require it) over `$!`. - RUBY - expect(cop.config_to_allow_offenses).to eq('Enabled' => false) - - expect_correction(<<~RUBY) - $ERROR_INFO; $ERROR_INFO - RUBY + context 'when add require English is enabled' do + let(:cop_config) do + { + 'EnforcedStyle' => 'use_english_names', + 'RequireEnglish' => true + } + end + + context 'when English has not been required at top-level' do + it 'adds require English for $$' do + expect_offense(<<~RUBY) + puts $$ + ^^ Prefer `$PROCESS_ID` or `$PID` from the stdlib 'English' module (don't forget to require it) over `$$`. + RUBY + + expect_correction(<<~RUBY) + require 'English' + puts $PROCESS_ID + RUBY + end + + it 'adds require English for $$ in nested code' do + expect_offense(<<~RUBY) + # frozen_string_literal: true + + x = true + if x + puts $$ + ^^ Prefer `$PROCESS_ID` or `$PID` from the stdlib 'English' module (don't forget to require it) over `$$`. + end + RUBY + + expect_correction(<<~RUBY) + # frozen_string_literal: true + + require 'English' + x = true + if x + puts $PROCESS_ID + end + RUBY + end + + it 'does not add for replacement outside of English lib' do + expect_offense(<<~RUBY) + puts $0 + ^^ Prefer `$PROGRAM_NAME` over `$0`. + RUBY + + expect_correction(<<~RUBY) + puts $PROGRAM_NAME + RUBY + end + end + + context 'when English is already required at top-level' do + it 'leaves require English alone for $$' do + expect_offense(<<~RUBY) + require 'English' + puts $$ + ^^ Prefer `$PROCESS_ID` or `$PID` from the stdlib 'English' module (don't forget to require it) over `$$`. + RUBY + + expect_correction(<<~RUBY) + require 'English' + puts $PROCESS_ID + RUBY + end + + it 'moves require English above replacement' do + expect_offense(<<~RUBY) + puts $$ + ^^ Prefer `$PROCESS_ID` or `$PID` from the stdlib 'English' module (don't forget to require it) over `$$`. + require 'English' + RUBY + + expect_correction(<<~RUBY) + require 'English' + puts $PROCESS_ID + RUBY + end + end end end