Add new Rails/ValidationsGrouping cop

changelog/new_add_validations_grouping_cop.md

@@ -0,0 +1 @@
+* [#555](https://github.com/rubocop/rubocop-rails/issues/555): Add new `Rails/ValidationsGrouping` cop. ([@fatkodima][])

config/default.yml

@@ -1192,6 +1192,16 @@ Rails/Validation:
   Include:
     - app/models/**/*.rb
 
+Rails/ValidationsGrouping:
+  Description: 'Group together all attribute validations.'
+  StyleGuide: 'https://rails.rubystyle.guide/#single-attribute-validations'
+  Enabled: pending
+  VersionAdded: '<<next>>'
+  # Enforces single attribute per validation.
+  SingleAttributeValidations: false
+  Include:
+    - app/models/**/*.rb
+
 Rails/WhereEquals:
   Description: 'Pass conditions to `where` and `where.not` as a hash instead of manually constructing SQL.'
   StyleGuide: 'https://rails.rubystyle.guide/#hash-conditions'

lib/rubocop/cop/rails/validations_grouping.rb

@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Enforces the grouping of attribute validations.
+      #
+      # @example
+      #   # bad
+      #   validates :name, :email, :bio, presence: true
+      #   validates :email, format: { with: /@/ }
+      #
+      #   # good
+      #   validates :name, :bio, presence: true
+      #   validates :email, presence: true, format: { with: /@/ }
+      #
+      #   # good
+      #   validates :name, :email, :bio, presence: true, if: :some_condition?
+      #   validates :email, format: { with: /@/ }
+      #
+      # @example SingleAttributeValidations: true
+      #   # Enforces single attribute per validation.
+      #
+      #   # bad
+      #   validates :name, :bio, presence: true
+      #
+      #   # good
+      #   validates :name, presence: true
+      #   validates :bio, presence: true
+      #
+      class ValidationsGrouping < Base
+        MSG = 'Group together all %<attribute>s validations.'
+        SINGLE_ATTRIBUTE_MSG = 'Specify single attribute per validation.'
+
+        VALIDATION_KEYS = %w[acceptance confirmation comparison exclusion format inclusion
+                             length numericality presence absence uniqueness].freeze
+
+        RESTRICT_ON_SEND = %i[validates].freeze
+
+        def_node_matcher :validates?, <<~PATTERN
+          (send nil? :validates ${sym str} + hash)
+        PATTERN
+
+        def on_new_investigation
+          # Hash is keyed by parent id and then by attribute name within it.
+          @validations = Hash.new do |outer, parent|
+            outer[parent] = Hash.new { |inner, attribute| inner[attribute] = [] }
+          end
+          @validations.compare_by_identity
+        end
+
+        def on_send(node)
+          validates?(node) do |attribute_nodes|
+            if attribute_nodes.size > 1 && cop_config['SingleAttributeValidations']
+              add_offense(node, message: SINGLE_ATTRIBUTE_MSG)
+            end
+
+            check_for_same_attributes_validations(node, attribute_nodes)
+          end
+        end
+
+        private
+
+        def check_for_same_attributes_validations(validation_node, attribute_nodes)
+          attribute_nodes.each do |attribute_node|
+            parent_node = validation_node.parent
+            attribute_name = attribute_node.value.to_s
+            @validations[parent_node][attribute_name] << validation_node
+
+            validation_nodes = @validations[parent_node][attribute_name]
+            next unless validation_nodes.size > 1 && same_options?(validation_nodes)
+
+            message = format(MSG, attribute: attribute_name)
+
+            validation_nodes.each do |node|
+              add_offense(attribute_node(node, attribute_name), message: message)
+            end
+          end
+        end
+
+        def same_options?(validation_nodes)
+          options = validation_nodes.map do |node|
+            option_node = node.last_argument
+            option_node.pairs.to_h do |pair|
+              [pair.key.source, pair.value.source]
+            end
+          end
+
+          options = options.map { |o| extra_options(o) }
+          options.all?(options.first)
+        end
+
+        def extra_options(options)
+          options.except(*VALIDATION_KEYS)
+        end
+
+        def attribute_node(validation_node, attribute_name)
+          validation_node.arguments[0..-2].find do |argument|
+            argument.value.to_s == attribute_name.to_s
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -134,6 +134,7 @@
 require_relative 'rails/unused_ignored_columns'
 require_relative 'rails/unused_render_content'
 require_relative 'rails/validation'
+require_relative 'rails/validations_grouping'
 require_relative 'rails/where_equals'
 require_relative 'rails/where_exists'
 require_relative 'rails/where_missing'

spec/rubocop/cop/rails/validations_grouping_spec.rb

@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::ValidationsGrouping, :config do
+  let(:cop_config) { { 'SingleAttributeValidations' => false } }
+
+  it 'registers an offense when attribute validations are split' do
+    expect_offense(<<~RUBY)
+      validates :name, :email, presence: true
+                       ^^^^^^ Group together all email validations.
+      validates :email, format: { with: /@/ }
+                ^^^^^^ Group together all email validations.
+    RUBY
+  end
+
+  it 'does not register an offense when attribute validations are grouped' do
+    expect_no_offenses(<<~RUBY)
+      validates :email, presence: true, format: { with: /@/ }
+    RUBY
+  end
+
+  it 'does not register an offense for not-literal attribute name validations' do
+    expect_no_offenses(<<~RUBY)
+      validates attribute, presence: true
+      validates attribute, format: { with: /@/ }
+    RUBY
+  end
+
+  it 'does not register an offense when attribute validations have different configuration options' do
+    expect_no_offenses(<<~RUBY)
+      validates :email, presence: true, if: :some_condition?
+      validates :email, format: { with: /@/ }
+    RUBY
+  end
+
+  it 'registers an offense when attribute validations have same configuration options' do
+    expect_offense(<<~RUBY)
+      validates :email, presence: true, if: :some_condition?
+                ^^^^^^ Group together all email validations.
+      validates :email, format: { with: /@/ }, if: :some_condition?
+                ^^^^^^ Group together all email validations.
+    RUBY
+  end
+
+  it 'registers an offense when attribute validations have different validation options' do
+    expect_offense(<<~RUBY)
+      validates :email, format: { with: /foo/ }
+                ^^^^^^ Group together all email validations.
+      validates :email, format: { with: /bar/ }
+                ^^^^^^ Group together all email validations.
+    RUBY
+  end
+
+  context 'with conditionals' do
+    it 'registers an offense when attribute validations within conditional are split' do
+      expect_offense(<<~RUBY)
+        validates :email, uniqueness: true
+
+        if condition?
+          validates :name, :email, presence: true
+                           ^^^^^^ Group together all email validations.
+          validates :email, format: { with: /@/ }
+                    ^^^^^^ Group together all email validations.
+        end
+      RUBY
+    end
+
+    it 'registers an offense when attribute validations outside conditional are split' do
+      expect_offense(<<~RUBY)
+        validates :name, :email, presence: true
+                         ^^^^^^ Group together all email validations.
+        validates :email, format: { with: /@/ }
+                  ^^^^^^ Group together all email validations.
+
+        if condition?
+          validates :email, uniqueness: true
+        end
+      RUBY
+    end
+  end
+
+  context 'SingleAttributeValidations is used' do
+    let(:cop_config) { { 'SingleAttributeValidations' => true } }
+
+    it 'registers and offense for multiple attributes per validation' do
+      expect_offense(<<~RUBY)
+        validates :name, :email, presence: true
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Specify single attribute per validation.
+        validates :name, :email, presence: true, if: :some_condition?
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Specify single attribute per validation.
+      RUBY
+    end
+  end
+end