Skip to content

Commit f028c15

Browse files
committed
[Fix #1053] Fix an error occurred for Rails/FilePath cop when nested File.join
1 parent c95d720 commit f028c15

File tree

4 files changed

+323
-6
lines changed

4 files changed

+323
-6
lines changed

.rubocop_todo.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ InternalAffairs/OnSendWithoutOnCSend:
2929
# Offense count: 10
3030
# Configuration parameters: CountComments, CountAsOne.
3131
Metrics/ClassLength:
32-
Max: 179
32+
Max: 163
3333

3434
# Offense count: 41
3535
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1053](https://github.com/rubocop/rubocop-rails/issues/1053): Fix an error occurred for `Rails/FilePath` cop when nested `File.join`. ([@ydakuka][])

lib/rubocop/cop/rails/file_path.rb

Lines changed: 87 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ module Rails
3434
# # good
3535
# Rails.root.join('app', 'models', 'goober').to_s
3636
#
37-
class FilePath < Base
37+
class FilePath < Base # rubocop:disable Metrics/ClassLength
3838
extend AutoCorrector
3939

4040
include ConfigurableEnforcedStyle
@@ -48,6 +48,14 @@ class FilePath < Base
4848
(send (const {nil? cbase} :File) :join ...)
4949
PATTERN
5050

51+
def_node_matcher :file_join_with_rails_root_nodes?, <<~PATTERN
52+
(send (const {nil? cbase} :File) :join (_ $_)* {(send #rails_root_nodes? :to_s) #rails_root_nodes?} ...)
53+
PATTERN
54+
55+
def_node_matcher :file_join_with_rails_root_join_nodes?, <<~PATTERN
56+
(send (const {nil? cbase} :File) :join (_ $_)* {(send #rails_root_join_nodes? :to_s) #rails_root_join_nodes?} ...)
57+
PATTERN
58+
5159
def_node_search :rails_root_nodes?, <<~PATTERN
5260
(send (const {nil? cbase} :Rails) :root)
5361
PATTERN
@@ -65,8 +73,11 @@ def on_dstr(node)
6573
end
6674

6775
def on_send(node)
76+
check_for_file_join_with_rails_root_join(node)
6877
check_for_file_join_with_rails_root(node)
78+
6979
return unless node.receiver
80+
return if node.ancestors.any? { |ancestor| file_join_nodes?(ancestor) }
7081

7182
check_for_rails_root_join_with_slash_separated_path(node)
7283
check_for_rails_root_join_with_string_arguments(node)
@@ -96,11 +107,21 @@ def check_for_extension_after_rails_root_join_in_dstr(node)
96107
end
97108

98109
def check_for_file_join_with_rails_root(node)
99-
return unless file_join_nodes?(node)
110+
return unless file_join_with_rails_root_nodes?(node)
100111
return unless node.arguments.any? { |e| rails_root_nodes?(e) }
101112

113+
return if node.descendants.any? { |descendant| file_join_nodes?(descendant) }
114+
115+
register_offense(node, require_to_s: true) do |corrector|
116+
autocorrect_file_join_with_rails_root(corrector, node) unless node.first_argument.array_type?
117+
end
118+
end
119+
120+
def check_for_file_join_with_rails_root_join(node)
121+
return unless file_join_with_rails_root_join_nodes?(node)
122+
102123
register_offense(node, require_to_s: true) do |corrector|
103-
autocorrect_file_join(corrector, node) unless node.first_argument.array_type?
124+
autocorrect_file_join_with_rails_root_join(corrector, node) unless node.first_argument.array_type?
104125
end
105126
end
106127

@@ -120,7 +141,7 @@ def check_for_rails_root_join_with_slash_separated_path(node)
120141
return unless style == :arguments
121142
return unless rails_root_nodes?(node)
122143
return unless rails_root_join_nodes?(node)
123-
return unless node.arguments.any? { |arg| string_with_slash?(arg) }
144+
return unless node.arguments.any? { |argument| string_with_slash?(argument) }
124145

125146
register_offense(node, require_to_s: false) do |corrector|
126147
autocorrect_rails_root_join_with_slash_separated_path(corrector, node)
@@ -172,7 +193,7 @@ def autocorrect_extension_after_rails_root_join_in_dstr(corrector, node, rails_r
172193
corrector.remove(extension_node)
173194
end
174195

175-
def autocorrect_file_join(corrector, node)
196+
def autocorrect_file_join_with_rails_root(corrector, node)
176197
replace_receiver_with_rails_root(corrector, node)
177198
remove_first_argument_with_comma(corrector, node)
178199
process_arguments(corrector, node.arguments)
@@ -209,6 +230,67 @@ def append_to_string_conversion(corrector, node)
209230
corrector.insert_after(node, '.to_s')
210231
end
211232

233+
def autocorrect_file_join_with_rails_root_join(corrector, node)
234+
rails_root_join_node = find_rails_root_join_node(node)
235+
return unless rails_root_join_node
236+
237+
combined_arguments = combine_arguments(rails_root_join_node, node)
238+
corrector.replace(node, "Rails.root.join(#{combined_arguments}).to_s")
239+
end
240+
241+
def find_rails_root_join_node(node)
242+
node.arguments.find { |argument| rails_root_join_typical?(argument) }
243+
end
244+
245+
def rails_root_join_typical?(node)
246+
rails_root_join_nodes?(node) || (node.send_type? && rails_root_join_nodes?(node.receiver))
247+
end
248+
249+
def extract_rails_root_call(rails_root_join_node)
250+
if rails_root_join_node.send_type? && rails_root_join_node.method?(:to_s)
251+
rails_root_join_node.receiver
252+
else
253+
rails_root_join_node
254+
end
255+
end
256+
257+
def combine_arguments(rails_root_join_node, node)
258+
rails_root_call = extract_rails_root_call(rails_root_join_node)
259+
rails_root_arguments = rails_root_call.arguments
260+
other_arguments = node.arguments.reject { |argument| argument == rails_root_join_node }
261+
262+
case style
263+
when :arguments
264+
combine_as_arguments(rails_root_arguments, other_arguments)
265+
when :slashes
266+
combine_as_slashes(rails_root_arguments, other_arguments)
267+
end
268+
end
269+
270+
def combine_as_arguments(rails_root_arguments, other_arguments)
271+
first_argument = other_arguments.first
272+
273+
other_arguments_values =
274+
if first_argument.dstr_type?
275+
first_argument.children.map do |child|
276+
"'#{child.value.delete_prefix('/')}'"
277+
end
278+
else
279+
["'#{first_argument.value.delete_prefix('/')}'"]
280+
end
281+
282+
(rails_root_arguments.map(&:source) + other_arguments_values).join(', ')
283+
end
284+
285+
def combine_as_slashes(rails_root_arguments, other_arguments)
286+
return "'#{rails_root_arguments.map(&:value).join}'" if other_arguments.empty?
287+
288+
first_value = other_arguments.first.value
289+
separator = first_value.start_with?(File::SEPARATOR) ? '' : '/'
290+
291+
"'#{(rails_root_arguments + other_arguments).map(&:value).join(separator)}'"
292+
end
293+
212294
def autocorrect_rails_root_join_with_string_arguments(corrector, node)
213295
corrector.replace(node.first_argument, %("#{node.arguments.map(&:value).join('/')}"))
214296
node.arguments[1..].each do |argument|

0 commit comments

Comments
 (0)