Skip to content

Enum implementation overrides custom toString #7227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cronokirby opened this issue Sep 14, 2019 · 14 comments · Fixed by #9549
Closed

Enum implementation overrides custom toString #7227

cronokirby opened this issue Sep 14, 2019 · 14 comments · Fixed by #9549

Comments

@cronokirby
Copy link

When overriding toStringin an enum, simple cases end up using their name instead of this new implementation.

minimized code

enum E:
  case A
  override def toString: String = "overriden"

println(E.A)

expectation

overriden

result

A

Note that adding an argument to that enum case will return the right result again:

enum E:
  case A(arg: Unit)
  override def toString: String = "overriden"

println(E.A)

This prints overriden as expected.

This has something to do with the desugaring of enums, and the generation of a default toString method.

@OlivierBlanvillain
Copy link
Contributor

This might be working as intended (which doesn't mean it isn't an issue...).

You example is overriding the toString method from trait E. As explain in the enum spec, parametrized enum desugar to case classes while parameterless enums desugar to their own custom thing. Case classes get a default toString method unless they already have/inherit a toString method other than the one from Object. On the other hand, the "custom thing" that parameterless enums desugar to unconditionally override toString.

@cronokirby
Copy link
Author

Yeah it does make sense considering how the implementation is.
I do think it does make it a somewhat leaky abstraction, because I need to reason about how the implementation of enum happens to work in order to understand things like this.

@OlivierBlanvillain
Copy link
Contributor

I do think it does make it a somewhat leaky abstraction, because I need to reason about how the implementation of enum happens to work in order to understand things like this.

Totally agree, I think having enum specified as a desugaring step is not really ideal...

@odersky
Copy link
Contributor

odersky commented Sep 27, 2019

Ideal or not, that's how enums are currently specified. So far there is a better alternative.

@odersky odersky closed this as completed Sep 27, 2019
@OlivierBlanvillain
Copy link
Contributor

Could we fix this issue by desugaring parameter-less enums to case objects instead of vals?

@anatoliykmetyuk
Copy link
Contributor

I don't think it is about vals vs case objects. Translation specifies that a case of an enum is an instance of an anonymous class extending the parent enum and overriding its toString method. The same can be done via an object but to change the semantics we need to change the spec of what an enum case is translated to.

@odersky
Copy link
Contributor

odersky commented Sep 30, 2019

Yes. vals and objects behave the same here. It makes no difference wether I write case A (which is implemented by a val) or case A extends E (which is implemented by an object). But for consistency it would be good if toString was not affected by whether a case is parameterized or not. Right now we get in

enum E {
  case A
  case B()
  override def toString: String = "overriden"

println(E.A)    // --> A
println(E.B())  // --> overridden

It would be good if we could change everything to overridden. This means we need a separate method to retrieve the name of an enum value; toString will not work in this role anymore.

So, to fix this, we need a redesign of (this aspect of) enums, and an implementation.

@anatoliykmetyuk
Copy link
Contributor

We should reconsider adding name methods to Scala enums

@bishabosha bishabosha mentioned this issue Nov 5, 2019
22 tasks
@viktorklang
Copy link
Contributor

@anatoliykmetyuk Indeed. And make the toString final and return the value of name?

bishabosha added a commit to dotty-staging/dotty that referenced this issue Aug 13, 2020
productPrefix is now overriden using the enum constant's name and
used in the by-name lookup in EnumValues.

java based enum values are optimised so that productPrefix will
forward to .name in the simple enum case, avoiding an extra field
bishabosha added a commit to dotty-staging/dotty that referenced this issue Aug 13, 2020
productPrefix is now overriden using the enum constant's name and
used in the by-name lookup in EnumValues.

java based enum values are optimised so that productPrefix will
forward to .name in the simple enum case, avoiding an extra field
bishabosha added a commit to dotty-staging/dotty that referenced this issue Aug 13, 2020
productPrefix is now overriden using the enum constant's name and
used in the by-name lookup in EnumValues.

java based enum values are optimised so that productPrefix will
forward to .name in the simple enum case, avoiding an extra field
@bishabosha
Copy link
Member

my current idea is since scala.Enum extends Product we can just use productPrefix instead of adding name

@anatoliykmetyuk
Copy link
Contributor

my current idea is since scala.Enum extends Product we can just use productPrefix instead of adding name

I don't think that's a good idea. While reasonable from DRY and engineering perspective, it adds a mental load on the programmer's mind. Now, to properly use enums, we need to keep in mind that they are products. For new programmers, this can be an extra learning curve when learning enums.

@bishabosha
Copy link
Member

@anatoliykmetyuk Thanks for your advice, I've said in this comment why at least the name method is difficult to add:
#9549 (comment)

@bishabosha
Copy link
Member

as per #9549 (comment) I can add an enumLabel method which should hopefully be simple to remember and not clash with any fields in class cases

@TheElectronWill
Copy link
Contributor

I like enumLabel, I think that's a good name. Thanks for implementing it!

bishabosha added a commit to dotty-staging/dotty that referenced this issue Aug 18, 2020
productPrefix is now overriden using the enum constant's name and
used in the by-name lookup in EnumValues.

java based enum values are optimised so that productPrefix will
forward to .name in the simple enum case, avoiding an extra field
bishabosha added a commit to dotty-staging/dotty that referenced this issue Aug 19, 2020
productPrefix is now overriden using the enum constant's name and
used in the by-name lookup in EnumValues.

java based enum values are optimised so that productPrefix will
forward to .name in the simple enum case, avoiding an extra field
bishabosha added a commit that referenced this issue Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants