diff --git a/changelog/new_add_new_railsrootpathnamemethods_cop.md b/changelog/new_add_new_railsrootpathnamemethods_cop.md new file mode 100644 index 0000000000..38d629c9af --- /dev/null +++ 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 index 732d1968be..f2acc5ca06 100644 --- a/codespell.txt +++ b/codespell.txt @@ -1 +1,2 @@ developpment +filetest diff --git a/config/default.yml b/config/default.yml index 7c886526eb..0965a3c778 100644 --- 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: '<>' + 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 new file mode 100644 index 0000000000..1ce24bb0e2 --- /dev/null +++ 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 = '`%s` is a `Pathname` so you can just append `#%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 index cf7c6c5769..b0e5e7e3a5 100644 --- 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 new file mode 100644 index 0000000000..c0bfce36cb --- /dev/null +++ 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}.#{method}(Rails.public_path) + #{'^' * receiver.size}^#{'^' * method.size}^^^^^^^^^^^^^^^^^^^ `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}.#{method}(::Rails.root.join('db', 'schema.rb')) + ^^#{'^' * receiver.size}^#{'^' * method.size}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::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}.#{method}(::Rails.root.join('db', 'schema.rb'), 20, 5) + ^^#{'^' * receiver.size}^#{'^' * method.size}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `::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