Skip to content

a better error message for disallowed trailing commas/additional parameters after rest elements in function params#9046

Merged
nicolo-ribaudo merged 4 commits intobabel:masterfrom
morozRed:7460-add-a-better-error-message
Nov 21, 2018
Merged

a better error message for disallowed trailing commas/additional parameters after rest elements in function params#9046
nicolo-ribaudo merged 4 commits intobabel:masterfrom
morozRed:7460-add-a-better-error-message

Conversation

@morozRed
Copy link
Copy Markdown
Contributor

@morozRed morozRed commented Nov 20, 2018

Q                       A
Fixed Issues? Fixes #7460
Patch: Bug Fix? n
Major: Breaking Change? n
Minor: New Feature? n
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR changes error message when we had other parameters after rest element.

Previous behaviour:

Test:

(
    first,
    ...second,
    third
) => {};

Result

Unexpected token, expected ")"

New behaviour:

Test:

(
    first,
    ...second,
    third
) => {};

Result:

Rest parameter must be last formal parameter

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Nov 20, 2018 •
edited
Loading

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9419/

@morozRed morozRed mentioned this pull request Nov 20, 2018
Closed
@xtuc xtuc added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: parser labels Nov 20, 2018
xtuc
xtuc approved these changes Nov 20, 2018
View reviewed changes
Comment thread packages/babel-parser/src/parser/expression.js Outdated
) {
this.raise(
this.state.start,
"Rest parameter must be last formal parameter",
Copy link
Copy Markdown
Member

@xtuc xtuc Nov 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for your PR, It looks good to me.

At first the error mesage looked odd to me, but it's actually V8's one 👍

Copy link
Copy Markdown
Contributor Author

@morozRed morozRed Nov 20, 2018

Choose a reason for hiding this comment

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

@xtuc thanks! I'm glad that my first PR was not bad 😃 Yeah, I decided to get this message from V8.

nicolo-ribaudo
nicolo-ribaudo reviewed Nov 20, 2018
View reviewed changes
Comment thread packages/babel-parser/src/parser/expression.js Outdated
@existentialism existentialism added PR: Polish 💅 A type of pull request used for our changelog categories and removed PR: New Feature 🚀 A type of pull request used for our changelog categories labels Nov 20, 2018
@nicolo-ribaudo
Copy link
Copy Markdown
Member

nicolo-ribaudo commented Nov 20, 2018

Could you add a test for (...rest,) => {}?

@morozRed
Copy link
Copy Markdown
Contributor Author

morozRed commented Nov 20, 2018

@nicolo-ribaudo yeah, sure

@nicolo-ribaudo
Copy link
Copy Markdown
Member

nicolo-ribaudo commented Nov 21, 2018

Thank you!

@nicolo-ribaudo nicolo-ribaudo merged commit 445b141 into babel:master Nov 21, 2018
This was referenced Dec 4, 2018
Merged
Merged
Open
Open
@lock lock Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@existentialism existentialism existentialism approved these changes

@xtuc xtuc xtuc approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees

No one assigned

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Polish 💅 A type of pull request used for our changelog categories

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a better error message for disallowed trailing commas after rest elements in function params

5 participants

@morozRed @babel-bot @nicolo-ribaudo @existentialism @xtuc