Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails/FilePath autocorrect shouldn't remove important trailing slash in dynamic string containing Rails.root #1177

Open
henrahmagix opened this issue Nov 2, 2023 · 1 comment

Comments

@henrahmagix
Copy link

I'm trying to use Rails.root to remove it from an absolute filepath to result in a filepath relative to Rails.root.

my_abs_path              = "/Users/me/rails_app/config/locales/text_en.yml"
expected_relative_result = "config/locales/text_en.yml"

my_abs_path.delete_prefix("#{Rails.root}/") == expected_relative_result
# => true

Expected behavior

# this should be allowed by Rails/FilePath cop because the last / in /Users/me/rails_app/ must be deleted, not just /Users/me/rails_app
my_abs_path.delete_prefix("#{Rails.root}/")

Actual behavior

my_abs_path.delete_prefix("#{Rails.root}/")
# raises Rails/FilePath, and is autocorrected to
my_abs_path.delete_prefix("#{Rails.root.join('')}")
# which raises Style/RedundantInterpolation, so we manually correct to
my_abs_path.delete_prefix(Rails.root.join('').to_s)
# which is wrong:
actual = my_abs_path.delete_prefix(Rails.root.join('').to_s)
actual == expected_relative_result
# => false
puts actual
# => "/config/locales/text_en.yml"

# so we try giving an argument to join, but still wrong because Rails.root.join never adds a trailing slash:
actual = my_abs_path.delete_prefix(Rails.root.join('./').to_s)
actual == expected_relative_result
# => false
puts actual
# => "/config/locales/text_en.yml"

Steps to reproduce the problem

Copy the below into a file, run RuboCop with autocorrect enabled and manually fix any other cops that are raised, then assert that the result is true.
rubocop-rails-filepath-test.rb

# where Rails.root is /Users/me/rails_app
my_abs_path = "/Users/me/rails_app/config/locales/text_en.yml"
expected_relative_result = "config/locales/text_en.yml"

actual = my_abs_path.delete_prefix("#{Rails.root}/")
if actual != expected_relative_result
  raise "Invalid autocorrection: #{actual.inspect} != #{expected_relative_result.inspect}"
end
rubocop -a rubocop-rails-filepath-test.rb
rubocop -A rubocop-rails-filepath-test.rb # autofix Style/RedundantInterpolation
bin/rails c < rubocop-rails-filepath-test.rb

RuboCop version

1.57.2 (using Parser 3.2.2.4, rubocop-ast 1.30.0, running on ruby 3.0.6) [arm64-darwin21]
  - rubocop-rails 2.22.1
  - rubocop-rspec 2.18.1
@henrahmagix
Copy link
Author

I can see autocorrection was added in #991 so is this considered a bug of that? If so, then I can submit a fix that allows "#{Rails.root}/" if that's acceptable?

I also saw #1071 which might get fixed at the same time; I can check.

Also theres #434 which includes the exact same case as this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant