From af86285eba586a8cac7219d10776cc7c76ba7841 Mon Sep 17 00:00:00 2001
From: Emily Samp <emily.samp@shopify.com>
Date: Thu, 18 Apr 2024 16:31:23 -0500
Subject: [PATCH] Introduce `ForbidComparableTEnum` cop

This cop adds an offense whenever `Comparable` is included in a `T::Enum` class because it isn't performant (default implementation is about 8x as slow as comparing between constants).
---
 config/default.yml                            |  5 ++
 .../sorbet/t_enum/forbid_comparable_t_enum.rb | 44 +++++++++++++++
 lib/rubocop/cop/sorbet_cops.rb                |  1 +
 .../t_enum/forbid_comparable_t_enum_spec.rb   | 55 +++++++++++++++++++
 4 files changed, 105 insertions(+)
 create mode 100644 lib/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum.rb
 create mode 100644 spec/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum_spec.rb

diff --git a/config/default.yml b/config/default.yml
index 07780afb..ddbaef59 100644
--- a/config/default.yml
+++ b/config/default.yml
@@ -317,3 +317,8 @@ Sorbet/MultipleTEnumValues:
   Description: 'Ensures that all `T::Enum`s have multiple values.'
   Enabled: true
   VersionAdded: 0.8.2
+
+Sorbet/ForbidComparableTEnum:
+  Description: 'Disallows including the `Comparable` module in a `T::Enum`.'
+  Enabled: true
+  VersionAdded: 0.8.2
diff --git a/lib/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum.rb b/lib/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum.rb
new file mode 100644
index 00000000..049a0eb1
--- /dev/null
+++ b/lib/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Sorbet
+      # Disallow including the `Comparable` module in `T::Enum`.
+      #
+      # @example
+      #
+      #  # bad
+      #  class Priority < T::Enum
+      #    include Comparable
+      #
+      #    enums do
+      #      High = new(3)
+      #      Medium = new(2)
+      #      Low = new(1)
+      #    end
+      #
+      #    def <=>(other)
+      #      serialize <=> other.serialize
+      #    end
+      #  end
+      class ForbidComparableTEnum < Base
+        include TEnum
+
+        MSG = "Do not use `T::Enum` as a comparable object because of significant performance overhead."
+
+        RESTRICT_ON_SEND = [:include, :prepend].freeze
+
+        # @!method mix_in_comparable?(node)
+        def_node_matcher :mix_in_comparable?, <<~PATTERN
+          (send nil? {:include | :prepend} (const nil? :Comparable))
+        PATTERN
+
+        def on_send(node)
+          return unless in_t_enum_class? && mix_in_comparable?(node)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/rubocop/cop/sorbet_cops.rb b/lib/rubocop/cop/sorbet_cops.rb
index 29ada026..ebd0280e 100644
--- a/lib/rubocop/cop/sorbet_cops.rb
+++ b/lib/rubocop/cop/sorbet_cops.rb
@@ -43,6 +43,7 @@
 require_relative "sorbet/sigils/enforce_sigil_order"
 require_relative "sorbet/sigils/enforce_single_sigil"
 
+require_relative "sorbet/t_enum/forbid_comparable_t_enum"
 require_relative "sorbet/t_enum/multiple_t_enum_values"
 
 require_relative "sorbet/mutable_constant_sorbet_aware_behaviour"
diff --git a/spec/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum_spec.rb b/spec/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum_spec.rb
new file mode 100644
index 00000000..67a6f0d6
--- /dev/null
+++ b/spec/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+RSpec.describe(RuboCop::Cop::Sorbet::ForbidComparableTEnum, :config) do
+  it "registers an offense when T::Enum includes Comparable" do
+    expect_offense(<<~RUBY)
+      class MyEnum < T::Enum
+        include Comparable
+        ^^^^^^^^^^^^^^^^^^ Do not use `T::Enum` as a comparable object because of significant performance overhead.
+      end
+    RUBY
+  end
+
+  it "registers an offense when T::Enum prepends Comparable" do
+    expect_offense(<<~RUBY)
+      class MyEnum < T::Enum
+        prepend Comparable
+        ^^^^^^^^^^^^^^^^^^ Do not use `T::Enum` as a comparable object because of significant performance overhead.
+      end
+    RUBY
+  end
+
+  it "does not register an offense when T::Enum includes other modules" do
+    expect_no_offenses(<<~RUBY)
+      class MyEnum < T::Enum
+        include T::Sig
+      end
+    RUBY
+  end
+
+  it "does not register an offense when T::Enum includes no other modules" do
+    expect_no_offenses(<<~RUBY)
+      class MyEnum < T::Enum; end
+    RUBY
+  end
+
+  it "does not register an offense when Comparable is included in a nested, non T::Enum class" do
+    expect_no_offenses(<<~RUBY)
+      class MyEnum < T::Enum
+        class Foo
+          include Comparable
+        end
+      end
+    RUBY
+  end
+
+  it "does not register an offense when Comparable is prepended in a nested, non T::Enum class" do
+    expect_no_offenses(<<~RUBY)
+      class MyEnum < T::Enum
+        class Foo
+          prepend Comparable
+        end
+      end
+    RUBY
+  end
+end