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

Wouldn't it be preferable to use slashes in Rails.root.join? #2641

Closed
castor4bit opened this issue Dec 2, 2022 · 2 comments · Fixed by #2642
Closed

Wouldn't it be preferable to use slashes in Rails.root.join? #2641

castor4bit opened this issue Dec 2, 2022 · 2 comments · Fixed by #2642

Comments

@castor4bit
Copy link
Contributor

In Pull Request #2640, it was commented to use multiple strings instead of slashes for Rails.root.join, but rubocop-rails defaults to using slashes.

This may be in consideration of platforms that do not use /, such as Windows, but since Rails.root.join is joined with File::SEPARATOR (/), both writing styles return the same result separated by slashes.

The Windows path separator can also be interpreted, so it will work as intended but will not be significant. Wouldn't it be preferable to adopt the rubocop-rails default and write Rails.root.join('foo/bar/baz') for the same behavior? (Otherwise rubocop will still warn)

Examples

Environment

  • rails (7.0.4)
  • rspec-core (3.12.0)
  • rspec-rails (6.0.1)
  • rubocop-rspec (2.15.0)
  • rubocop-rails (2.17.3)

Behavior of Rails.root.join under Windows

C:\Users\test\sample>systeminfo
...
OS Name:                   Microsoft Windows 10 Pro
OS Version:                10.0.19044 N/A Build 19044
...

C:\Users\test\sample>ruby -v
ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x64-mingw-ucrt]

C:\Users\test\sample>rails c
Loading development environment (Rails 7.0.4)

irb(main):001:0> p Rails.root
#<Pathname:C:/Users/test/sample>
=> #<Pathname:C:/Users/test/sample>
irb(main):002:0> p Rails.root.join('foo/bar')
#<Pathname:C:/Users/test/sample/foo/bar>
=> #<Pathname:C:/Users/test/sample/foo/bar>
irb(main):003:0> p Rails.root.join('foo', 'bar')
#<Pathname:C:/Users/test/sample/foo/bar>
=> #<Pathname:C:/Users/test/sample/foo/bar>

Windows path separators can be mixed (and still work fine)

irb(main):004:0> p Pathname.new(ENV['TMP']).join('foo', 'bar')
#<Pathname:C:\Users\test\AppData\Local\Temp/foo/bar>
=> #<Pathname:C:\Users\test\AppData\Local\Temp/foo/bar>

Result for rubocop

$ bundle exec rubocop spec/rails_helper.rb
Inspecting 1 file
C

Offenses:

spec/rails_helper.rb:36:25: C: Rails/FilePath: Prefer Rails.root.join('path/to').
  config.fixture_path = Rails.root.join('spec', 'fixtures')
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
@pirj
Copy link
Member

pirj commented Dec 3, 2022

This explanation makes sense.
Would you like to send a PR?

@castor4bit
Copy link
Contributor Author

Sure. I created #2642, thanks!

@pirj pirj closed this as completed in #2642 Dec 4, 2022
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

Successfully merging a pull request may close this issue.

2 participants