Discourage misuses of T::Enum

[egiurleo]

While T::Enum provides a low-effort, type-safe way to construct an exhaustive set of values, it is also less performant than implementing the same functionality using data structures from Ruby (e.g. list of constants).

rubocop-sorbet should discourage using T::Enum in ways that exacerbates performance issues or doesn't use T::Enum for its intended purpose.

Potential cops include:

• Convert any T::Enum with one value into a constant
• Disallow including the Comparable module in T::Enum (comparing T::Enum values has particularly poor performance)
• Disallow case statements with more than a certain number of T::Enum values (motivation)

---

EDIT (2024-04-22): I decided not to create a cop forbidding case statements above a certain size.

After running some benchmarks, it seems that, at runtime, case statements have exponentially decreasing performance whether they're being used with T::Enum values or Ruby constants, and when statically type checking, time spent increases linearly. There isn't an obvious point at which performance drops off in either of these cases, making any cut off we assign totally arbitrary. We can revisit this issue if it comes up in the future.

---

EDIT (2024-04-23): I've just verified that these results are the same with or without YJIT on (thanks @amomchilov for prompting me to do that).

[amomchilov]

it is also less performant than implementing the same functionality using data structures from Ruby (e.g. list of constants)

The most common operation done with enum values is to compare them (either directly with ==, or indirectly via case/when/===)

T::Enum#== and T::Enum#=== can be made just as fast as comparing string/symbol constants (in either case, it just uses the identity-based comparison inherited from Object#==).

I explore this in https://github.com/Shopify/shopify-types/pull/454/files , but had trouble getting CI to validate my changes. Perhaps that's worth exploring first?

I think that change (having #== and #=== only be overridden when T::Configuration.legacy_t_enum_migration_mode? is set) can even be upstreamed into sorbet/sorbet.

[egiurleo]

@amomchilov That's really interesting! I will take a look at this as it ties in well with our Sorbet performance project. I wonder if we could apply the same logic to the <=> method? I did some benchmarks where </> comparisons were 8x slower with T::Enum values than constants, which was pretty wild to me.

[amomchilov]

Hmm, when is <=> even used? T::Enum doesn't even include Comparable