Add InOrderOf cop

Author: davidenglishmusic

changelog/new_in_order_of_cop.md

@@ -0,0 +1 @@
+* [#1554](https://github.com/rubocop/rubocop-rails/pull/1554): Add new `Rails/InOrderOf` cop to suggest using `in_order_of` for ordering instead of manually constructing SQL. ([@davidenglishmusic][])

config/default.yml

@@ -665,6 +665,12 @@ Rails/IgnoredSkipActionFilterOption:
     - '**/app/controllers/**/*.rb'
     - '**/app/mailers/**/*.rb'
 
+Rails/InOrderOf:
+  Description: 'Use `in_order_of` for ordering instead of manually constructing SQL.'
+  Reference: 'https://api.rubyonrails.org/classes/ActiveRecord/QueryMethods.html#method-i-in_order_of'
+  Enabled: pending
+  VersionAdded: '<<next>>'
+
 Rails/IndexBy:
   Description: 'Prefer `index_by` over `each_with_object`, `to_h`, or `map`.'
   Enabled: true

lib/rubocop/cop/rails/in_order_of.rb

@@ -0,0 +1,107 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Checks for manual construction of ordering logic and suggests using `in_order_of` instead.
+      #
+      # `in_order_of` was introduced in Rails 7.0 to allow custom ordering of records
+      # without using raw SQL with `CASE` statements or `Arel.sql`.
+      #
+      # @example
+      #   # bad
+      #   Post.order(
+      #     Arel.sql(
+      #       <<~SQL
+      #         case status
+      #         when 'draft' then 1
+      #         when 'published' then 2
+      #         when 'archived' then 3
+      #         else 4 end
+      #       SQL
+      #     )
+      #   )
+      #
+      #   # good
+      #   Post.in_order_of(:status, %w[draft published archived])
+      #
+      #   # bad - using MySQL FIELD function
+      #   Post.order(
+      #     Arel.sql("FIELD(status, 'draft', 'published', 'archived')")
+      #   )
+      #
+      #   # good
+      #   Post.in_order_of(:status, %w[draft published archived])
+      #
+      #   # bad - with string interpolation
+      #   Post.order(Arel.sql("FIELD(status, '#{statuses.join("', '")}')"))
+      #
+      #   # good
+      #   Post.in_order_of(:status, statuses)
+      #
+      class InOrderOf < Base
+        extend TargetRailsVersion
+
+        minimum_target_rails_version 7.0
+
+        MSG = 'Use `in_order_of` for ordering instead of manually constructing SQL.'
+        RESTRICT_ON_SEND = %i[order].freeze
+
+        # Matches: case column when ... or case #{var} when ...
+        CASE_WHEN_PATTERN = /\bcase\s+(\w+|#\{[^}]+\})\s+when\b/im.freeze
+        # Matches: FIELD(column, ...)
+        FIELD_FUNCTION_PATTERN = /\bFIELD\s*\(/i.freeze
+
+        AREL_CONST_NAME = 'Arel'
+        HEREDOC_START = '<<'
+
+        def on_send(node)
+          return unless node.method?(:order)
+          return unless (sql_string = extract_sql_string(node))
+          return unless custom_order_sql?(sql_string)
+
+          add_offense(node.loc.selector)
+        end
+
+        private
+
+        def extract_sql_string(node)
+          sql_arg = sql_argument(node)
+          extract_string_content(sql_arg) if sql_arg
+        end
+
+        def sql_argument(node)
+          arg = node.first_argument
+          return unless arg&.send_type? && arg.method?(:sql)
+          return unless arel_const?(arg.receiver)
+
+          arg.first_argument if arg.first_argument&.type?(:str, :dstr)
+        end
+
+        def extract_string_content(sql_arg)
+          case sql_arg.type
+          when :str then sql_arg.str_content
+          when :dstr then extract_dstr_content(sql_arg)
+          end
+        end
+
+        def extract_dstr_content(sql_arg)
+          source = sql_arg.source
+          source.start_with?(HEREDOC_START) ? heredoc_content(sql_arg) : source[1...-1]
+        end
+
+        def heredoc_content(sql_arg)
+          sql_arg.each_child_node(:str).map(&:str_content).join
+        end
+
+        def arel_const?(node)
+          node&.const_type? && node.const_name == AREL_CONST_NAME
+        end
+
+        def custom_order_sql?(sql_string)
+          sql_string.match?(CASE_WHEN_PATTERN) || sql_string.match?(FIELD_FUNCTION_PATTERN)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -73,6 +73,7 @@
 require_relative 'rails/i18n_locale_texts'
 require_relative 'rails/ignored_columns_assignment'
 require_relative 'rails/ignored_skip_action_filter_option'
+require_relative 'rails/in_order_of'
 require_relative 'rails/index_by'
 require_relative 'rails/index_with'
 require_relative 'rails/inquiry'

spec/rubocop/cop/rails/in_order_of_spec.rb

@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::InOrderOf, :config do
+  context 'Rails >= 7.0', :rails70 do
+    context 'when using Arel.sql with CASE statement' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          Post.order(
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+            Arel.sql(
+              <<~SQL
+                case status
+                when 'draft' then 1
+                when 'published' then 2
+                when 'archived' then 3
+                else 4 end
+              SQL
+            )
+          )
+        RUBY
+      end
+    end
+
+    context 'when using Arel.sql with inline CASE statement' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          User.order(Arel.sql("CASE status WHEN 'active' THEN 1 ELSE 2 END"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+    end
+
+    context 'when using Arel.sql with FIELD function' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          Post.order(Arel.sql("FIELD(status, 'draft', 'published', 'archived')"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+    end
+
+    context 'when using ::Arel.sql with CASE statement' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          Post.order(::Arel.sql("case priority when 'high' then 1 else 2 end"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+    end
+
+    context 'when using Arel.sql without CASE or FIELD' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          User.order(Arel.sql("created_at DESC"))
+        RUBY
+      end
+    end
+
+    context 'when using regular order methods' do
+      it 'does not register an offense with symbol' do
+        expect_no_offenses(<<~RUBY)
+          User.order(:created_at)
+        RUBY
+      end
+
+      it 'does not register an offense with hash' do
+        expect_no_offenses(<<~RUBY)
+          User.order(created_at: :desc)
+        RUBY
+      end
+
+      it 'does not register an offense with string' do
+        expect_no_offenses(<<~RUBY)
+          User.order('created_at DESC')
+        RUBY
+      end
+    end
+
+    context 'when using in_order_of' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          Post.in_order_of(:status, %w[draft published archived])
+        RUBY
+      end
+    end
+
+    context 'when using Arel.sql with FIELD and string interpolation' do
+      it 'registers an offense' do
+        expect_offense(<<~'RUBY')
+          TITLES = ['foo', 'bar']
+          Post.order(Arel.sql("FIELD(title,'#{TITLES.join(",")}')"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+
+      it 'registers an offense with join using single quotes' do
+        expect_offense(<<~'RUBY')
+          Post.order(Arel.sql("FIELD(status,'#{UNPUBLISHED_STATES.join("','")}')"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+
+      it 'registers an offense with exact user pattern' do
+        expect_offense(<<~'RUBY')
+          TITLES = ['foo', 'bar']
+          Post.order(Arel.sql("FIELD(title,'#{TITLES.join("','")}')"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+    end
+
+    context 'when using Arel.sql with CASE and string interpolation' do
+      it 'registers an offense' do
+        expect_offense(<<~'RUBY')
+          column = 'status'
+          User.order(Arel.sql("CASE #{column} WHEN 'active' THEN 1 ELSE 2 END"))
+               ^^^^^ Use `in_order_of` for ordering instead of manually constructing SQL.
+        RUBY
+      end
+    end
+  end
+
+  context 'Rails <= 6.1', :rails61 do
+    it 'does not register an offense for Arel.sql with CASE' do
+      expect_no_offenses(<<~RUBY)
+        Post.order(Arel.sql("case status when 'draft' then 1 else 2 end"))
+      RUBY
+    end
+  end
+end