Add new Rails/RootPathnameMethods cop

`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`](#586).
rubocop · Jul 17, 2022 · 467da4e · 467da4e
1 parent c1d5cd7
commit 467da4e
Show file tree

Hide file tree

Showing 6 changed files with 294 additions and 0 deletions.
diff --git a/changelog/new_add_new_railsrootpathnamemethods_cop.md b/changelog/new_add_new_railsrootpathnamemethods_cop.md
@@ -0,0 +1 @@
+* [#587](https://github.com/rubocop/rubocop-rails/pull/587): Add new `Rails/RootPathnameMethods` cop. ([@leoarnold][])
diff --git a/codespell.txt b/codespell.txt
@@ -1 +1,2 @@
 developpment
+filetest
diff --git a/config/default.yml b/config/default.yml
@@ -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

diff --git a/lib/rubocop/cop/rails/root_pathname_methods.rb b/lib/rubocop/cop/rails/root_pathname_methods.rb
@@ -0,0 +1,194 @@
+# 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>s` is a `Pathname` so you can just append `#%<method>s`.'
+
+        DIR_METHODS = %i[
+          children
+          delete
+          each_child
+          empty?
+          entries
+          exist?
+          glob
+          mkdir
+          open
+          rmdir
+          unlink
+        ].to_set.freeze
+
+        FILE_METHODS = %i[
+          atime
+          basename
+          binread
+          binwrite
+          birthtime
+          blockdev?
+          chardev?
+          chmod
+          chown
+          ctime
+          delete
+          directory?
+          dirname
+          empty?
+          executable?
+          executable_real?
+          exist?
+          expand_path
+          extname
+          file?
+          fnmatch
+          fnmatch?
+          ftype
+          grpowned?
+          join
+          lchmod
+          lchown
+          lstat
+          mtime
+          open
+          owned?
+          pipe?
+          read
+          readable?
+          readable_real?
+          readlines
+          readlink
+          realdirpath
+          realpath
+          rename
+          setgid?
+          setuid?
+          size
+          size?
+          socket?
+          split
+          stat
+          sticky?
+          symlink?
+          sysopen
+          truncate
+          unlink
+          utime
+          world_readable?
+          world_writable?
+          writable?
+          writable_real?
+          write
+          zero?
+        ].to_set.freeze
+
+        FILE_TEST_METHODS = %i[
+          blockdev?
+          chardev?
+          directory?
+          empty?
+          executable?
+          executable_real?
+          exist?
+          file?
+          grpowned?
+          owned?
+          pipe?
+          readable?
+          readable_real?
+          setgid?
+          setuid?
+          size
+          size?
+          socket?
+          sticky?
+          symlink?
+          world_readable?
+          world_writable?
+          writable?
+          writable_real?
+          zero?
+        ].to_set.freeze
+
+        FILE_UTILS_METHODS = %i[
+          chmod
+          chown
+          mkdir
+          mkpath
+          rmdir
+          rmtree
+        ].to_set.freeze
+
+        RESTRICT_ON_SEND = (DIR_METHODS + FILE_METHODS + FILE_TEST_METHODS + FILE_UTILS_METHODS).to_set.freeze
+
+        def_node_matcher :pathname_method, <<~PATTERN
+          {
+            (send (const {nil? cbase} :Dir) $DIR_METHODS $_ $...)
+            (send (const {nil? cbase} {:IO :File}) $FILE_METHODS $_ $...)
+            (send (const {nil? cbase} :FileTest) $FILE_TEST_METHODS $_ $...)
+            (send (const {nil? cbase} :FileUtils) $FILE_UTILS_METHODS $_ $...)
+          }
+        PATTERN
+
+        def_node_matcher :rails_root_pathname?, <<~PATTERN
+          {
+            $#rails_root?
+            (send $#rails_root? :join ...)
+          }
+        PATTERN
+
+        # @!method rails_root?(node)
+        def_node_matcher :rails_root?, <<~PATTERN
+          (send (const {nil? cbase} :Rails) {:root :public_path})
+        PATTERN
+
+        def on_send(node)
+          evidence(node) do |method, path, args, rails_root|
+            add_offense(node, message: format(MSG, method: method, rails_root: rails_root.source)) do |corrector|
+              replacement = "#{path.source}.#{method}"
+              replacement += "(#{args.map(&:source).join(', ')})" unless args.empty?
+
+              corrector.replace(node, replacement)
+            end
+          end
+        end
+
+        private
+
+        def evidence(node)
+          return if node.method?(:open) && node.parent&.send_type?
+          return unless (method, path, args = pathname_method(node)) && (rails_root = rails_root_pathname?(path))
+
+          yield(method, path, args, rails_root)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb
@@ -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'

diff --git a/spec/rubocop/cop/rails/root_pathname_methods_spec.rb b/spec/rubocop/cop/rails/root_pathname_methods_spec.rb
@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::RootPathnameMethods, :config do
+  {
+    Dir: described_class::DIR_METHODS,
+    File: described_class::FILE_METHODS,
+    FileTest: described_class::FILE_TEST_METHODS,
+    FileUtils: described_class::FILE_UTILS_METHODS,
+    IO: described_class::FILE_METHODS
+  }.each do |receiver, methods|
+    methods.each do |method|
+      it "registers an offense when using `#{receiver}.#{method}(Rails.public_path)` (if arity exists)" do
+        expect_offense(<<~RUBY, receiver: receiver, method: method)
+          %{receiver}.%{method}(Rails.public_path)
+          ^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^ `Rails.public_path` is a `Pathname` so you can just append `#%{method}`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Rails.public_path.#{method}
+        RUBY
+      end
+
+      it "registers an offense when using `::#{receiver}.#{method}(::Rails.root.join(...))` (if arity exists)" do
+        expect_offense(<<~RUBY, receiver: receiver, method: method)
+          ::%{receiver}.%{method}(::Rails.root.join('db', 'schema.rb'))
+          ^^^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::Rails.root` is a `Pathname` so you can just append `#%{method}`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          ::Rails.root.join('db', 'schema.rb').#{method}
+        RUBY
+      end
+
+      it "registers an offense when using `::#{receiver}.#{method}(::Rails.root.join(...), ...)` (if arity exists)" do
+        expect_offense(<<~RUBY, receiver: receiver, method: method)
+          ::%{receiver}.%{method}(::Rails.root.join('db', 'schema.rb'), 20, 5)
+          ^^^{receiver}^^{method}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::Rails.root` is a `Pathname` so you can just append `#%{method}`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          ::Rails.root.join('db', 'schema.rb').#{method}(20, 5)
+        RUBY
+      end
+    end
+  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.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