[Fix #603] Auto-correct with multiple validation attributes

Author: pirj

changelog/fix_redundant_presence_validation_with_multi_attributes.md

@@ -0,0 +1 @@
+* [#603](https://github.com/rubocop/rubocop-rails/issues/603): Fix autocorrection of multiple attributes for `Rails/RedundantPresenceValidationOnBelongsTo` cop. ([@pirj][])

lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb

@@ -52,7 +52,7 @@ class RedundantPresenceValidationOnBelongsTo < Base
         #   @example source that matches - by a foreign key
         #     validates :user_id, presence: true
         def_node_matcher :presence_validation?, <<~PATTERN
-          $(
+          (
             send nil? :validates
             (sym $_)+
             $(hash <$(pair (sym :presence) {true hash}) ...>)
@@ -152,27 +152,44 @@ class RedundantPresenceValidationOnBelongsTo < Base
         PATTERN
 
         def on_send(node)
-          validation, all_keys, options, presence = presence_validation?(node)
-          return unless validation
+          presence_validation?(node) do |all_keys, options, presence|
+            keys = non_optional_belongs_to(node.parent, all_keys)
+            return if keys.none?
 
-          keys = all_keys.select do |key|
-            belongs_to = belongs_to_for(node.parent, key)
-            belongs_to && !optional?(belongs_to)
+            add_offense_and_correct(node, all_keys, keys, options, presence)
           end
-          return if keys.none?
+        end
 
+        private
+
+        def add_offense_and_correct(node, all_keys, keys, options, presence)
           add_offense(presence, message: message_for(keys)) do |corrector|
-            remove_presence_validation(corrector, node, options, presence)
+            if options.children.one? # `presence: true` is the only option
+              if keys == all_keys
+                remove_validation(corrector, node)
+              else
+                remove_keys_from_validation(corrector, node, keys)
+              end
+            elsif keys == all_keys
+              remove_presence_option(corrector, presence)
+            else
+              extract_validation_for_keys(corrector, node, keys, options)
+            end
           end
         end
 
-        private
-
         def message_for(keys)
           display_keys = keys.map { |key| "`#{key}`" }.join('/')
           format(MSG, association: display_keys)
         end
 
+        def non_optional_belongs_to(node, keys)
+          keys.select do |key|
+            belongs_to = belongs_to_for(node, key)
+            belongs_to && !optional?(belongs_to)
+          end
+        end
+
         def belongs_to_for(model_class_node, key)
           if key.to_s.end_with?('_id')
             normalized_key = key.to_s.delete_suffix('_id').to_sym
@@ -182,17 +199,48 @@ def belongs_to_for(model_class_node, key)
           end
         end
 
-        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
+        def remove_validation(corrector, node)
+          corrector.remove(validation_range(node))
+        end
+
+        def remove_keys_from_validation(corrector, node, keys)
+          keys.each do |key|
+            key_node = node.arguments.find { |arg| arg.value == key }
+            key_range = range_with_surrounding_space(
+              range: range_with_surrounding_comma(key_node.source_range, :right),
+              side: :right
             )
-            corrector.remove(range)
+            corrector.remove(key_range)
           end
         end
+
+        def remove_presence_option(corrector, presence)
+          range = range_with_surrounding_comma(
+            range_with_surrounding_space(range: presence.source_range, side: :left),
+            :left
+          )
+          corrector.remove(range)
+        end
+
+        def extract_validation_for_keys(corrector, node, keys, options)
+          indentation = ' ' * node.source_range.column
+          options_without_presence = options.children.reject { |pair| pair.key.value == :presence }
+          source = [
+            indentation,
+            'validates ',
+            keys.map(&:inspect).join(', '),
+            ', ',
+            options_without_presence.map(&:source).join(', '),
+            "\n"
+          ].join
+
+          remove_keys_from_validation(corrector, node, keys)
+          corrector.insert_after(validation_range(node), source)
+        end
+
+        def validation_range(node)
+          range_by_whole_lines(node.source_range, include_final_newline: true)
+        end
       end
     end
   end

spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb

@@ -115,6 +115,64 @@
         RUBY
       end
 
+      it 'registers an offense for multiple attributes when not all are associations' do
+        expect_offense(<<~RUBY)
+          belongs_to :user
+          validates :user, :name, presence: true
+                                  ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          belongs_to :user
+          validates :name, presence: true
+        RUBY
+      end
+
+      it 'registers an offense for a secondary attribute' do
+        expect_offense(<<~RUBY)
+          belongs_to :user
+          validates :name, :user, presence: true
+                                  ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          belongs_to :user
+          validates :name, presence: true
+        RUBY
+      end
+
+      it 'registers an offense for multiple attributes and options' do
+        expect_offense(<<~RUBY)
+          belongs_to :user
+          validates :user, :name, presence: true, uniqueness: true
+                                  ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          belongs_to :user
+          validates :name, presence: true, uniqueness: true
+          validates :user, uniqueness: true
+        RUBY
+      end
+
+      it 'preserves indentation for the extracted validation line' do
+        expect_offense(<<~RUBY)
+          class Profile
+            belongs_to :user
+            validates :user, :name, presence: true, uniqueness: true
+                                    ^^^^^^^^^^^^^^ Remove explicit presence validation for `user`.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class Profile
+            belongs_to :user
+            validates :name, presence: true, uniqueness: true
+            validates :user, uniqueness: true
+          end
+        RUBY
+      end
+
       it 'registers an offense for presence with a message' do
         expect_offense(<<~RUBY)
           belongs_to :user