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

Change Rails/RootPathnameMethods to detect offenses on Dir.[] #1003

Merged
merged 1 commit into from
Aug 15, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1003](https://github.com/rubocop/rubocop-rails/pull/1003): Change `Rails/RootPathnameMethods` to detect offenses on `Dir.[]`. ([@r7kamura][])
10 changes: 5 additions & 5 deletions lib/rubocop/cop/rails/root_pathname_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class RootPathnameMethods < Base # rubocop:disable Metrics/ClassLength

MSG = '`%<rails_root>s` is a `Pathname` so you can just append `#%<method>s`.'

DIR_GLOB_METHODS = %i[glob].to_set.freeze
DIR_GLOB_METHODS = %i[[] glob].to_set.freeze

DIR_NON_GLOB_METHODS = %i[
children
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
17 changes: 14 additions & 3 deletions spec/rubocop/cop/rails/root_pathname_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down