Skip to content

Commit

Permalink
[Fix rubocop#1053] Fix an error occurred for Rails/FilePath cop when …
Browse files Browse the repository at this point in the history
…nested File.join
  • Loading branch information
ydakuka committed Jan 26, 2025
1 parent c95d720 commit dee1b76
Show file tree
Hide file tree
Showing 4 changed files with 323 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ InternalAffairs/OnSendWithoutOnCSend:
# Offense count: 10
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 179
Max: 163

# Offense count: 41
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1053](https://github.com/rubocop/rubocop-rails/issues/1053): Fix an error occurred for `Rails/FilePath` cop when nested `File.join`. ([@ydakuka][])
92 changes: 87 additions & 5 deletions lib/rubocop/cop/rails/file_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ module Rails
# # good
# Rails.root.join('app', 'models', 'goober').to_s
#
class FilePath < Base
class FilePath < Base # rubocop:disable Metrics/ClassLength
extend AutoCorrector

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

def_node_matcher :file_join_with_rails_root_nodes?, <<~PATTERN
(send (const {nil? cbase} :File) :join (_ $_)* {(send #rails_root_nodes? :to_s) #rails_root_nodes?} ...)
PATTERN

def_node_matcher :file_join_with_rails_root_join_nodes?, <<~PATTERN
(send (const {nil? cbase} :File) :join (_ $_)* {(send #rails_root_join_nodes? :to_s) #rails_root_join_nodes?} ...)
PATTERN

def_node_search :rails_root_nodes?, <<~PATTERN
(send (const {nil? cbase} :Rails) :root)
PATTERN
Expand All @@ -65,8 +73,11 @@ def on_dstr(node)
end

def on_send(node)
check_for_file_join_with_rails_root_join(node)
check_for_file_join_with_rails_root(node)

return unless node.receiver
return if node.ancestors.any? { |ancestor| file_join_nodes?(ancestor) }

check_for_rails_root_join_with_slash_separated_path(node)
check_for_rails_root_join_with_string_arguments(node)
Expand Down Expand Up @@ -96,11 +107,21 @@ def check_for_extension_after_rails_root_join_in_dstr(node)
end

def check_for_file_join_with_rails_root(node)
return unless file_join_nodes?(node)
return unless file_join_with_rails_root_nodes?(node)
return unless node.arguments.any? { |e| rails_root_nodes?(e) }

return if node.descendants.any? { |descendant| file_join_nodes?(descendant) }

register_offense(node, require_to_s: true) do |corrector|
autocorrect_file_join_with_rails_root(corrector, node) unless node.first_argument.array_type?
end
end

def check_for_file_join_with_rails_root_join(node)
return unless file_join_with_rails_root_join_nodes?(node)

register_offense(node, require_to_s: true) do |corrector|
autocorrect_file_join(corrector, node) unless node.first_argument.array_type?
autocorrect_file_join_with_rails_root_join(corrector, node) unless node.first_argument.array_type?
end
end

Expand All @@ -120,7 +141,7 @@ def check_for_rails_root_join_with_slash_separated_path(node)
return unless style == :arguments
return unless rails_root_nodes?(node)
return unless rails_root_join_nodes?(node)
return unless node.arguments.any? { |arg| string_with_slash?(arg) }
return unless node.arguments.any? { |argument| string_with_slash?(argument) }

register_offense(node, require_to_s: false) do |corrector|
autocorrect_rails_root_join_with_slash_separated_path(corrector, node)
Expand Down Expand Up @@ -172,7 +193,7 @@ def autocorrect_extension_after_rails_root_join_in_dstr(corrector, node, rails_r
corrector.remove(extension_node)
end

def autocorrect_file_join(corrector, node)
def autocorrect_file_join_with_rails_root(corrector, node)
replace_receiver_with_rails_root(corrector, node)
remove_first_argument_with_comma(corrector, node)
process_arguments(corrector, node.arguments)
Expand Down Expand Up @@ -209,6 +230,67 @@ def append_to_string_conversion(corrector, node)
corrector.insert_after(node, '.to_s')
end

def autocorrect_file_join_with_rails_root_join(corrector, node)
rails_root_join_node = find_rails_root_join_node(node)
return unless rails_root_join_node

combined_arguments = combine_arguments(rails_root_join_node, node)
corrector.replace(node, "Rails.root.join(#{combined_arguments}).to_s")
end

def find_rails_root_join_node(node)
node.arguments.find { |argument| rails_root_join_typical?(argument) }
end

def rails_root_join_typical?(node)
rails_root_join_nodes?(node) || (node.send_type? && rails_root_join_nodes?(node.receiver))
end

def extract_rails_root_call(rails_root_join_node)
if rails_root_join_node.send_type? && rails_root_join_node.method?(:to_s)
rails_root_join_node.receiver
else
rails_root_join_node
end
end

def combine_arguments(rails_root_join_node, node)
rails_root_call = extract_rails_root_call(rails_root_join_node)
rails_root_arguments = rails_root_call.arguments
other_arguments = node.arguments.reject { |argument| argument == rails_root_join_node }

case style
when :arguments
combine_as_arguments(rails_root_arguments, other_arguments)
when :slashes
combine_as_slashes(rails_root_arguments, other_arguments)
end
end

def combine_as_arguments(rails_root_arguments, other_arguments)
first_argument = other_arguments.first

other_arguments_values =
if first_argument.dstr_type?
first_argument.children.map do |child|
"'#{child.value.delete_prefix('/')}'"
end
else
["'#{first_argument.value.delete_prefix('/')}'"]
end

(rails_root_arguments.map(&:source) + other_arguments_values).join(', ')
end

def combine_as_slashes(rails_root_arguments, other_arguments)
return "'#{rails_root_arguments.map(&:value).join}'" if other_arguments.empty?

first_value = other_arguments.first.value
separator = first_value.start_with?(File::SEPARATOR) ? '' : '/'

"'#{(rails_root_arguments + other_arguments).map(&:value).join(separator)}'"
end

def autocorrect_rails_root_join_with_string_arguments(corrector, node)
corrector.replace(node.first_argument, %("#{node.arguments.map(&:value).join('/')}"))
node.arguments[1..].each do |argument|
Expand Down
Loading

0 comments on commit dee1b76

Please sign in to comment.