Add enum example in documentation and a test for 'case class cannot extend enum' #5014
Conversation
…ass cannot extend enum'
There was a problem hiding this comment.
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:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
skvithalani
changed the title
There was a problem hiding this comment.
Thanks for the attempt! But we can't merge test cases that fail (see comments).
Sorry if my vague comment mislead you — I certainly did not mean "this is a good issue for novices" — we use the exp:novice or help wanted labels for that.
https://github.com/lampepfl/dotty/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22+
https://github.com/lampepfl/dotty/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22
| checkMessagesAfter(RefChecks.name) { | ||
| """ | ||
| | enum Foo {} | ||
| | case class Bar() extends Foo |
There was a problem hiding this comment.
No space after the | — this means code indented by one space.
| """ | ||
| | enum Foo {} | ||
| | enum Bar extends Foo {} | ||
| """.stripMargin |
There was a problem hiding this comment.
The best we can do there is place it where it isn't run, tho we usually keep those as issues. Writing good testcases is part of fixing an issue properly anyway.
To observe the failure, you can put the following in tests/neg/i5008.scala, where the // error marker (the space matters!) signals an expected error.
enum Foo {}
enum Bar extends Foo {} // errorBy our conventions, we can at best merge that in tests/pending/neg/i5008.scala, not sure what others think.
|
I tried to sign CLA and authorized dotty-bot to access my github but the process does not go through till the end and I get the error Invalid login at http://dotty-ci.epfl.ch/. I am assuming since I have already authorized the access, I don't have any more actions to do in terms of CLA. But the status shows CLA not signed. Am I missing something ? |
|
Hm, the CLA is indeed not signed yet (https://www.lightbend.com/contribute/cla/scala/check/skvithalani). Can you sign the Scala CLA from https://www.lightbend.com/contribute/cla/scala/sign? If you get to http://dotty-ci.epfl.ch you're trying to login to control our CI (which only committers can). |
|
Thanks. I signed it. |
| implicit val ctx: Context = ictx | ||
| assertMessageCount(1, messages) | ||
| val errorMsg = messages.head | ||
| assertEquals("normal case class Bar in package <empty> cannot extend an enum", errorMsg.msg) |
There was a problem hiding this comment.
FWIW: we usually don't test strings but that's the correct thing *as long as we don't switch to error messages here (#1589). @skvithalani are you interested in doing that in a followup? Accepting this meanwhile.
There was a problem hiding this comment.
Sure . I will take that up.
There was a problem hiding this comment.
Thanks! Beyond the pre-labeled issues, ones in parser/desugar tend to be more approachable.
|
Related: follow-ups in #5051. |
I tried to fix #5008 by referring the comment of @Blaisorblade here but am struggling a bit. I happen to play around Typer, Checking and RefChecks to be specific but no luck.
So, this pull request just captures the missing test of 'case class cannot extend enums' and an example addition in enum documentation.
Also, is it possible that I can fix the bug, if so, I would be happy to contribute.