Enum#inspect should be more specific

[straight-shoota]

Enum#inspect prints the name of the enum member, or the numerical value if it's unnamed. It is the same as Enum#to_s.

This is ambiguous when multiple enums share the same member names.

enum Foo
  One
end

enum Bar
  One
end

Foo::One # => One
Bar::One # => One

I think this is okay for #to_s but #inspect should be unambiguous as possible. Using the full class name should help to fix that.

Foo::One # => Foo::One
Bar::One # => Bar::One

Of course this is less succinct, but I think that's acceptable. #to_s would be the shorter, but less precise alternative.

For flag enum values, the fully qualified name does not need to be repeated for every member. Following the syntax of Enum.flags method works great for that:

FlagsEnum.flags(One | Two | Three) # => FlagsEnum.flags(One | Two | Three)

[straight-shoota]

A more succinct format with square brackets: FlagsEnum[One | Two | Three]

I think that's better because it conveys the same information with less space.
The result won't be valid code to recreate that enum value,¹ but that's acceptable.

This should also work great as a representation format for unnamed members (cf. #12883) which wouldn't be valid syntax for .flags.²

FlagsEnum::None                                         # => FlagsEnum::None
FlagsEnum::One                                          # => FlagsEnum::One
FlagsEnum.new(128)                                      # => FlagsEnum[128]
FlagsEnum::All                                          # => FlagsEnum[One | Two | Three]
(FlagsEnum::One | FlagsEnum::Two)                       # => FlagsEnum[One | Two]
(FlagsEnum::One | FlagsEnum.new(128))                   # => FlagsEnum[One | 128]
(FlagsEnum::One | FlagsEnum.new(8) | FlagsEnum.new(16)) # => FlagsEnum[One | 24]
(FlagsEnum::One | FlagsEnum::Two | FlagsEnum.new(16))   # => FlagsEnum[One | Two | 16]

Footnotes

1. We could consider adding Enum.[] exactly like this, basically as a replacement for Enum.flags because there the same benefit applies: it's more concise. ↩

2. Supporting numerical values could be a potential enhancement for .flags/.[] as well. ↩

[asterite]

I think separating elements with a comma instead of a pipe will allow implementing it as a method instead of a macro.

[straight-shoota]

@asterite Yes, we could use symbols for autocasting. I considered that as well. But that would potentially be ambiguous. The mapping between member name and symbol is not exactly 1:1. Symbol autocasting is case-insensitive, which would lead to problems when an enum contains members with the same letters but different casing.

/ref #11660

enum Foo
  Bar
  BAR
end
Foo.new(:BAR) # => Bar

An unambiguous literal representation of the member name is necessary for #inspect.

I suppose we could use the literal casing for #inspect. That should be unambiguous. The problem appears when parsing back the output as Crystal code. But producing valid code with potentially invalid semantics doesn't sound like a good idea.

[asterite]

Idea: disallow two enum members if their snake case lead to the same value.

[Sija]

Idea: disallow two enum members if their snake case lead to the same value.

@asterite Sounds good for a regular code, but what about lib bindings where you cannot control the member naming?

[asterite]

How can you not control that?

[straight-shoota]

Yeah, I think this should be controllable, though it means deviating from the original spelling of constants. Not sure if that would be a huge problem, but it causes some friction.

However, this uniqueness validation would definitely be a breaking change, so we couldn't enforce it short term.

So I'd leave that beside for now and just implement a format for #inspect that is unambiguous and can work with what we currently have: #12884 (comment)

[Sija]

Oh, well I guess you can reassign values, forgot about that 🤦‍♂️

[asterite]

In any case, it seems that to implement this it will require a macro, and separating values by commas will be easier to implement.

[straight-shoota]

Right, using a comma for simplicity makes sense.

[straight-shoota]

An edge case is a flag enum with a composite member:

@[Flags]
enum FlagsEnum
  One
  Two
  OneTwo = One | Two
end

FlagsEnum::OneTwo

#to_s currently returns "One | Two | OneTwo". But that's unnecessary. "OneTwo" should be enough. Similarly, for #inspect, it's "FlagsEnum::OneTwo.

A common use case for this is the autogenerated All member.