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

Allow trailing comma for function use and parameter lists #664

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mkornaukhov03
Copy link
Contributor

@mkornaukhov03 mkornaukhov03 commented Oct 31, 2022

Support PHP8 feature.
Main issue: #290
Previous attempt: #308
I didn't rebase because of merge conflict.

@quasilyte
Copy link
Contributor

quasilyte commented Nov 2, 2022

Hi!

I would suggest adding the tests.

  1. One test should fail for the incorrect syntax combinations. Two trailing commas, etc. See @kphp-should-fail annotations in tests/phpt/
  2. Another test should check that trailing commas work as expected (@ok test).

Basically, this feature should work when it works in PHP and fail to compile when it's not a valid syntax for PHP. You may look at the PHP pull request for an inspiration. It probably includes some tests.

See the docs for more info about contributing (how to run tests, etc.)

Or you can ask the questions here.
For Russian-speaking contributors I would suggest a Telegram chat: https://t.me/kphp_chat

@Danil42Russia Danil42Russia added the PHP8 PHP8 feature label Nov 3, 2022
@mkornaukhov03
Copy link
Contributor Author

Please check

@mkornaukhov03 mkornaukhov03 requested review from unserialize and quasilyte and removed request for unserialize February 8, 2023 16:11
@mkornaukhov03 mkornaukhov03 force-pushed the mkornaukhov03_trailing_comma branch 3 times, most recently from f5937ec to 8f78498 Compare February 15, 2023 16:53
@Tsygankov-Slava Tsygankov-Slava mentioned this pull request Jul 3, 2023
44 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP8 PHP8 feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants