-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Added error message for Symbol is not a value #1589 #3746
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
Conversation
benkobalog
commented
Jan 4, 2018
•
edited
Loading
edited
- Added SymbolIsNotAValue error Message class
- Substituted the original error message at Checking.scala:526 with the new class and added a test
- Typo fix at Checking.scala:211
1. Added SymbolIsNotAValue error Message class 2. Substituted the original error message with the new class and added a test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Have an awesome day! ☀️
@@ -208,7 +208,7 @@ object messages { | |||
|
|||
case class WildcardOnTypeArgumentNotAllowedOnNew()(implicit ctx: Context) | |||
extends Message(WildcardOnTypeArgumentNotAllowedOnNewID) { | |||
val kind = "syntax" | |||
val kind = "Syntax" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the only place where this string started with a lowercase. I assumed it was a typo, if somebody could confirm it, it would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good
val msg = hl"${symbol.show} is not a value" | ||
val kind = "Type Mismatch" | ||
val explanation = | ||
hl"Scala or Java packages cannot be assigned to a value" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scala packages can be used as values if they have a package object.
Java statics and packages cannot be used as a value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should be the explanation then?
What are Java statics in this context? I guess not static fields because """object O {val a = Math.PI}""" compiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the relevant conversation. I will write a better explanation based on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the explanation.
@@ -2064,4 +2064,11 @@ object messages { | |||
val explanation = | |||
hl"An object that contains ${"@static"} members must have a companion class." | |||
} | |||
|
|||
case class SymbolIsNotAValue(symbol: Symbol)(implicit ctx: Context) extends Message(SymbolIsNotAValueID) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SymbolIsNotAValue
-> JavaSymbolIsNotAValue
Same for ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the class name, ID and test name too.
@@ -2064,4 +2064,11 @@ object messages { | |||
val explanation = | |||
hl"An object that contains ${"@static"} members must have a companion class." | |||
} | |||
|
|||
case class SymbolIsNotAValue(symbol: Symbol)(implicit ctx: Context) extends Message(SymbolIsNotAValueID) { | |||
val msg = hl"${symbol.show} is not a value" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need show here $symbol
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the show method.
I will close this pull request and open a new one, because I couldn't properly resolve the conflicts (I'm new to this part of git) |