Skip to content

Commit 94e126d

Browse files
committed
[Fix rubocop#763] Fix a false positive for Rails/RootPathnameMethods
Fixes rubocop#763. This PR fix a false positive for `Rails/RootPathnameMethods` when using `Dir.glob`. And marks unsafe for autocorrection because `Dir`'s methods return string element, but `Pathname`'s methods return `Pathname` element.
1 parent 2f68e9f commit 94e126d

File tree

4 files changed

+102
-2
lines changed

4 files changed

+102
-2
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#763](https://github.com/rubocop/rubocop-rails/issues/763): Mark `Rails/RootPathnameMethods` as unsafe and fix an incorrect autocorrect when using `Dir.glob`. ([@koic][])

config/default.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,7 @@ Rails/RootJoinChain:
845845
Rails/RootPathnameMethods:
846846
Description: 'Use `Rails.root` IO methods instead of passing it to `File`.'
847847
Enabled: pending
848+
SafeAutocorrect: false
848849
VersionAdded: '2.16'
849850

850851
Rails/RootPublicPath:

lib/rubocop/cop/rails/root_pathname_methods.rb

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ module Rails
1111
# This cop works best when used together with
1212
# `Style/FileRead`, `Style/FileWrite` and `Rails/RootJoinChain`.
1313
#
14+
# @safety
15+
# This cop is unsafe for autocorrection because `Dir`'s `children`, `each_child`, `entries`, and `glob`
16+
# methods return string element, but these methods of `Pathname` return `Pathname` element.
17+
#
1418
# @example
1519
# # bad
1620
# File.open(Rails.root.join('db', 'schema.rb'))
@@ -30,6 +34,7 @@ module Rails
3034
#
3135
class RootPathnameMethods < Base
3236
extend AutoCorrector
37+
include RangeHelp
3338

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

@@ -138,6 +143,11 @@ class RootPathnameMethods < Base
138143
}
139144
PATTERN
140145

146+
def_node_matcher :dir_glob?, <<~PATTERN
147+
(send
148+
(const {cbase nil?} :Dir) :glob ...)
149+
PATTERN
150+
141151
def_node_matcher :rails_root_pathname?, <<~PATTERN
142152
{
143153
$#rails_root?
@@ -153,8 +163,12 @@ class RootPathnameMethods < Base
153163
def on_send(node)
154164
evidence(node) do |method, path, args, rails_root|
155165
add_offense(node, message: format(MSG, method: method, rails_root: rails_root.source)) do |corrector|
156-
replacement = "#{path.source}.#{method}"
157-
replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?
166+
if dir_glob?(node)
167+
replacement = build_path_glob(path, method)
168+
else
169+
replacement = "#{path.source}.#{method}"
170+
replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?
171+
end
158172

159173
corrector.replace(node, replacement)
160174
end
@@ -169,6 +183,31 @@ def evidence(node)
169183

170184
yield(method, path, args, rails_root)
171185
end
186+
187+
def build_path_glob(path, method)
188+
receiver = range_between(path.loc.expression.begin_pos, path.children.first.loc.selector.end_pos).source
189+
190+
argument = if path.arguments.one?
191+
path.first_argument.source
192+
else
193+
join_arguments(path.arguments)
194+
end
195+
196+
"#{receiver}.#{method}(#{argument})"
197+
end
198+
199+
def include_interpolation?(arguments)
200+
arguments.any? do |argument|
201+
argument.children.any? { |child| child.respond_to?(:begin_type?) && child.begin_type? }
202+
end
203+
end
204+
205+
def join_arguments(arguments)
206+
quote = include_interpolation?(arguments) ? '"' : "'"
207+
joined_arguments = arguments.map(&:value).join('/')
208+
209+
"#{quote}#{joined_arguments}#{quote}"
210+
end
172211
end
173212
end
174213
end

spec/rubocop/cop/rails/root_pathname_methods_spec.rb

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
IO: described_class::FILE_METHODS
1010
}.each do |receiver, methods|
1111
methods.each do |method|
12+
next if method == :glob
13+
1214
it "registers an offense when using `#{receiver}.#{method}(Rails.public_path)` (if arity exists)" do
1315
expect_offense(<<~RUBY, receiver: receiver, method: method)
1416
%{receiver}.%{method}(Rails.public_path)
@@ -44,6 +46,63 @@
4446
end
4547
end
4648

49+
context 'when using `Dir.glob`' do
50+
it "registers an offense when using `Dir.glob(Rails.root.join('**/*.rb'))`" do
51+
expect_offense(<<~RUBY)
52+
Dir.glob(Rails.root.join('**/*.rb'))
53+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
54+
RUBY
55+
56+
expect_correction(<<~RUBY)
57+
Rails.root.glob('**/*.rb')
58+
RUBY
59+
end
60+
61+
it "registers an offense when using `::Dir.glob(Rails.root.join('**/*.rb'))`" do
62+
expect_offense(<<~RUBY)
63+
::Dir.glob(Rails.root.join('**/*.rb'))
64+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
65+
RUBY
66+
67+
expect_correction(<<~RUBY)
68+
Rails.root.glob('**/*.rb')
69+
RUBY
70+
end
71+
72+
it "registers an offense when using `Dir.glob(Rails.root.join('**/\#{path}/*.rb'))`" do
73+
expect_offense(<<~'RUBY')
74+
Dir.glob(Rails.root.join("**/#{path}/*.rb"))
75+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
76+
RUBY
77+
78+
expect_correction(<<~'RUBY')
79+
Rails.root.glob("**/#{path}/*.rb")
80+
RUBY
81+
end
82+
83+
it "registers an offense when using `Dir.glob(Rails.root.join('**', '*.rb'))`" do
84+
expect_offense(<<~RUBY)
85+
Dir.glob(Rails.root.join('**', '*.rb'))
86+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
87+
RUBY
88+
89+
expect_correction(<<~RUBY)
90+
Rails.root.glob('**/*.rb')
91+
RUBY
92+
end
93+
94+
it "registers an offense when using `Dir.glob(Rails.root.join('**', \"\#{path}\", '*.rb'))`" do
95+
expect_offense(<<~'RUBY')
96+
Dir.glob(Rails.root.join('**', "#{path}", '*.rb'))
97+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
98+
RUBY
99+
100+
expect_correction(<<~'RUBY')
101+
Rails.root.glob("**/#{path}/*.rb")
102+
RUBY
103+
end
104+
end
105+
47106
# This is handled by `Rails/RootJoinChain`
48107
it 'does not register an offense when using `File.read(Rails.root.join(...).join(...))`' do
49108
expect_no_offenses(<<~RUBY)

0 commit comments

Comments
 (0)