Skip to content
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

Add unbound placeholder parameter message. #1618

Conversation

rubenpieters
Copy link
Contributor

Part of #1589
Addition of message for the 'unbound placeholder parameter' error message (Parsers.scala 337).
I tried to check what is available in the vd ValDef in checkNoEscapingPlaceholders (Parsers.scala 330), but I couldn't really find anything which could be used to enhance the error message.

|Note that this use of `_` is not placeholder syntax,
|but an uninitialized var definition
""".stripMargin
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great job generalizing this error!

For the explanation I would inline the values codeUnboundInBlock, codeBoundInBlock, codeUnboundVal and codeVar, which would get the error message down from 55 lines to roughly 30.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that makes sense, the PR has been updated.

@rubenpieters rubenpieters force-pushed the issue-1589-unbound-placeholder-parameter-error-message branch from 54a1ced to 726b129 Compare October 21, 2016 16:35
@felixmulder
Copy link
Contributor

Looks good @rubenpieters - could you bump the errorId to 16? That way I merge #1615 and then yours :)

@rubenpieters rubenpieters force-pushed the issue-1589-unbound-placeholder-parameter-error-message branch from 726b129 to 273df6b Compare October 21, 2016 19:44
@rubenpieters
Copy link
Contributor Author

Ok done.

@felixmulder
Copy link
Contributor

@rubenpieters - unfortunately merging the other PR caused this to become a merge conflict. So it needs to be rebased. Sorry about that :(

@rubenpieters rubenpieters force-pushed the issue-1589-unbound-placeholder-parameter-error-message branch from 273df6b to bf80147 Compare October 22, 2016 06:38
@rubenpieters
Copy link
Contributor Author

No problem, the conflicts have been resolved.

@felixmulder felixmulder merged commit 0e3b4aa into scala:master Oct 22, 2016
@felixmulder
Copy link
Contributor

Merged! Thanks for your patience 🎉

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.

2 participants