Add new Rails/RootJoinChain cop

Instead of chaining `#join` on `Rails.root` or `Rails.public_path`

```ruby
Rails.root.join('db').join('schema.rb')
Rails.root.join('db').join(migrate).join('migration.rb')
Rails.public_path.join('path').join('file.pdf')
Rails.public_path.join('path').join(to).join('file.pdf')
```

we can combine all arguments into a single join

```ruby
Rails.root.join('db', 'schema.rb')
Rails.root.join('db', migrate, 'migration.rb')
Rails.public_path.join('path', 'file.pdf')
Rails.public_path.join('path', to, 'file.pdf')
```

Author: leoarnold

changelog/new_add_new_railsrootjoinchain_cop.md

@@ -0,0 +1 @@
+* [#586](https://github.com/rubocop/rubocop-rails/pull/586): Add new `Rails/RootJoinChain` cop. ([@leoarnold][])

config/default.yml

@@ -701,6 +701,11 @@ Rails/ReversibleMigrationMethodDefinition:
   Include:
     - db/migrate/*.rb
 
+Rails/RootJoinChain:
+  Description: 'Use a single `#join` instead of chaining on `Rails.root` or `Rails.public_path`.'
+  Enabled: pending
+  VersionAdded: '<<next>>'
+
 Rails/SafeNavigation:
   Description: "Use Ruby's safe navigation operator (`&.`) instead of `try!`."
   Enabled: true

lib/rubocop/cop/rails/root_join_chain.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Use a single `#join` instead of chaining on `Rails.root` or `Rails.public_path`.
+      #
+      # @example
+      #   # bad
+      #   Rails.root.join('db').join('schema.rb')
+      #   Rails.root.join('db').join(migrate).join('migration.rb')
+      #   Rails.public_path.join('path').join('file.pdf')
+      #   Rails.public_path.join('path').join(to).join('file.pdf')
+      #
+      #   # good
+      #   Rails.root.join('db', 'schema.rb')
+      #   Rails.root.join('db', migrate, 'migration.rb')
+      #   Rails.public_path.join('path', 'file.pdf')
+      #   Rails.public_path.join('path', to, 'file.pdf')
+      #
+      class RootJoinChain < Base
+        extend AutoCorrector
+        include RangeHelp
+
+        MSG = 'Use `%<root>s.join(...)` instead of chaining `#join` calls.'
+
+        RESTRICT_ON_SEND = %i[join].to_set.freeze
+
+        # @!method rails_root?(node)
+        def_node_matcher :rails_root?, <<~PATTERN
+          (send (const {nil? cbase} :Rails) {:root :public_path})
+        PATTERN
+
+        # @!method join?(node)
+        def_node_matcher :join?, <<~PATTERN
+          (send _ :join $...)
+        PATTERN
+
+        def on_send(node)
+          evidence(node) do |rails_node, args|
+            add_offense(node, message: format(MSG, root: rails_node.source)) do |corrector|
+              range = range_between(rails_node.loc.selector.end_pos, node.loc.expression.end_pos)
+              replacement = ".join(#{args.map(&:source).join(', ')})"
+
+              corrector.replace(range, replacement)
+            end
+          end
+        end
+
+        private
+
+        def evidence(node)
+          # Are we at the *end* of the join chain?
+          return if join?(node.parent)
+          # Is there only one join?
+          return if rails_root?(node.receiver)
+
+          all_args = []
+
+          while (args = join?(node))
+            all_args = args + all_args
+            node = node.receiver
+          end
+
+          rails_root?(node) do
+            yield(node, all_args)
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -11,7 +11,6 @@
 require_relative 'rails/active_record_callbacks_order'
 require_relative 'rails/active_record_override'
 require_relative 'rails/active_support_aliases'
-require_relative 'rails/schema_comment'
 require_relative 'rails/add_column_index'
 require_relative 'rails/after_commit_override'
 require_relative 'rails/application_controller'
@@ -85,9 +84,11 @@
 require_relative 'rails/require_dependency'
 require_relative 'rails/reversible_migration'
 require_relative 'rails/reversible_migration_method_definition'
+require_relative 'rails/root_join_chain'
 require_relative 'rails/safe_navigation'
 require_relative 'rails/safe_navigation_with_blank'
 require_relative 'rails/save_bang'
+require_relative 'rails/schema_comment'
 require_relative 'rails/scope_args'
 require_relative 'rails/short_i18n'
 require_relative 'rails/skips_model_validations'

spec/project_spec.rb

@@ -13,6 +13,8 @@
       config.tap { |c| c.delete('inherit_mode') }.keys
     end
 
+    let(:version_regexp) { /\A\d+\.\d+\z|\A<<next>>\z/ }
+
     it 'has a nicely formatted description for all cops' do
       cop_names.each do |name|
         description = config[name]['Description']
@@ -23,11 +25,23 @@
 
     it 'requires a nicely formatted `VersionAdded` metadata for all cops' do
       cop_names.each do |name|
-        version = config[name]['VersionAdded']
+        version = config.dig(name, 'VersionAdded')
         expect(version.nil?).to(be(false),
-                                "VersionAdded is required for #{name}.")
-        expect(version).to(match(/\A\d+\.\d+\z/),
-                           "#{version} should be format ('X.Y') for #{name}.")
+                                "`VersionAdded` configuration is required for `#{name}`.")
+        expect(version).to(match(version_regexp),
+                           "#{version} should be format ('X.Y' or '<<next>>') for #{name}.")
+      end
+    end
+
+    %w[VersionChanged VersionRemoved].each do |version_type|
+      it "requires a nicely formatted `#{version_type}` metadata for all cops" do
+        cop_names.each do |name|
+          version = config.dig(name, version_type)
+          next unless version
+
+          expect(version).to(match(version_regexp),
+                             "#{version} should be format ('X.Y' or '<<next>>') for #{name}.")
+        end
       end
     end
 

spec/rubocop/cop/rails/root_join_chain_spec.rb

@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::RootJoinChain, :config do
+  it 'does not register an offense for `Rails.root.join(...)`' do
+    expect_no_offenses(<<~RUBY)
+      Rails.root.join('db', 'schema.rb')
+    RUBY
+  end
+
+  it 'registers and offense and corrects for `::Rails.root.join(...).join(...)`' do
+    expect_offense(<<~RUBY)
+      ::Rails.root.join('db').join('sch' + 'ema.rb')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `::Rails.root.join(...)` instead of chaining `#join` calls.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      ::Rails.root.join('db', 'sch' + 'ema.rb')
+    RUBY
+  end
+
+  it 'registers and offense and corrects for `::Rails.root.join(...).join(...).read`' do
+    expect_offense(<<~RUBY)
+      ::Rails.root.join('db').join('sch' + 'ema.rb').read
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `::Rails.root.join(...)` instead of chaining `#join` calls.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      ::Rails.root.join('db', 'sch' + 'ema.rb').read
+    RUBY
+  end
+
+  it 'registers and offense and corrects for `Rails.root.join(...).join(...)`' do
+    expect_offense(<<~RUBY)
+      Rails.root.join('db').join('sch' + 'ema.rb')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Rails.root.join(...)` instead of chaining `#join` calls.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      Rails.root.join('db', 'sch' + 'ema.rb')
+    RUBY
+  end
+
+  it 'registers and offense and corrects for `Rails.root` with any number of joins greater one' do
+    expect_offense(<<~RUBY)
+      Rails.root.join.join.join('db').join(migrate).join.join("migration.\#{rb}")
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Rails.root.join(...)` instead of chaining `#join` calls.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      Rails.root.join('db', migrate, "migration.\#{rb}")
+    RUBY
+  end
+
+  it 'does not register an offense for `Rails.public_path.join(...)`' do
+    expect_no_offenses(<<~RUBY)
+      Rails.public_path.join('path', 'file.pdf')
+    RUBY
+  end
+
+  it 'registers and offense and corrects for `Rails.public_path.join(...).join(...)`' do
+    expect_offense(<<~RUBY)
+      Rails.public_path.join('path').join('fi' + 'le.pdf')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Rails.public_path.join(...)` instead of chaining `#join` calls.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      Rails.public_path.join('path', 'fi' + 'le.pdf')
+    RUBY
+  end
+
+  it 'registers and offense and corrects for `Rails.public_path` with any number of joins greater one' do
+    expect_offense(<<~RUBY)
+      Rails.public_path.join.join.join('path').join(to).join.join("file.\#{pdf}")
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Rails.public_path.join(...)` instead of chaining `#join` calls.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      Rails.public_path.join('path', to, "file.\#{pdf}")
+    RUBY
+  end
+end