Add new Rails/RedundantForeignKey cop

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [#211](https://github.com/rubocop-hq/rubocop-rails/issues/211): Add autocorrect to `Rails/RakeEnvironment` cop. ([@tejasbubane][])
 * [#242](https://github.com/rubocop-hq/rubocop-rails/pull/242): Add `Rails/ContentTag` cop. ([@tabuchi0919][])
 * [#249](https://github.com/rubocop-hq/rubocop-rails/pull/249): Add new `Rails/Pick` cop. ([@eugeneius][])
+* [#257](https://github.com/rubocop-hq/rubocop-rails/pull/257): Add new `Rails/RedundantForeignKey` cop. ([@eugeneius][])
 
 ### Bug fixes
 

config/default.yml

@@ -408,6 +408,11 @@ Rails/RedundantAllowNil:
   Include:
     - app/models/**/*.rb
 
+Rails/RedundantForeignKey:
+  Description: 'Checks for associations where the `:foreign_key` option is redundant.'
+  Enabled: true
+  VersionAdded: '2.6'
+
 Rails/RedundantReceiverInWithOptions:
   Description: 'Checks for redundant receiver in `with_options`.'
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -48,6 +48,7 @@
 * xref:cops_rails.adoc#railsrakeenvironment[Rails/RakeEnvironment]
 * xref:cops_rails.adoc#railsreadwriteattribute[Rails/ReadWriteAttribute]
 * xref:cops_rails.adoc#railsredundantallownil[Rails/RedundantAllowNil]
+* xref:cops_rails.adoc#railsredundantforeignkey[Rails/RedundantForeignKey]
 * xref:cops_rails.adoc#railsredundantreceiverinwithoptions[Rails/RedundantReceiverInWithOptions]
 * xref:cops_rails.adoc#railsreflectionclassname[Rails/ReflectionClassName]
 * xref:cops_rails.adoc#railsrefutemethods[Rails/RefuteMethods]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -2391,6 +2391,44 @@ validates :x, length: { is: 5 }, allow_nil: true, allow_blank: false
 | Array
 |===
 
+== Rails/RedundantForeignKey
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Enabled
+| Yes
+| Yes
+| 2.6
+| -
+|===
+
+This cop detects cases where the `:foreign_key` option on associations
+is redundant.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+class Post
+  has_many :comments, foreign_key: 'post_id'
+end
+
+class Comment
+  belongs_to :post, foreign_key: 'post_id'
+end
+
+# good
+class Post
+  has_many :comments
+end
+
+class Comment
+  belongs_to :author, foreign_key: 'user_id'
+end
+----
+
 == Rails/RedundantReceiverInWithOptions
 
 |===

lib/rubocop/cop/rails/redundant_foreign_key.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cop detects cases where the `:foreign_key` option on associations
+      # is redundant.
+      #
+      # @example
+      #   # bad
+      #   class Post
+      #     has_many :comments, foreign_key: 'post_id'
+      #   end
+      #
+      #   class Comment
+      #     belongs_to :post, foreign_key: 'post_id'
+      #   end
+      #
+      #   # good
+      #   class Post
+      #     has_many :comments
+      #   end
+      #
+      #   class Comment
+      #     belongs_to :author, foreign_key: 'user_id'
+      #   end
+      class RedundantForeignKey < Cop
+        include RangeHelp
+
+        MSG = 'Specifying the default value for `foreign_key` is redundant.'
+
+        def_node_matcher :association_with_foreign_key, <<~PATTERN
+          (send nil? ${:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_)
+            $(hash <$(pair (sym :foreign_key) ({sym str} $_)) ...>)
+          )
+        PATTERN
+
+        def on_send(node)
+          association_with_foreign_key(node) do |type, name, options, foreign_key_pair, foreign_key|
+            if redundant?(node, type, name, options, foreign_key)
+              add_offense(node, location: foreign_key_pair.loc.expression)
+            end
+          end
+        end
+
+        def autocorrect(node)
+          _type, _name, _options, foreign_key_pair, _foreign_key = association_with_foreign_key(node)
+          range = range_with_surrounding_space(range: foreign_key_pair.source_range, side: :left)
+          range = range_with_surrounding_comma(range, :left)
+
+          lambda do |corrector|
+            corrector.remove(range)
+          end
+        end
+
+        private
+
+        def redundant?(node, association_type, association_name, options, foreign_key)
+          foreign_key.to_s == default_foreign_key(node, association_type, association_name, options)
+        end
+
+        def default_foreign_key(node, association_type, association_name, options)
+          if association_type == :belongs_to
+            "#{association_name}_id"
+          elsif (as = find_as_option(options))
+            "#{as}_id"
+          else
+            node.parent_module_name&.foreign_key
+          end
+        end
+
+        def find_as_option(options)
+          options.pairs.find do |pair|
+            pair.key.sym_type? && pair.key.value == :as
+          end&.value&.value
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -50,6 +50,7 @@
 require_relative 'rails/rake_environment'
 require_relative 'rails/read_write_attribute'
 require_relative 'rails/redundant_allow_nil'
+require_relative 'rails/redundant_foreign_key'
 require_relative 'rails/redundant_receiver_in_with_options'
 require_relative 'rails/reflection_class_name'
 require_relative 'rails/refute_methods'

spec/rubocop/cop/rails/redundant_foreign_key_spec.rb

@@ -0,0 +1,174 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::RedundantForeignKey do
+  subject(:cop) { described_class.new }
+
+  context '`belongs_to` associations' do
+    it 'registers an offense when the `:foreign_key` option is redundant' do
+      expect_offense(<<~RUBY)
+        class Comment
+          belongs_to :post, foreign_key: 'post_id'
+                            ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class Comment
+          belongs_to :post
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when the `:foreign_key` option is not redundant' do
+      expect_no_offenses(<<~RUBY)
+        class Comment
+          belongs_to :author, foreign_key: 'user_id'
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when the `:foreign_key` option is absent' do
+      expect_no_offenses(<<~RUBY)
+        class Comment
+          belongs_to :author
+        end
+      RUBY
+    end
+
+    it 'registers an offense even when other options are used' do
+      expect_offense(<<~RUBY)
+        class Comment
+          belongs_to :post, class_name: 'SpecialPost', foreign_key: 'post_id', dependent: :destroy
+                                                       ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class Comment
+          belongs_to :post, class_name: 'SpecialPost', dependent: :destroy
+        end
+      RUBY
+    end
+
+    it 'registers an offense even when defined in a block' do
+      expect_offense(<<~RUBY)
+        class_methods do
+          belongs_to :post, foreign_key: 'post_id'
+                            ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class_methods do
+          belongs_to :post
+        end
+      RUBY
+    end
+  end
+
+  %w[has_one has_many has_and_belongs_to_many].each do |association_type|
+    context "`#{association_type}` associations" do
+      it 'registers an offense when the `:foreign_key` option is redundant' do
+        expect_offense(<<~RUBY, association_type: association_type)
+          class Book
+            %{association_type} :chapter, foreign_key: 'book_id'
+            _{association_type}           ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class Book
+            #{association_type} :chapter
+          end
+        RUBY
+      end
+
+      it 'does not register an offense when the `:foreign_key` option is not redundant' do
+        expect_no_offenses(<<~RUBY)
+          class Book
+            #{association_type} :chapter, foreign_key: 'publication_id'
+          end
+        RUBY
+      end
+
+      it 'does not register an offense when the `:foreign_key` option is absent' do
+        expect_no_offenses(<<~RUBY)
+          class Book
+            #{association_type} :chapter
+          end
+        RUBY
+      end
+
+      it 'registers an offense even when other options are used' do
+        expect_offense(<<~RUBY, association_type: association_type)
+          class Book
+            %{association_type} :chapter, class_name: 'SpecialChapter', foreign_key: 'book_id', dependent: :destroy
+            _{association_type}                                         ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class Book
+            #{association_type} :chapter, class_name: 'SpecialChapter', dependent: :destroy
+          end
+        RUBY
+      end
+
+      it 'registers an offense even when multiple associations are defined' do
+        expect_offense(<<~RUBY, association_type: association_type)
+          class Book
+            belongs_to :series
+
+            %{association_type} :chapter, foreign_key: 'book_id'
+            _{association_type}           ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class Book
+            belongs_to :series
+
+            #{association_type} :chapter
+          end
+        RUBY
+      end
+
+      it 'does not register an offense when defined in a block' do
+        expect_no_offenses(<<~RUBY)
+          class_methods do
+            #{association_type} :chapter, foreign_key: 'book_id'
+          end
+        RUBY
+      end
+
+      it 'registers an offense when the `:foreign_key` options is redundant with the `:as` option' do
+        expect_offense(<<~RUBY, association_type: association_type)
+          class Book
+            %{association_type} :chapter, as: :publishable, foreign_key: 'publishable_id'
+            _{association_type}                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value for `foreign_key` is redundant.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class Book
+            #{association_type} :chapter, as: :publishable
+          end
+        RUBY
+      end
+
+      it 'does not register an offense when the `:foreign_key` option is not redundant with the `:as` option' do
+        expect_no_offenses(<<~RUBY)
+          class Book
+            #{association_type} :chapter, as: :publishable, foreign_key: 'book_id'
+          end
+        RUBY
+      end
+
+      it 'does not register an offense when the class cannot be determined' do
+        expect_no_offenses(<<~RUBY)
+          #{association_type} :chapter, foreign_key: 'book_id'
+        RUBY
+      end
+    end
+  end
+end