diff --git a/changelog/change_rails_root_pathname_methods_to_detect_offenses_on_dir_index.md b/changelog/change_rails_root_pathname_methods_to_detect_offenses_on_dir_index.md new file mode 100644 index 0000000000..30a32029f1 --- /dev/null +++ b/changelog/change_rails_root_pathname_methods_to_detect_offenses_on_dir_index.md @@ -0,0 +1 @@ +* [#1003](https://github.com/rubocop/rubocop-rails/pull/1003): Change `Rails/RootPathnameMethods` to detect offenses on `Dir.[]`. ([@r7kamura][]) diff --git a/lib/rubocop/cop/rails/root_pathname_methods.rb b/lib/rubocop/cop/rails/root_pathname_methods.rb index da308656a4..233b1744d0 100644 --- a/lib/rubocop/cop/rails/root_pathname_methods.rb +++ b/lib/rubocop/cop/rails/root_pathname_methods.rb @@ -38,7 +38,7 @@ class RootPathnameMethods < Base # rubocop:disable Metrics/ClassLength MSG = '`%s` is a `Pathname` so you can just append `#%s`.' - DIR_GLOB_METHODS = %i[glob].to_set.freeze + DIR_GLOB_METHODS = %i[[] glob].to_set.freeze DIR_NON_GLOB_METHODS = %i[ children @@ -171,7 +171,7 @@ class RootPathnameMethods < Base # rubocop:disable Metrics/ClassLength def_node_matcher :dir_glob?, <<~PATTERN (send - (const {cbase nil?} :Dir) :glob ...) + (const {cbase nil?} :Dir) DIR_GLOB_METHODS ...) PATTERN def_node_matcher :rails_root_pathname?, <<~PATTERN @@ -190,7 +190,7 @@ def on_send(node) evidence(node) do |method, path, args, rails_root| add_offense(node, message: format(MSG, method: method, rails_root: rails_root.source)) do |corrector| replacement = if dir_glob?(node) - build_path_glob_replacement(path, method) + build_path_glob_replacement(path) else build_path_replacement(path, method, args) end @@ -217,12 +217,12 @@ def pathname_method(node) end end - def build_path_glob_replacement(path, method) + def build_path_glob_replacement(path) receiver = range_between(path.source_range.begin_pos, path.children.first.loc.selector.end_pos).source argument = path.arguments.one? ? path.first_argument.source : join_arguments(path.arguments) - "#{receiver}.#{method}(#{argument})" + "#{receiver}.glob(#{argument})" end def build_path_replacement(path, method, args) diff --git a/spec/rubocop/cop/rails/root_pathname_methods_spec.rb b/spec/rubocop/cop/rails/root_pathname_methods_spec.rb index fdeeeba603..04b9508073 100644 --- a/spec/rubocop/cop/rails/root_pathname_methods_spec.rb +++ b/spec/rubocop/cop/rails/root_pathname_methods_spec.rb @@ -2,15 +2,13 @@ RSpec.describe RuboCop::Cop::Rails::RootPathnameMethods, :config do { - Dir: described_class::DIR_METHODS, + Dir: described_class::DIR_NON_GLOB_METHODS, File: described_class::FILE_METHODS, FileTest: described_class::FILE_TEST_METHODS, FileUtils: described_class::FILE_UTILS_METHODS, IO: described_class::FILE_METHODS }.each do |receiver, methods| methods.each do |method| - next if method == :glob - it "registers an offense when using `#{receiver}.#{method}(Rails.public_path)` (if arity exists)" do expect_offense(<<~RUBY, receiver: receiver, method: method) %{receiver}.%{method}(Rails.public_path) @@ -143,6 +141,19 @@ end end + context 'when using Dir.[] on Ruby 2.5 or higher', :ruby25 do + it 'registers offense when using `Dir[Rails.root.join(...)]`' do + expect_offense(<<~RUBY) + Dir[Rails.root.join('spec/support/**/*.rb')] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#[]`. + RUBY + + expect_correction(<<~RUBY) + Rails.root.glob('spec/support/**/*.rb') + RUBY + end + end + # This is handled by `Rails/RootJoinChain` it 'does not register an offense when using `File.read(Rails.root.join(...).join(...))`' do expect_no_offenses(<<~RUBY)