Skip to content

Error on TypedefOrDcl #3325

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

Merged

Conversation

hermesespinola
Copy link
Contributor

Tackles #1589. More specifically, it provides a detailed explanation for the error message in Parsers.scala:2150 : "=', >:', or `<:' expected".

Copy link
Member

@dottybot dottybot left a 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! ☀️

@hermesespinola
Copy link
Contributor Author

hermesespinola commented Oct 14, 2017 via email

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test case?

hl"""`=', `>:', or `<:' expected"""

val explanation =
hl"""|In Scala, type parameters and abstract types may be constrained by a type bound.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove "In Scala"

extends Message(ExpectedTypeBoundOrEqualsID) {
val kind = "Syntax"
val msg =
hl"""`=', `>:', or `<:' expected"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use backquotes. Interpolated strings will be highlighted. For example here:

val msg = hl"${"="}, ${">:"}, or ${"<:"} expected"

@@ -2156,7 +2156,7 @@ object Parsers {
case SUPERTYPE | SUBTYPE | SEMI | NEWLINE | NEWLINES | COMMA | RBRACE | EOF =>
TypeDef(name, lambdaAbstract(tparams, typeBounds())).withMods(mods).setComment(in.getDocComment(start))
case _ =>
syntaxErrorOrIncomplete("`=', `>:', or `<:' expected")
syntaxErrorOrIncomplete(ExpectedTypeBoundOrEquals())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could display the faulty token it in the error message:

hl"... expected, but ${Tokens.showToken(found)} found"

Show faulty token in the error message
Don't use backquotes.
Remove "In Scala" in explanation message
@hermesespinola
Copy link
Contributor Author

hermesespinola commented Oct 18, 2017

@allanrenucci I changed what you requested and added a test at the end of ErrorMessagesTests.scala.
Is it ok now? :octocat:

""".stripMargin
}.expect { (ictx, messages) =>
implicit val ctx: Context = ictx
val defn = ictx.definitions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need defn here.

@hermesespinola
Copy link
Contributor Author

I think it's ok now.

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you 🎉

@allanrenucci allanrenucci merged commit 2e864c0 into scala:master Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants