Skip to content

Commit 5d24672

Browse files
ydakukaydakuka
committedFeb 15, 2025
[Fix rubocop#1177] Fix for Rails/FilePath autocorrect removing an important trailing slash in a dynamic string containing Rails.root 
1 parent 4d0e655 commit 5d24672

File tree

3 files changed

+252
-3
lines changed

3 files changed

+252
-3
lines changed
 

‎changelog/fix_rails_file_path_to_properly_handle_trailing_slashes.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1177](https://github.com/rubocop/rubocop-rails/issues/1177): Enhance `Rails/FilePath` to properly handle trailing slashes. ([@ydakuka][])

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

+17-3
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,20 +133,29 @@ 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? ||
138+
string_contains_multiple_slashes?(arg) || string_with_trailing_slash?(arg)
139+
end
136140
end
137141

138142
def valid_string_arguments_for_rails_root_join?(arguments)
139143
return false unless arguments.size > 1
140144
return false unless arguments.all?(&:str_type?)
141145

142-
arguments.none? { |arg| string_with_leading_slash?(arg) || string_contains_multiple_slashes?(arg) }
146+
arguments.none? do |arg|
147+
string_with_leading_slash?(arg) ||
148+
string_contains_multiple_slashes?(arg) || string_with_trailing_slash?(arg)
149+
end
143150
end
144151

145152
def valid_slash_separated_path_for_rails_root_join?(arguments)
146153
return false unless arguments.any? { |arg| string_contains_slash?(arg) }
147154

148-
arguments.none? { |arg| string_with_leading_slash?(arg) || string_contains_multiple_slashes?(arg) }
155+
arguments.none? do |arg|
156+
string_with_leading_slash?(arg) ||
157+
string_contains_multiple_slashes?(arg) || string_with_trailing_slash?(arg)
158+
end
149159
end
150160

151161
def string_contains_slash?(node)
@@ -160,6 +170,10 @@ def string_with_leading_slash?(node)
160170
node.str_type? && node.value.start_with?(File::SEPARATOR)
161171
end
162172

173+
def string_with_trailing_slash?(node)
174+
node.str_type? && node.value.end_with?(File::SEPARATOR)
175+
end
176+
163177
def register_offense(node, require_to_s:, &block)
164178
line_range = node.loc.column...node.loc.last_column
165179
source_range = source_range(processed_source.buffer, node.first_line, line_range)

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

+234
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,123 @@
140140
end
141141
end
142142

143+
context 'when using File.join with Rails.root and a single forward slash 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 the last argument' do
152+
it 'does not register an offense' do
153+
expect_no_offenses(<<~RUBY)
154+
File.join(Rails.root, 'app', '/')
155+
RUBY
156+
end
157+
end
158+
159+
context 'when using File.join with Rails.root and multiple string paths with different trailing slash placements' do
160+
it 'does not register an offense' do
161+
expect_no_offenses(<<~RUBY)
162+
File.join(Rails.root, 'app/models/', 'goober')
163+
RUBY
164+
165+
expect_no_offenses(<<~RUBY)
166+
File.join(Rails.root, 'app/models/', '/goober')
167+
RUBY
168+
169+
expect_no_offenses(<<~RUBY)
170+
File.join(Rails.root, 'app/models', 'goober/')
171+
RUBY
172+
173+
expect_no_offenses(<<~RUBY)
174+
File.join(Rails.root, 'app/models/', 'goober/')
175+
RUBY
176+
end
177+
end
178+
179+
context 'when using Rails.root.join and a single forward slash as a path' do
180+
it 'does not register an offense' do
181+
expect_no_offenses(<<~RUBY)
182+
Rails.root.join("/")
183+
RUBY
184+
end
185+
end
186+
187+
context 'when using Rails.root.join and a single forward slash as the last argument' do
188+
it 'does not register an offense' do
189+
expect_no_offenses(<<~RUBY)
190+
Rails.root.join('app', '/')
191+
RUBY
192+
end
193+
end
194+
195+
context 'when using Rails.root.join and multiple string paths with different trailing slash placements' do
196+
it 'does not register an offense' do
197+
expect_no_offenses(<<~RUBY)
198+
Rails.root.join('app/models/', 'goober')
199+
RUBY
200+
201+
expect_no_offenses(<<~RUBY)
202+
Rails.root.join('app/models/', '/goober')
203+
RUBY
204+
205+
expect_no_offenses(<<~RUBY)
206+
Rails.root.join('app/models', 'goober/')
207+
RUBY
208+
209+
expect_no_offenses(<<~RUBY)
210+
Rails.root.join('app/models/', 'goober/')
211+
RUBY
212+
end
213+
end
214+
215+
context 'when using Rails.root.join with Rails.root and a single forward slash as a path' do
216+
it 'does not register an offense' do
217+
expect_no_offenses(<<~RUBY)
218+
Rails.root.join(Rails.root, "/")
219+
RUBY
220+
end
221+
end
222+
223+
context 'when using Rails.root.join with Rails.root and a single forward slash as the last argument' do
224+
it 'does not register an offense' do
225+
expect_no_offenses(<<~RUBY)
226+
Rails.root.join(Rails.root, 'app', '/')
227+
RUBY
228+
end
229+
end
230+
231+
context 'when using Rails.root.join with Rails.root and multiple string paths ' \
232+
'with different trailing slash placements' do
233+
it 'does not register an offense' do
234+
expect_no_offenses(<<~RUBY)
235+
Rails.root.join(Rails.root, 'app/models/', 'goober')
236+
RUBY
237+
238+
expect_no_offenses(<<~RUBY)
239+
Rails.root.join(Rails.root, 'app/models/', '/goober')
240+
RUBY
241+
242+
expect_no_offenses(<<~RUBY)
243+
Rails.root.join(Rails.root, 'app/models', 'goober/')
244+
RUBY
245+
246+
expect_no_offenses(<<~RUBY)
247+
Rails.root.join(Rails.root, 'app/models/', 'goober/')
248+
RUBY
249+
end
250+
end
251+
252+
context 'when using Rails.root in string interpolation followed by a forward slash' do
253+
it 'does not register an offense' do
254+
expect_no_offenses(<<~'RUBY')
255+
"#{Rails.root}/"
256+
RUBY
257+
end
258+
end
259+
143260
context 'when using Rails.root called by double quoted string' do
144261
it 'registers an offense' do
145262
expect_offense(<<~'RUBY')
@@ -526,6 +643,123 @@
526643
end
527644
end
528645

646+
context 'when using File.join with Rails.root and a single forward slash as a path' do
647+
it 'does not register an offense' do
648+
expect_no_offenses(<<~RUBY)
649+
File.join(Rails.root, "/")
650+
RUBY
651+
end
652+
end
653+
654+
context 'when using File.join with Rails.root and a single forward slash as the last argument' do
655+
it 'does not register an offense' do
656+
expect_no_offenses(<<~RUBY)
657+
File.join(Rails.root, 'app', '/')
658+
RUBY
659+
end
660+
end
661+
662+
context 'when using File.join with Rails.root and multiple string paths with different trailing slash placements' do
663+
it 'does not register an offense' do
664+
expect_no_offenses(<<~RUBY)
665+
File.join(Rails.root, 'app/models/', 'goober')
666+
RUBY
667+
668+
expect_no_offenses(<<~RUBY)
669+
File.join(Rails.root, 'app/models/', '/goober')
670+
RUBY
671+
672+
expect_no_offenses(<<~RUBY)
673+
File.join(Rails.root, 'app/models', 'goober/')
674+
RUBY
675+
676+
expect_no_offenses(<<~RUBY)
677+
File.join(Rails.root, 'app/models/', 'goober/')
678+
RUBY
679+
end
680+
end
681+
682+
context 'when using Rails.root.join and a single forward slash as a path' do
683+
it 'does not register an offense' do
684+
expect_no_offenses(<<~RUBY)
685+
Rails.root.join("/")
686+
RUBY
687+
end
688+
end
689+
690+
context 'when using Rails.root.join and a single forward slash as the last argument' do
691+
it 'does not register an offense' do
692+
expect_no_offenses(<<~RUBY)
693+
Rails.root.join('app', '/')
694+
RUBY
695+
end
696+
end
697+
698+
context 'when using Rails.root.join and multiple string paths with different trailing slash placements' do
699+
it 'does not register an offense' do
700+
expect_no_offenses(<<~RUBY)
701+
Rails.root.join('app/models/', 'goober')
702+
RUBY
703+
704+
expect_no_offenses(<<~RUBY)
705+
Rails.root.join('app/models/', '/goober')
706+
RUBY
707+
708+
expect_no_offenses(<<~RUBY)
709+
Rails.root.join('app/models', 'goober/')
710+
RUBY
711+
712+
expect_no_offenses(<<~RUBY)
713+
Rails.root.join('app/models/', 'goober/')
714+
RUBY
715+
end
716+
end
717+
718+
context 'when using Rails.root.join with Rails.root and a single forward slash as a path' do
719+
it 'does not register an offense' do
720+
expect_no_offenses(<<~RUBY)
721+
Rails.root.join(Rails.root, "/")
722+
RUBY
723+
end
724+
end
725+
726+
context 'when using Rails.root.join with Rails.root and a single forward slash as the last argument' do
727+
it 'does not register an offense' do
728+
expect_no_offenses(<<~RUBY)
729+
Rails.root.join(Rails.root, 'app', '/')
730+
RUBY
731+
end
732+
end
733+
734+
context 'when using Rails.root.join with Rails.root and multiple string paths ' \
735+
'with different trailing slash placements' do
736+
it 'does not register an offense' do
737+
expect_no_offenses(<<~RUBY)
738+
Rails.root.join(Rails.root, 'app/models/', 'goober')
739+
RUBY
740+
741+
expect_no_offenses(<<~RUBY)
742+
Rails.root.join(Rails.root, 'app/models/', '/goober')
743+
RUBY
744+
745+
expect_no_offenses(<<~RUBY)
746+
Rails.root.join(Rails.root, 'app/models', 'goober/')
747+
RUBY
748+
749+
expect_no_offenses(<<~RUBY)
750+
Rails.root.join(Rails.root, 'app/models/', 'goober/')
751+
RUBY
752+
end
753+
end
754+
755+
context 'when using Rails.root in string interpolation followed by a forward slash' do
756+
it 'does not register an offense' do
757+
expect_no_offenses(<<~'RUBY')
758+
"#{Rails.root}/"
759+
RUBY
760+
end
761+
end
762+
529763
context 'when using Rails.root called by double quoted string' do
530764
it 'registers an offense' do
531765
expect_offense(<<~'RUBY')

0 commit comments

Comments
 (0)