Skip to content

Commit

Permalink
Add new Rails/RootPathnameMethods cop
Browse files Browse the repository at this point in the history
`Rails.root` is an instance of `Pathname`. So instead of

```ruby
File.open(Rails.root.join('db', 'schema.rb'))
File.open(Rails.root.join('db', 'schema.rb'), 'w')
File.read(Rails.root.join('db', 'schema.rb'))
File.binread(Rails.root.join('db', 'schema.rb'))
File.write(Rails.root.join('db', 'schema.rb'), content)
File.binwrite(Rails.root.join('db', 'schema.rb'), content)
```

we can simply write

```ruby
Rails.root.join('db', 'schema.rb').open
Rails.root.join('db', 'schema.rb').open('w')
Rails.root.join('db', 'schema.rb').read
Rails.root.join('db', 'schema.rb').binread
Rails.root.join('db', 'schema.rb').write(content)
Rails.root.join('db', 'schema.rb').binwrite(content)
```

This cop works best when used together with
[`Style/FileRead`](rubocop/rubocop#10261),
[`Style/FileWrite`](rubocop/rubocop#10260),
and [`Rails/RootJoinChain`](rubocop#586).
  • Loading branch information
leoarnold committed Jul 16, 2022
1 parent c1d5cd7 commit aedbf22
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_railsrootpathnamemethods_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#587](https://github.com/rubocop/rubocop-rails/pull/587): Add new `Rails/RootPathnameMethods` cop. ([@leoarnold][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,11 @@ Rails/RootJoinChain:
Enabled: pending
VersionAdded: '2.13'

Rails/RootPathnameMethods:
Description: 'Use `Rails.root` IO methods instead of passing it to `File`.'
Enabled: pending
VersionAdded: '<<next>>'

Rails/RootPublicPath:
Description: "Favor `Rails.public_path` over `Rails.root` with `'public'`."
Enabled: pending
Expand Down
101 changes: 101 additions & 0 deletions lib/rubocop/cop/rails/root_pathname_methods.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Use `Rails.root` IO methods instead of passing it to `File`.
#
# `Rails.root` is an instance of `Pathname`
# so we can apply many IO methods directly.
#
# This cop works best when used together with
# `Style/FileRead`, `Style/FileWrite` and `Rails/RootJoinChain`.
#
# @example
# # bad
# File.open(Rails.root.join('db', 'schema.rb'))
# File.open(Rails.root.join('db', 'schema.rb'), 'w')
# File.read(Rails.root.join('db', 'schema.rb'))
# File.binread(Rails.root.join('db', 'schema.rb'))
# File.write(Rails.root.join('db', 'schema.rb'), content)
# File.binwrite(Rails.root.join('db', 'schema.rb'), content)
#
# # good
# Rails.root.join('db', 'schema.rb').open
# Rails.root.join('db', 'schema.rb').open('w')
# Rails.root.join('db', 'schema.rb').read
# Rails.root.join('db', 'schema.rb').binread
# Rails.root.join('db', 'schema.rb').write(content)
# Rails.root.join('db', 'schema.rb').binwrite(content)
#
class RootPathnameMethods < Base
extend AutoCorrector

MSG = '`Rails.root` is a `Pathname` so you can just append `#%<method>s`.'

RESTRICT_ON_SEND = %i[binread binwrite open read write].to_set.freeze

def_node_matcher :file_open, <<~PATTERN
(send (const {nil? cbase} :File) :open $_ $...)
PATTERN

def_node_matcher :file_read, <<~PATTERN
(send (const {nil? cbase} :File) ${:binread :read} $_ $...)
PATTERN

def_node_matcher :file_write, <<~PATTERN
(send (const {nil? cbase} :File) ${:binwrite :write} $_ $...)
PATTERN

def_node_matcher :rails_root_pathname?, <<~PATTERN
{
#rails_root?
(send #rails_root? :join ...)
}
PATTERN

def_node_matcher :rails_root?, <<~PATTERN
(send (const {nil? cbase} :Rails) {:root :public_path})
PATTERN

def on_send(node)
investigate_file_open(node) || investigate_file_read(node) || investigate_file_write(node)
end

private

def investigate_file_open(node)
return if node.parent&.send_type?
return unless (rails_root, args = file_open(node)) && rails_root_pathname?(rails_root)

add_offense(node, message: format(MSG, method: :open)) do |corrector|
replacement = "#{rails_root.source}.open"
replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?

corrector.replace(node, replacement)
end
end

def investigate_file_read(node)
return unless (method, rails_root, args = file_read(node)) && rails_root_pathname?(rails_root)

add_offense(node, message: format(MSG, method: method)) do |corrector|
replacement = "#{rails_root.source}.#{method}"
replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?

corrector.replace(node, replacement)
end
end

def investigate_file_write(node)
return unless (method, rails_root, args = file_write(node)) && rails_root_pathname?(rails_root)

add_offense(node, message: format(MSG, method: method)) do |corrector|
replacement = "#{rails_root.source}.#{method}(#{args.map(&:source).join(', ')})"
corrector.replace(node, replacement)
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
require_relative 'rails/reversible_migration'
require_relative 'rails/reversible_migration_method_definition'
require_relative 'rails/root_join_chain'
require_relative 'rails/root_pathname_methods'
require_relative 'rails/root_public_path'
require_relative 'rails/safe_navigation'
require_relative 'rails/safe_navigation_with_blank'
Expand Down
137 changes: 137 additions & 0 deletions spec/rubocop/cop/rails/root_pathname_methods_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::RootPathnameMethods, :config do
it 'registers an offense when using `::File.open(::Rails.root.join(...))`' do
expect_offense(<<~RUBY)
::File.open(::Rails.root.join('db', 'schema.rb'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#open`.
RUBY

expect_correction(<<~RUBY)
::Rails.root.join('db', 'schema.rb').open
RUBY
end

it 'registers an offense when using `File.open(Rails.root.join(...))`' do
expect_offense(<<~RUBY)
File.open(Rails.root.join('db', 'schema.rb'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#open`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').open
RUBY
end

it "registers an offense when using `File.open(Rails.root.join(...), 'w')`" do
expect_offense(<<~RUBY)
File.open(Rails.root.join('db', 'schema.rb'), 'w')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#open`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').open('w')
RUBY
end

# This is handled by `Rails/RootJoinChain`
it 'does not register an offense when using `File.read(Rails.root.join(...).join(...))`' do
expect_no_offenses(<<~RUBY)
File.read(Rails.root.join('db').join('schema.rb'))
RUBY
end

# This is handled by `Style/FileRead`
it 'does not register an offense when using `File.open(Rails.root.join(...)).read`' do
expect_no_offenses(<<~RUBY)
File.open(Rails.root.join('db', 'schema.rb')).read
RUBY
end

# This is handled by `Style/FileRead`
it 'does not register an offense when using `File.open(Rails.root.join(...)).binread`' do
expect_no_offenses(<<~RUBY)
File.open(Rails.root.join('db', 'schema.rb')).binread
RUBY
end

# This is handled by `Style/FileWrite`
it 'does not register an offense when using `File.open(Rails.root.join(...)).write(content)`' do
expect_no_offenses(<<~RUBY)
File.open(Rails.root.join('db', 'schema.rb')).write(content)
RUBY
end

# This is handled by `Style/FileWrite`
it 'does not register an offense when using `File.open(Rails.root.join(...)).binwrite(content)`' do
expect_no_offenses(<<~RUBY)
File.open(Rails.root.join('db', 'schema.rb')).binwrite(content)
RUBY
end

it 'registers an offense when using `File.read(Rails.root.join(...))`' do
expect_offense(<<~RUBY)
File.read(Rails.root.join('db', 'schema.rb'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#read`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').read
RUBY
end

it 'registers an offense when using `File.read(Rails.root.join(...), ...)`' do
expect_offense(<<~RUBY)
File.read(Rails.root.join('db', 'schema.rb'), 20, 5)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#read`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').read(20, 5)
RUBY
end

it 'registers an offense when using `File.binread(Rails.root.join(...))`' do
expect_offense(<<~RUBY)
File.binread(Rails.root.join('db', 'schema.rb'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#binread`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').binread
RUBY
end

it 'registers an offense when using `File.write(Rails.root.join(...), ...)`' do
expect_offense(<<~RUBY)
File.write(Rails.root.join('db', 'schema.rb'), content, 20)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#write`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').write(content, 20)
RUBY
end

it 'registers an offense when using `File.binwrite(Rails.root.join(...), content)`' do
expect_offense(<<~RUBY)
File.binwrite(Rails.root.join('db', 'schema.rb'), content)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#binwrite`.
RUBY

expect_correction(<<~RUBY)
Rails.root.join('db', 'schema.rb').binwrite(content)
RUBY
end

it 'registers an offense when using `File.open(Rails.root.join(...), ...)` inside an iterator' do
expect_offense(<<~RUBY)
files.map { |file| File.open(Rails.root.join('db', file), 'wb') }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rails.root` is a `Pathname` so you can just append `#open`.
RUBY

expect_correction(<<~RUBY)
files.map { |file| Rails.root.join('db', file).open('wb') }
RUBY
end
end

0 comments on commit aedbf22

Please sign in to comment.