Skip to content

Commit fa8c533

Browse files
ydakukaydakuka
committed
[Fix rubocop#1177] Fix for Rails/FilePath autocorrect removing an important trailing slash in a dynamic string containing Rails.root 
1 parent c9a5749 commit fa8c533

File tree

2 files changed

+222
-1
lines changed

2 files changed

+222
-1
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ def on_send(node)
8080
def check_for_slash_after_rails_root_in_dstr(node)
8181
rails_root_index = find_rails_root_index(node)
8282
slash_node = node.children[rails_root_index + 1]
83+
return if slash_node&.source == File::SEPARATOR
8384
return unless slash_node&.str_type? && slash_node.source.start_with?(File::SEPARATOR)
8485
return unless node.children[rails_root_index].children.first.send_type?
8586

@@ -132,7 +133,10 @@ def check_for_rails_root_join_with_slash_separated_path(node)
132133
def valid_arguments_for_file_join_with_rails_root?(arguments)
133134
return false unless arguments.any? { |arg| rails_root_nodes?(arg) }
134135

135-
arguments.none? { |arg| arg.variable? || arg.const_type? || string_contains_multiple_slashes?(arg) }
136+
arguments.none? do |arg|
137+
arg.variable? || arg.const_type? || empty_string?(arg) || string_with_trailing_slash?(arg) ||
138+
string_contains_multiple_slashes?(arg) || string_contains_only_slash?(arg)
139+
end
136140
end
137141

138142
def valid_string_arguments_for_rails_root_join?(arguments)
@@ -148,10 +152,18 @@ def valid_slash_separated_path_for_rails_root_join?(arguments)
148152
arguments.none? { |arg| string_with_leading_slash?(arg) || string_contains_multiple_slashes?(arg) }
149153
end
150154

155+
def empty_string?(node)
156+
node.str_type? && node.value.empty?
157+
end
158+
151159
def string_contains_slash?(node)
152160
node.str_type? && node.value.include?(File::SEPARATOR)
153161
end
154162

163+
def string_contains_only_slash?(node)
164+
node.str_type? && node.value == File::SEPARATOR
165+
end
166+
155167
def string_contains_multiple_slashes?(node)
156168
node.str_type? && node.value.include?('//')
157169
end
@@ -160,6 +172,10 @@ def string_with_leading_slash?(node)
160172
node.str_type? && node.value.start_with?(File::SEPARATOR)
161173
end
162174

175+
def string_with_trailing_slash?(node)
176+
node.str_type? && node.value.end_with?(File::SEPARATOR)
177+
end
178+
163179
def register_offense(node, require_to_s:, &block)
164180
line_range = node.loc.column...node.loc.last_column
165181
source_range = source_range(processed_source.buffer, node.first_line, line_range)

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

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,106 @@
140140
end
141141
end
142142

143+
context 'when using File.join with Rails.root and an empty string as a path' do
144+
it 'does not register an offense' do
145+
expect_no_offenses(<<~RUBY)
146+
File.join(Rails.root, "")
147+
RUBY
148+
end
149+
end
150+
151+
context 'when using File.join with Rails.root and a single forward slash as a path' do
152+
it 'does not register an offense' do
153+
expect_no_offenses(<<~RUBY)
154+
File.join(Rails.root, "/")
155+
RUBY
156+
end
157+
end
158+
159+
context 'when using File.join with Rails.root and a string path ending in a slash' do
160+
it 'does not register an offense' do
161+
expect_no_offenses(<<~RUBY)
162+
File.join(Rails.root, 'app/')
163+
RUBY
164+
end
165+
end
166+
167+
context 'when using File.join with Rails.root and multiple string paths with different trailing slash placements' do
168+
it 'does not register an offense' do
169+
expect_no_offenses(<<~RUBY)
170+
File.join(Rails.root, 'app', 'models/')
171+
RUBY
172+
173+
expect_no_offenses(<<~RUBY)
174+
File.join(Rails.root, 'app/', 'models')
175+
RUBY
176+
177+
expect_no_offenses(<<~RUBY)
178+
File.join(Rails.root, 'app/', 'models/')
179+
RUBY
180+
181+
expect_no_offenses(<<~RUBY)
182+
File.join(Rails.root, 'app/', '/models')
183+
RUBY
184+
185+
expect_no_offenses(<<~RUBY)
186+
File.join(Rails.root, '/app', 'models/')
187+
RUBY
188+
end
189+
end
190+
191+
context 'when using File.join with Rails.root and string paths with one or more end with a trailing slash' do
192+
it 'does not register an offense' do
193+
expect_no_offenses(<<~RUBY)
194+
File.join(Rails.root, 'app/models/', 'goober')
195+
RUBY
196+
197+
expect_no_offenses(<<~RUBY)
198+
File.join(Rails.root, 'app/models/', '/goober')
199+
RUBY
200+
201+
expect_no_offenses(<<~RUBY)
202+
File.join(Rails.root, 'app/models', 'goober/')
203+
RUBY
204+
205+
expect_no_offenses(<<~RUBY)
206+
File.join(Rails.root, 'app/models/', 'goober/')
207+
RUBY
208+
end
209+
end
210+
211+
context 'when using File.join with Rails.root and an empty last argument' do
212+
it 'does not register an offense' do
213+
expect_no_offenses(<<~RUBY)
214+
File.join(Rails.root, "app", "")
215+
RUBY
216+
end
217+
end
218+
219+
context 'when using File.join with Rails.root and a single forward slash as the last argument' do
220+
it 'does not register an offense' do
221+
expect_no_offenses(<<~RUBY)
222+
File.join(Rails.root, 'app', '/')
223+
RUBY
224+
end
225+
end
226+
227+
context 'when using Rails.root.join with a directory ending with a slash' do
228+
it 'does not register an offense' do
229+
expect_no_offenses(<<~RUBY)
230+
Rails.root.join("app/").to_s
231+
RUBY
232+
end
233+
end
234+
235+
context 'when using Rails.root in string interpolation followed by a forward slash' do
236+
it 'does not register an offense' do
237+
expect_no_offenses(<<~'RUBY')
238+
"#{Rails.root}/"
239+
RUBY
240+
end
241+
end
242+
143243
context 'when using Rails.root called by double quoted string' do
144244
it 'registers an offense' do
145245
expect_offense(<<~'RUBY')
@@ -526,6 +626,111 @@
526626
end
527627
end
528628

629+
context 'when using File.join with Rails.root and an empty string as a path' do
630+
it 'does not register an offense' do
631+
expect_no_offenses(<<~RUBY)
632+
File.join(Rails.root, "")
633+
RUBY
634+
end
635+
end
636+
637+
context 'when using File.join with Rails.root and a single forward slash as a path' do
638+
it 'does not register an offense' do
639+
expect_no_offenses(<<~RUBY)
640+
File.join(Rails.root, "/")
641+
RUBY
642+
end
643+
end
644+
645+
context 'when using File.join with Rails.root and a directory path ending in a slash' do
646+
it 'does not register an offense' do
647+
expect_no_offenses(<<~RUBY)
648+
File.join(Rails.root, 'app/')
649+
RUBY
650+
end
651+
end
652+
653+
context 'when using File.join with Rails.root and multiple string paths with different trailing slash placements' do
654+
it 'does not register an offense' do
655+
expect_no_offenses(<<~RUBY)
656+
File.join(Rails.root, 'app', 'models/')
657+
RUBY
658+
659+
expect_no_offenses(<<~RUBY)
660+
File.join(Rails.root, 'app/', 'models')
661+
RUBY
662+
663+
expect_no_offenses(<<~RUBY)
664+
File.join(Rails.root, 'app/', 'models/')
665+
RUBY
666+
667+
expect_no_offenses(<<~RUBY)
668+
File.join(Rails.root, 'app/', '/models')
669+
RUBY
670+
671+
expect_no_offenses(<<~RUBY)
672+
File.join(Rails.root, '/app', 'models/')
673+
RUBY
674+
end
675+
end
676+
677+
context 'when using File.join with Rails.root and string paths with one or more end with a trailing slash' do
678+
it 'does not register an offense' do
679+
expect_no_offenses(<<~RUBY)
680+
File.join(Rails.root, 'app/models/', 'goober')
681+
RUBY
682+
683+
expect_no_offenses(<<~RUBY)
684+
File.join(Rails.root, 'app/models/', '/goober')
685+
RUBY
686+
687+
expect_no_offenses(<<~RUBY)
688+
File.join(Rails.root, 'app/models', 'goober/')
689+
RUBY
690+
691+
expect_no_offenses(<<~RUBY)
692+
File.join(Rails.root, 'app/models/', 'goober/')
693+
RUBY
694+
end
695+
end
696+
697+
context 'when using File.join with Rails.root and an empty last argument' do
698+
it 'does not register an offense' do
699+
expect_no_offenses(<<~RUBY)
700+
File.join(Rails.root, "app", "")
701+
RUBY
702+
end
703+
end
704+
705+
context 'when using File.join with Rails.root and a single forward slash as the last argument' do
706+
it 'does not register an offense' do
707+
expect_no_offenses(<<~RUBY)
708+
File.join(Rails.root, 'app', '/')
709+
RUBY
710+
end
711+
end
712+
713+
context 'when using Rails.root.join with a directory ending with a slash' do
714+
it 'registers an offense' do
715+
expect_offense(<<~RUBY)
716+
Rails.root.join("app/").to_s
717+
^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Rails.root.join('path', 'to')`.
718+
RUBY
719+
720+
expect_correction(<<~RUBY)
721+
Rails.root.join("app", "").to_s
722+
RUBY
723+
end
724+
end
725+
726+
context 'when using Rails.root in string interpolation followed by a forward slash' do
727+
it 'does not register an offense' do
728+
expect_no_offenses(<<~'RUBY')
729+
"#{Rails.root}/"
730+
RUBY
731+
end
732+
end
733+
529734
context 'when using Rails.root called by double quoted string' do
530735
it 'registers an offense' do
531736
expect_offense(<<~'RUBY')

0 commit comments

Comments
 (0)