Skip to content

Commit 0f36342

Browse files
EarlopainEarlopain
committed
[Fix #1345] Improve offense message for Rails/RootPathnameMethods
The current message is confusing/nonsensical for `Dir[]`. Let's display the correct code instead to show what needs to be done
1 parent 4c1b7d7 commit 0f36342

File tree

2 files changed

+24
-20
lines changed

2 files changed

+24
-20
lines changed

‎lib/rubocop/cop/rails/root_pathname_methods.rb

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ module Rails
2323
# File.binread(Rails.root.join('db', 'schema.rb'))
2424
# File.write(Rails.root.join('db', 'schema.rb'), content)
2525
# File.binwrite(Rails.root.join('db', 'schema.rb'), content)
26+
# Dir.glob(Rails.root.join('db', 'schema.rb'))
27+
# Dir[Rails.root.join('db', 'schema.rb')]
2628
#
2729
# # good
2830
# Rails.root.join('db', 'schema.rb').open
@@ -31,12 +33,13 @@ module Rails
3133
# Rails.root.join('db', 'schema.rb').binread
3234
# Rails.root.join('db', 'schema.rb').write(content)
3335
# Rails.root.join('db', 'schema.rb').binwrite(content)
36+
# Rails.root.glob("db/schema.rb")
3437
#
3538
class RootPathnameMethods < Base # rubocop:disable Metrics/ClassLength
3639
extend AutoCorrector
3740
include RangeHelp
3841

39-
MSG = '`%<rails_root>s` is a `Pathname` so you can just append `#%<method>s`.'
42+
MSG = '`%<rails_root>s` is a `Pathname`, so you can use `%<replacement>s`.'
4043

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

@@ -188,13 +191,14 @@ class RootPathnameMethods < Base # rubocop:disable Metrics/ClassLength
188191

189192
def on_send(node)
190193
evidence(node) do |method, path, args, rails_root|
191-
add_offense(node, message: format(MSG, method: method, rails_root: rails_root.source)) do |corrector|
192-
replacement = if dir_glob?(node)
193-
build_path_glob_replacement(path)
194-
else
195-
build_path_replacement(path, method, args)
196-
end
194+
replacement = if dir_glob?(node)
195+
build_path_glob_replacement(path)
196+
else
197+
build_path_replacement(path, method, args)
198+
end
197199

200+
message = format(MSG, rails_root: rails_root.source, replacement: replacement)
201+
add_offense(node, message: message) do |corrector|
198202
corrector.replace(node, replacement)
199203
end
200204
end

‎spec/rubocop/cop/rails/root_pathname_methods_spec.rb

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
it "registers an offense when using `#{receiver}.#{method}(Rails.public_path)` (if arity exists)" do
1313
expect_offense(<<~RUBY, receiver: receiver, method: method)
1414
%{receiver}.%{method}(Rails.public_path)
15-
^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^ `Rails.public_path` is a `Pathname` so you can just append `#%{method}`.
15+
^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^ `Rails.public_path` is a `Pathname`, so you can use `Rails.public_path.%{method}`.
1616
RUBY
1717

1818
expect_correction(<<~RUBY)
@@ -23,7 +23,7 @@
2323
it "registers an offense when using `::#{receiver}.#{method}(::Rails.root.join(...))` (if arity exists)" do
2424
expect_offense(<<~RUBY, receiver: receiver, method: method)
2525
::%{receiver}.%{method}(::Rails.root.join('db', 'schema.rb'))
26-
^^^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::Rails.root` is a `Pathname` so you can just append `#%{method}`.
26+
^^^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::Rails.root` is a `Pathname`, so you can use `::Rails.root.join('db', 'schema.rb').%{method}`.
2727
RUBY
2828

2929
expect_correction(<<~RUBY)
@@ -34,7 +34,7 @@
3434
it "registers an offense when using `::#{receiver}.#{method}(::Rails.root.join(...), ...)` (if arity exists)" do
3535
expect_offense(<<~RUBY, receiver: receiver, method: method)
3636
::%{receiver}.%{method}(::Rails.root.join('db', 'schema.rb'), 20, 5)
37-
^^^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::Rails.root` is a `Pathname` so you can just append `#%{method}`.
37+
^^^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::Rails.root` is a `Pathname`, so you can use `::Rails.root.join('db', 'schema.rb').%{method}(20, 5)`.
3838
RUBY
3939

4040
expect_correction(<<~RUBY)
@@ -56,7 +56,7 @@
5656
it "registers an offense when using `Dir.glob(Rails.root.join('**/*.rb'))`" do
5757
expect_offense(<<~RUBY)
5858
Dir.glob(Rails.root.join('**/*.rb'))
59-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
59+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob('**/*.rb')`.
6060
RUBY
6161

6262
expect_correction(<<~RUBY)
@@ -67,7 +67,7 @@
6767
it "registers an offense when using `::Dir.glob(Rails.root.join('**/*.rb'))`" do
6868
expect_offense(<<~RUBY)
6969
::Dir.glob(Rails.root.join('**/*.rb'))
70-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
70+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob('**/*.rb')`.
7171
RUBY
7272

7373
expect_correction(<<~RUBY)
@@ -78,7 +78,7 @@
7878
it "registers an offense when using `Dir.glob(Rails.root.join('**/\#{path}/*.rb'))`" do
7979
expect_offense(<<~'RUBY')
8080
Dir.glob(Rails.root.join("**/#{path}/*.rb"))
81-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
81+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob("**/#{path}/*.rb")`.
8282
RUBY
8383

8484
expect_correction(<<~'RUBY')
@@ -89,7 +89,7 @@
8989
it "registers an offense when using `Dir.glob(Rails.root.join('**', '*.rb'))`" do
9090
expect_offense(<<~RUBY)
9191
Dir.glob(Rails.root.join('**', '*.rb'))
92-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
92+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob('**/*.rb')`.
9393
RUBY
9494

9595
expect_correction(<<~RUBY)
@@ -105,7 +105,7 @@
105105
it "registers an offense when using `Dir.glob(Rails.root.join('**', '*.rb'))`" do
106106
expect_offense(<<~RUBY)
107107
Dir.glob(Rails.root.join('**', '*.rb'))
108-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
108+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob("**/*.rb")`.
109109
RUBY
110110

111111
expect_correction(<<~RUBY)
@@ -117,7 +117,7 @@
117117
it "registers an offense when using `Dir.glob(Rails.root.join('**', \"\#{path}\", '*.rb'))`" do
118118
expect_offense(<<~'RUBY')
119119
Dir.glob(Rails.root.join('**', "#{path}", '*.rb'))
120-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
120+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob("**/#{path}/*.rb")`.
121121
RUBY
122122

123123
expect_correction(<<~'RUBY')
@@ -128,7 +128,7 @@
128128
it 'registers an offense when using `Rails.env` argument within `Dir.glob`' do
129129
expect_offense(<<~RUBY)
130130
Dir.glob(Rails.root.join("db", "seeds", Rails.env, "*.rb")).sort.each do |file|
131-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#glob`.
131+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob("db/seeds/\#{Rails.env}/*.rb")`.
132132
load file
133133
end
134134
RUBY
@@ -145,7 +145,7 @@
145145
it 'registers offense when using `Dir[Rails.root.join(...)]`' do
146146
expect_offense(<<~RUBY)
147147
Dir[Rails.root.join('spec/support/**/*.rb')]
148-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#[]`.
148+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.glob('spec/support/**/*.rb')`.
149149
RUBY
150150

151151
expect_correction(<<~RUBY)
@@ -192,7 +192,7 @@
192192
it 'registers an offense when using `File.open(Rails.root.join(...), ...)` inside an iterator' do
193193
expect_offense(<<~RUBY)
194194
files.map { |file| File.open(Rails.root.join('db', file), 'wb') }
195-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#open`.
195+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.join('db', file).open('wb')`.
196196
RUBY
197197

198198
expect_correction(<<~RUBY)
@@ -203,7 +203,7 @@
203203
it 'registers an offense when using `File.open Rails.root.join ...` without parens' do
204204
expect_offense(<<~RUBY)
205205
file = File.open Rails.root.join 'docs', 'invoice.pdf'
206-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#open`.
206+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname`, so you can use `Rails.root.join('docs', 'invoice.pdf').open`.
207207
RUBY
208208

209209
expect_correction(<<~RUBY)

0 commit comments

Comments
 (0)