Fix compiler crash for repeated Enums

[snadorp]

Fix: #4470

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

1. Separate subject from body with a blank line
2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
3. Limit the subject line to 72 characters
4. Capitalize the subject line
5. Do not end the subject line with a period
6. Use the imperative mood in the subject line ("Add" instead of "Added")
7. Wrap the body at 80 characters
8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

[snadorp]

CLA signed.

[OlivierBlanvillain]

@smarter wdyt?

[smarter]

• Doing the check in expandSimpleEnumCase is too late since it doesn't cover enum cases with parents:

enum Maybe[T] {
  case Foo extends Maybe[Int]
}

enum Maybe[T] {
  case Foo extends Maybe[Int]
}

• we probably should add an assert(ctx.reporter.errorsReported) since we should never get to this case unless the "class has already been compiled" error has been reported.

[snadorp]

@smarter Sadly assert(ctx.reporter.errorsReported) does not work.
I've added your example as test case and fixed the crash.

[smarter]

Instead of duplicating the check, you should be able to have only one check done earlier, in expandEnumModule

[snadorp]

You are right, I'll look into it.

[allanrenucci]

Here is another test case that crashes on master:

object DuplicatedEnum {
  enum Maybe[+T] {
    case Some(x: T)
  }

  enum Maybe[+T] {
    case Some(x: T)
  }
}

[allanrenucci]

Our convention is to name our regression test files like i4470.scala. If you have mutiple test cases: i4470a.scala, i4470b.scala

[snadorp]

Alrighty, will rename the files thanks for the hint.

[snadorp]

@allanrenucci will look into it.

[snadorp]

From my perspective there's no simple way of getting rid of the 2 ifs in expandEnumModule and expandSimpleEnumCase as they are both called from Desugar on different occasions.
Expanding only in cases when there's no compiler error breaks a couple of unit test and is therefore not the wanted behaviour.
Any other ideas?

[allanrenucci]

Thanks @snadorp 🎉