Skip to content

Commit

Permalink
Ignore leading slashes in string literal arguments for `Rails.root.jo…
Browse files Browse the repository at this point in the history
…in` and multiple slashes in string literal arguments for `Rails.root.join` and `File.join`
  • Loading branch information
ydakuka committed Feb 4, 2025
1 parent c11c400 commit 4d84c42
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 4 deletions.
24 changes: 20 additions & 4 deletions lib/rubocop/cop/rails/file_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ module Rails
# Identifies usages of file path joining process to use `Rails.root.join` clause.
# It is used to add uniformity when joining paths.
#
# NOTE: This cop ignores leading slashes in string literal arguments for `Rails.root.join`
# and multiple slashes in string literal arguments for `Rails.root.join` and `File.join`.
#
# @example EnforcedStyle: slashes (default)
# # bad
# Rails.root.join('app', 'models', 'goober')
Expand Down Expand Up @@ -99,6 +102,7 @@ def check_for_file_join_with_rails_root(node)
return unless file_join_nodes?(node)
return unless node.arguments.any? { |e| rails_root_nodes?(e) }
return if node.arguments.any?(&:variable?)
return if node.arguments.any? { |arg| string_contains_multiple_slashes?(arg) }

register_offense(node, require_to_s: true) do |corrector|
autocorrect_file_join(corrector, node) unless node.first_argument.array_type?
Expand All @@ -111,6 +115,8 @@ def check_for_rails_root_join_with_string_arguments(node)
return unless rails_root_join_nodes?(node)
return unless node.arguments.size > 1
return unless node.arguments.all?(&:str_type?)
return if node.arguments.any? { |arg| string_with_leading_slash?(arg) }
return if node.arguments.any? { |arg| string_contains_multiple_slashes?(arg) }

register_offense(node, require_to_s: false) do |corrector|
autocorrect_rails_root_join_with_string_arguments(corrector, node)
Expand All @@ -121,15 +127,25 @@ 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? { |arg| string_contains_slash?(arg) }
return if node.arguments.any? { |arg| string_with_leading_slash?(arg) }
return if node.arguments.any? { |arg| string_contains_multiple_slashes?(arg) }

register_offense(node, require_to_s: false) do |corrector|
autocorrect_rails_root_join_with_slash_separated_path(corrector, node)
end
end

def string_with_slash?(node)
node.str_type? && node.source.include?(File::SEPARATOR)
def string_contains_slash?(node)
node.str_type? && node.value.include?(File::SEPARATOR)
end

def string_contains_multiple_slashes?(node)
node.str_type? && node.value.include?('//')
end

def string_with_leading_slash?(node)
node.str_type? && node.value.start_with?("/")
end

def register_offense(node, require_to_s:, &block)
Expand Down Expand Up @@ -227,7 +243,7 @@ def autocorrect_rails_root_join_with_string_arguments(corrector, node)

def autocorrect_rails_root_join_with_slash_separated_path(corrector, node)
node.arguments.each do |argument|
next unless string_with_slash?(argument)
next unless string_contains_slash?(argument)

index = argument.source.index(File::SEPARATOR)
rest = inner_range_of(argument).adjust(begin_pos: index - 1)
Expand Down
80 changes: 80 additions & 0 deletions spec/rubocop/cop/rails/file_path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,46 @@
RUBY
end
end

context 'when using Rails.root.join with a leading slash' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('/app/models')
RUBY
end
end

context 'when using Rails.root.join with mixed leading and normal path strings' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('/app', 'models')
RUBY
end
end

context 'when using Rails.root.join with mixed normal and leading path strings' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('app', '/models')
RUBY
end
end

context 'when using Rails.root.join with multiple slashes in a path' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('public//', 'assets')
RUBY
end
end

context 'when using File.join with multiple slashes in a path' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
File.join(Rails.root, 'public//', 'assets')
RUBY
end
end
end

context 'when EnforcedStyle is `arguments`' do
Expand Down Expand Up @@ -658,5 +698,45 @@
RUBY
end
end

context 'when using Rails.root.join with a leading slash' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('/app/models')
RUBY
end
end

context 'when using Rails.root.join with mixed leading and normal path strings' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('/app', 'models')
RUBY
end
end

context 'when using Rails.root.join with mixed normal and leading path strings' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('app', '/models')
RUBY
end
end

context 'when using Rails.root.join with multiple slashes in a path' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Rails.root.join('public//', 'assets')
RUBY
end
end

context 'when using File.join with multiple slashes in a path' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
File.join(Rails.root, 'public//', 'assets')
RUBY
end
end
end
end

0 comments on commit 4d84c42

Please sign in to comment.