Add an error message class for sealed classes #3473
Conversation
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 ("Added" instead of "Add")
- 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! ☀️
maseev
changed the title
|
I'm just wondering, isn't this supposed to be
and not
in the 6th rule of commit messages? According to this rule, you need to use the present simple tense and not the past tense for your commit messages. Otherwise, it would be an indicative mood which is all about reporting facts. |
Yes. Feel free to submit a PR. The message is in |
|
|
||
| case class UnableToExtendSealedClass(pclazz: Symbol)(implicit ctx: Context) extends Message(UnableToExtendSealedClassID) { | ||
| val kind = "Syntax" | ||
| val msg = hl"cannot extend sealed $pclazz in different compilation unit" |
There was a problem hiding this comment.
I would refactor that and the extended message to only talk about files instead. It seams unnecessary to talk about compilation units here.
Here is what we have in the spec if that helps:
The sealed modifier applies to class definitions. A sealed class may not be directly inherited, except if the inheriting template is defined in the same source file as the inherited class. However, subclasses of a sealed class can be inherited anywhere.
| |sealed class A | ||
| |class B extends A""".stripMargin | ||
|
|
||
| hl"""A ${"sealed"} keyword makes sure that all classes that extend a sealed class are |
There was a problem hiding this comment.
TBH I'm not convinced a super long description with correct/incorrect examples is helpful here. The top SO answer just goes straight to the point:
A sealed trait can be extended only in the same file as its declaration.
|
@OlivierBlanvillain PTAL |
|
👍, thanks! |
|
@OlivierBlanvillain, @allanrenucci sorry, I forgot to pull your changes first before doing a rebase. I applied the same changes you did to the message and explanation fields. |
|
No worries, thank you for the PR! |
This PR is related to #1589. Specifically, it adds a new error message class for the following case -
Namer.scala:880.Unfortunately, this PR doesn't have a unit test because I wasn't able to find a way to trigger this particular behavior. It seems to me that it's impossible at the moment to check some code which is separated into two different compilation units inside a unit test.
Maybe I'm simply missing something.