rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
802 stars 257 forks source link

Confusing message for `Rails/RootPathnameMethods` in `Dir[]` #1345

Closed Tietew closed 1 week ago

Tietew commented 2 weeks ago

RuboCop suggests confusing solution on following code:

files = Dir[Rails.root.join('path/*')]

Expected behavior

RuboCop suggests to rewrite Dir[Rails.root.join(...)] to Rails.root.glob(...) as autocorrection does.

Actual behavior

RuboCop suggests to append #[].

files = Dir[Rails.root.join('path/*')]
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
C: [Correctable] Rails/RootPathnameMethods: Rails.root is a Pathname so you can just append #[].

But appending #[] is an incorrect solution.

Note

Autocorrection works correctly. The issue is only in the message.

# bad
files = Dir[Rails.root.join('path/*')]
# autocorrect
files = Rails.root.glob('path/*')

RuboCop version

$ be rubocop -V
1.65.1 (using Parser 3.3.4.2, rubocop-ast 1.32.1, running on ruby 3.3.4) [x86_64-linux]
  - rubocop-capybara 2.21.0
  - rubocop-factory_bot 2.26.1
  - rubocop-performance 1.21.1
  - rubocop-rails 2.26.0
  - rubocop-rake 0.6.0
  - rubocop-rspec 3.0.4
  - rubocop-rspec_rails 2.30.0
Earlopain commented 2 weeks ago

Do you think the message

Rails.root is a Pathname so you can just use Rails.root.glob

would be better? Or do you have something else in mind?

vinbarnes commented 2 weeks ago

We also were scratching our heads for a few minutes on this one, then figured it out. Not sure the clearest way to message the user though.

taylorthurlow commented 2 weeks ago

Also chiming in to say that I could not figure this one out until I found this issue. I think including a specific example with the suggested alternative to Dir[] would go a long way.

Tietew commented 2 weeks ago

@Earlopain Thanks for suggestion! Looks good tome.