Skip to content

Commit

Permalink
Introduce ForbidComparableTEnum cop
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
egiurleo committed Apr 19, 2024
1 parent 603bdea commit af86285
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 0 deletions.
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
44 changes: 44 additions & 0 deletions lib/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lib/rubocop/cop/sorbet_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
55 changes: 55 additions & 0 deletions spec/rubocop/cop/sorbet/t_enum/forbid_comparable_t_enum_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit af86285

Please sign in to comment.