[Fix rubocop#587] Add Rails/RedundantPresenceValidationOnBelongsTo cop

changelog/new_redundant_presence_validation_for_required_belongs_to_association_cop.md

@@ -0,0 +1 @@
+* [#594](https://github.com/rubocop/rubocop-rails/pulls/594): Add `Rails/RedundantPresenceValidationOnBelongsTo` cop. ([@pirj][])

config/default.yml

@@ -618,6 +618,11 @@ Rails/RedundantForeignKey:
   Enabled: true
   VersionAdded: '2.6'
 
+Rails/RedundantPresenceValidationOnBelongsTo:
+  Description: 'Checks for redundant presence validation on belongs_to association.'
+  Enabled: pending
+  VersionAdded: '<<next>>'
+
 Rails/RedundantReceiverInWithOptions:
   Description: 'Checks for redundant receiver in `with_options`.'
   Enabled: true

lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb

@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Since Rails 5.0 the default for `belongs_to` is `optional: false`
+      # unless `config.active_record.belongs_to_required_by_default` is
+      # explicitly set to `false`. The presence validator is added
+      # automatically, and explicit presence validation is redundant.
+      #
+      # @example
+      #   # bad
+      #   belongs_to :user
+      #   validates :user, presence: true
+      #
+      #   # bad
+      #   belongs_to :user
+      #   validates :user_id, presence: true
+      #
+      #   # bad
+      #   belongs_to :author, foreign_key: :user_id
+      #   validates :user_id, presence: true
+      #
+      #   # good
+      #   belongs_to :user
+      #
+      #   # good
+      #   belongs_to :author, foreign_key: :user_id
+      #
+      class RedundantPresenceValidationOnBelongsTo < Base
+        include RangeHelp
+        extend AutoCorrector
+        extend TargetRailsVersion
+
+        MSG = 'Remove explicit presence validation for `%<association>s`.'
+        RESTRICT_ON_SEND = %i[validates].freeze
+
+        minimum_target_rails_version 5.0
+
+        def_node_matcher :presence_validation?, <<~PATTERN
+          $(
+            send nil? :validates
+            (sym $_)
+            ...
+            $(hash <$(pair (sym :presence) {true hash}) ...>)
+          )
+        PATTERN
+
+        def_node_matcher :optional_option?, <<~PATTERN
+          {
+            (hash <(pair (sym :optional) true) ...>)
+            (hash <(pair (sym :required) false) ...>)
+          }
+        PATTERN
+
+        def_node_matcher :optional?, <<~PATTERN
+          (send nil? :belongs_to _ ... #optional_option?)
+        PATTERN
+
+        def_node_matcher :class_with_required_belongs_to?, <<~PATTERN
+          (
+            begin
+            <
+              ${
+                (send nil? :belongs_to (sym %key) ...)
+                (send nil? :belongs_to ... (hash <(pair (sym :foreign_key) (sym %fk)) ...>))
+              }
+              ...
+            >
+          )
+        PATTERN
+
+        def on_send(node)
+          validation, key, options, presence = presence_validation?(node)
+          return unless validation
+
+          normalized_key = key.to_s.sub(/_id\Z/, '').to_sym
+          default_foreign_key = :"#{normalized_key}_id"
+          belongs_to = class_with_required_belongs_to?(node.parent, key: normalized_key, fk: default_foreign_key)
+          return unless belongs_to
+          return if optional?(belongs_to)
+
+          message = format(MSG, association: key.to_s)
+
+          add_offense(presence, message: message) do |corrector|
+            remove_presence_validation(corrector, node, options, presence)
+          end
+        end
+
+        private
+
+        def remove_presence_validation(corrector, node, options, presence)
+          if options.children.one?
+            corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))
+          else
+            range = range_with_surrounding_comma(
+              range_with_surrounding_space(range: presence.source_range, side: :left),
+              :left
+            )
+            corrector.remove(range)
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -73,6 +73,7 @@
 require_relative 'rails/read_write_attribute'
 require_relative 'rails/redundant_allow_nil'
 require_relative 'rails/redundant_foreign_key'
+require_relative 'rails/redundant_presence_validation_on_belongs_to'
 require_relative 'rails/redundant_receiver_in_with_options'
 require_relative 'rails/redundant_travel_back'
 require_relative 'rails/reflection_class_name'

spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb

@@ -0,0 +1,179 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::RedundantPresenceValidationOnBelongsTo, :config do
+  context 'with an explicit redundant presence validation for a required belongs_to association' do
+    it 'registers an offense' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates :user, presence: true
+                         ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+      RUBY
+    end
+
+    it 'registers an offense for the default foreign key' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates :user_id, presence: true
+                            ^^^^^^^^^^^^^^ Remove explicit presence validation for `user_id`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+      RUBY
+    end
+
+    it 'registers an offense when belongs_to has an explicit foreign key' do
+      expect_offense(<<~RUBY)
+        belongs_to :author, foreign_key: :user_id
+        validates :user_id, presence: true
+                            ^^^^^^^^^^^^^^ Remove explicit presence validation for `user_id`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :author, foreign_key: :user_id
+      RUBY
+    end
+
+    it 'registers an offense when belongs_to has an explicit foreign key with other options' do
+      expect_offense(<<~RUBY)
+        belongs_to :author, class_name: 'User', foreign_key: :user_id
+        validates :user_id, presence: true
+                            ^^^^^^^^^^^^^^ Remove explicit presence validation for `user_id`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :author, class_name: 'User', foreign_key: :user_id
+      RUBY
+    end
+
+    it 'registers an offense even when belongs_to has an explicit foreign key' do
+      expect_offense(<<~RUBY)
+        belongs_to :author, foreign_key: :user_id
+        validates :author, presence: true
+                           ^^^^^^^^^^^^^^ Remove explicit presence validation for `author`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :author, foreign_key: :user_id
+      RUBY
+    end
+
+    it 'registers an offense even when validates has other preceding options' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates :user, uniqueness: true, presence: true
+                                           ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+        validates :user, uniqueness: true
+      RUBY
+    end
+
+    it 'registers an offense even when validates has other trailing options' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates :user, presence: true, uniqueness: true
+                         ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+        validates :user, uniqueness: true
+      RUBY
+    end
+
+    it 'registers an offense for presence with a message' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates :user, presence: { message: 'Must be present' }
+                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+      RUBY
+    end
+
+    pending 'registers an offense even when the presence option is factored out' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        with_options presence: true do
+                     ^^^^^^^^^^^^^^ Remove explicit presence validation for `:user`.
+          validates :user
+        end
+      RUBY
+    end
+
+    pending 'registers an offense for non-absence option' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates :user, absence: false
+                         ^^^^^^^^^^^^^^ Remove explicit presence validation for `:user`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+      RUBY
+    end
+
+    pending 'registers an offense when validation key is a string' do
+      expect_offense(<<~RUBY)
+        belongs_to :user
+        validates 'user', presence: true
+                          ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        belongs_to :user
+      RUBY
+    end
+  end
+
+  it 'does not register an offense with redundant presence' do
+    expect_no_offenses(<<~RUBY)
+      belongs_to :user
+      validates :user_count, presence: true
+    RUBY
+  end
+
+  it 'does not register an offense with presence validation turned off' do
+    expect_no_offenses(<<~RUBY)
+      belongs_to :user
+      validates :user_id, presence: false
+    RUBY
+  end
+
+  it 'does not register an offense with an optional belongs_to' do
+    expect_no_offenses(<<~RUBY)
+      belongs_to :user, optional: true
+      validates :user_id, presence: true
+    RUBY
+  end
+
+  it 'does not register an offense with a non-required belongs_to' do
+    expect_no_offenses(<<~RUBY)
+      belongs_to :user, required: false
+      validates :user_id, presence: true
+    RUBY
+  end
+
+  pending 'does not register an offense when foreign keys do not match' do
+    expect_no_offenses(<<~RUBY)
+      belongs_to :author, foreign_key: :user_id
+      validates :author_id, presence: true
+    RUBY
+  end
+
+  it 'does not register an offense for optional association with explicit foreign key' do
+    expect_no_offenses(<<~RUBY)
+      belongs_to :author, foreign_key: :user_id, optional: true
+      validates :author, presence: true
+    RUBY
+  end
+end