Add ForbidTEnum cop

[Morriar]

So we can deprecate usages of T::Enum.

[Morriar]

I'm not convinced it's the best way to represent this? Maybe @rafaelfranca has a better idea?

[vinistock]

This good section feels odd to me. Do we want to advise people to write their own enums like this one?

Or maybe I'm missing context. What are people intended to use in replacement for the enum?

[rafaelfranca]

class MyEnum
  A = "a"
  B = "b"
  C = "c"
end

[rafaelfranca]

Just to make clear, the problem with T::Enum is the runtime and cognitive overhead that it adds. People need to serialize, deserialize enum values, and I don't think any suggestion we make should add the same runtime overhead.

[Morriar]

This version doesn't play nicely with typing though.

You can't use the class type in your signature:

sig { params(x: MyEnum).void }
def foo(x); end

foo(MyEnum::A) # error: Expected MyEnum but found String for argument x

You can't use the constant union either:

sig { params(x: T.any(MyEnum::A, MyEnum::B, MyEnum::C)).void } # error: Constant MyEnum::A is not a class or type alias
def bar(x); end

bar(MyEnum::A)

Going with type alias doesn't restrain the value you can pass:

sig { params(x: T.any(MyEnum::A, MyEnum::B, MyEnum::C)).void }
def bar(x); end

bar("z") # shouldn't be accepted

Ideally we could make Sorbet support literal types for this:

sig { params(x: T.any("a", "b", "c")).void }
def qux(x); end

In the meantime, the version I proposed in the # good works with type checking and can be extended to also store a value if required. This could be used as an autocorrect to (almost) seamlessly replace current T::Enum usages.

As we currently do not provide an autocorrect, we could simply use your suggestion for documentation purpose and maybe add a note to look into Rails enums?

[ipvalverde]

But then adding a custom class to act as an enum would have the same drawbacks as using T::Enum, right? Even a bit worse because people would have to add their own serializers as needed.

[rafaelfranca]

This version doesn't play nicely with typing though.

Yes, and that is fine. We are losing some typing safety to avoid the runtime cost.

The signature for your example would be:

sig { params(x: String).void }
def foo(x); end

foo(MyEnum::A)

Ideally we could make Sorbet support literal types

Yes. If we can that would be great.

[rafaelfranca]

But then adding a custom class to act as an enum would have the same drawbacks as using T::Enum, right? Even a bit worse because people would have to add their own serializers as needed.

Yes, exactly, that is why I don't want people to use custom classes. Just use the raw values.

[vinistock]

Code looks good to me, just wondering about the recommendation for fixing the violations

[vinistock]

This good section feels odd to me. Do we want to advise people to write their own enums like this one?

Or maybe I'm missing context. What are people intended to use in replacement for the enum?

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.