Use enum instead of module for HTTP::WebSocket::CloseCodes

[Sija]

Followup to #8975

[bcardiff]

I think the problem with close codes is that there are lots of values. Is not the same situation as http status codes which are fixed by the rfc.

I'm fine with using enums, if the option for using integers are still available. And if having those overloads does not prevent the autocasting feature to work on enums.

Ref: https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent#Status_codes

[Sija]

I'm fine with using enums, if the option for using integers are still available. And if having those overloads does not prevent the autocasting feature to work on enums.

@bcardiff It is available, and it has to be for http status codes to work as well, since there are many unassigned ones that could be used by the developers anyway.

autocasting will obviously work only for the assigned ones.

[straight-shoota]

Is not the same situation as http status codes which are fixed by the rfc.

HTTP status codes are in fact extensible and not limited to registered ones (see https://tools.ietf.org/html/rfc2616#section-6.1.1). So it's one and the same thing.

autocasting will obviously work only for the assigned ones.

I think a problem could be when we need to know all possible enum values at compile time. That's at least a possible scenario we should be aware of.

But for now it is IMO fine to apply this change because the close codes are conceptually identical to HTTP status codes (registered set, but extensible). If the implementation of enum changes at some point to only allow pre-known values, we would need to figure out something else anyway. And probably also take these codes into consideration.

[Blacksmoke16]

If there is a valid range like there is with HTTP::Status, we could do something like https://github.com/crystal-lang/crystal/blob/master/src/http/status.cr#L71-L76 to validate the value is within that range.

[Sija]

@Blacksmoke16 Good idea! I've force-pushed changes with the check.

[asterite]

My idea is to force enums to have valid values, always. Right now Enum.new and also when returning enums from C bindings don't have this check, but we should have them.

Then, we could introduce an @[OpenEnum] or similar annotation to say "This enum can have more values than those listed", and then the compiler would always require an else when doing case over such enum.

[Sija]

@asterite Sounds good, but please open an RFC issue about that, to not to stall this PR.

[asterite]

Oh, of course. It's an idea, it's not necessary for this PR.

[RX14]

I'm not sure this has any actual benefits...

[Sija]

Anything left to do in here?

[Sija]

All comments addressed.