Fix #3784 : add an enum for Error kind #3785
Conversation
| @@ -0,0 +1,28 @@ | |||
| package dotty.tools.dotc.reporting.diagnostic; | |||
|
|
|||
| public enum ErrorKind { | |||
There was a problem hiding this comment.
It would be better to have this in Scala.
class ErrorKind(name: String) {
def toString(): String = name
}
object ErrorKinds {
val NO_KIND = new ErrorKind("")
val COMPATIBILITY = new ErrorKind("Compatibility")
val DEFINITION_NOT_FOUND = new ErrorKind("Definition Not Found")
...
}
There was a problem hiding this comment.
We are already using java enums for ErrorMessageID. I think it is fine to stick to java enum until we migrate everything to Dotty enum.
I would however use Scala naming convention: NoKind instead of NO_KIND
There was a problem hiding this comment.
Ok.
But move NoKind to the first position.
| @@ -117,7 +117,7 @@ class ExtendMessage(_msg: () => Message)(f: String => String) { self => | |||
| /** The fallback `Message` containing no explanation and having no `kind` */ | |||
| class NoExplanation(val msg: String) extends Message(ErrorMessageID.NoExplanationID) { | |||
| val explanation = "" | |||
| val kind = "" | |||
| val kind = ErrorKind.NO_KIND | |||
There was a problem hiding this comment.
With the change I proposed, this should become val kind = NO_KIND
|
Otherwise it looks good |
|
Ok thanks, I wasn't sure about the Java enum vs. Scala object vs. the new enum system coming. I read the full discussion on #1970 and I guess it ended up confusing me ^^ I changed it. |
| @@ -0,0 +1,20 @@ | |||
| package dotty.tools.dotc.reporting.diagnostic | |||
|
|
|||
| sealed case class ErrorCategory(name: String) { | |||
There was a problem hiding this comment.
If you intend to make it behave like an enum, I suggest you use this pattern:
sealed abstract class ErrorCategory(category: String) {
...
}
object ErrorCategory {
val NoKind = new ErrorCategory("") {}
...
}| override def toString: String = name | ||
| } | ||
| object ErrorCategories { | ||
| val COMPATIBILITY = ErrorCategory("Compatibility") |
There was a problem hiding this comment.
Please use Scala naming convention: NoKind instead of NO_KIND. Also I would put NoKind at the top.
There was a problem hiding this comment.
I can definitely rename everything, but it unfortunately clashes with the case class PatternMatchExhaustivity(uncovered: String) (both the message and kind clash together). What would you recommend ? Keep the same name but prefix like val kind = ErrorCategory.PatternMatchExhaustivity or rename the enum members ? (if so, how ?). Thanks for your insights
There was a problem hiding this comment.
Yes you can disambiguate with val kind = ErrorCategory.PatternMatchExhaustivity
|
That should be OK now. |
|
@allanrenucci is it OK for you now ? The longer this PR is opened, the more conflicts with people implementing new error messages we may get :) |
|
Sorry I must have messed up with the merge... I have a lot of compiler errors now. |
|
You can reopen a new PR if you feel like it is easier |
|
Let's revisit using Dotty enums once we have the bootstrap (hopefully real soon now!) |
|
Good, and thanks. |
ping @aesteve |
|
I'm giving it a try with a proper Scala enum here but tests are failing with |
|
@aesteve Is this PR still active, or do you want to close it? |
|
Hello and sorry. Sorry once again, never found the actual time to do this properly. |
|
Don't worry -- thanks for your quick feedback. |
First step to give newcomers a better view of the different kind of errors the compiler may throw.
Having a quick glimpse at existing error kinds should help picking the right one (or adding a new one).