Too many PHP Warning related to Switch - Continue PHP statement

[trianity]

Bug Description

1690 lines of PHP Warning appeared in the log file from 14:10 to 20:00 related to Switch statement and Continue.
This is far from ideal because you have to turn off the PHP Warning if you want to operate the system, so many messages are generated.
Every Cron run creates all of these 3 PHH Warnings.

Q	A
Mautic version	v.2.15.2
PHP version	7.2.21 (Build Data, Aug 31 2019 20:45:48)
Browser	n./a.

Steps to reproduce

1. Install a fresh Mautic
2. Setup (required) crons to send email and monitor the Imap box
3. Wait a little, and check the server PHP error_log

Log errors

In the UnitOfWork.php file:
[01-Sep-2019 14:12:02 UTC] PHP Warning:  "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /home/path-to/mautic/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php on line 2636
[01-Sep-2019 14:12:02 UTC] PHP Warning:  "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /home/path-to/mautic/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php on line 2665

In the RequestTrait.php file:
[01-Sep-2019 17:38:02 UTC] PHP Warning:  "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /home/path-to/mautic/app/bundles/CoreBundle/Form/RequestTrait.php on line 42

How to handle?

By the documentation:
Note: In PHP the switch statement is considered a looping structure for the purposes of continue. continue behaves like break (when no arguments are passed). If a switch is inside a loop, continue 2 will continue with the next iteration of the outer loop.

It would be useful decide what was the original intent of the author of this part of code:
use Break or Use Continue with 2 or 3.

[Woeler]

Two of them are in vendor, so we can't directly edit that. I changed the one in the core though: #7845

[npracht]

Fixed in 2.16.0, release coming soon.

[onlineexpert]

We didn't see these warnings before on 2.15.3 but after upgrading to 2.16.0 the two in vendor started to appear on every Cron run. How can this be solved without turning of PHP Warnings?

PHP Warning:  "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /var/www/prod/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php on line 2636
PHP Warning:  "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in /var/www/prod/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php on line 2665

[RCheesley]

@online-expert according to the comment above from @Woeler (#7843 (comment)) there are two which relate to vendors where we are not able to make changes. The one which related to Mautic core was resolved with #7845.

@dennisameling any thoughts on this?

[RCheesley]

Found this PR which seemed to address this in the Doctrine GH repo in case it's of any help: doctrine/orm#7325

[onlineexpert]

Manually replacing 'continue' into 'break' on the two lines in vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php made the PHP Warnings go away for now but how does this code in vendor directories of Mautic gets updated?

[dennisameling]

Did some more checks. We already have a fix for this in our composer.json:

"patches": {
  "doctrine/orm": {
    "#6991 Support PHP 7.3 without dropping support for 5.6/7.0": "https://github.com/doctrine/doctrine2/commit/07fc401d255a4cdb4a557147d1c8a3fc7a0c718d.diff"
  }
}

When I check out the release-2.16 branch locally, I indeed see that the patch is applied in UnitOfWork.php, thus there are no errors. However, in our release package, 2.16.0.zip, the fix isn't applied.

This made me realize that something in our build scripts (the scripts that package Mautic for release versions) causes the patch to not be applied in our release package. I found this line in the build script:
composer install --no-dev --no-scripts --optimize-autoloader

When I run the command manually, the patch is indeed not applied. But when I remove the --no-scripts argument, the patch is applied:

Applying patches for doctrine/orm
    https://github.com/doctrine/doctrine2/commit/07fc401d255a4cdb4a557147d1c8a3fc7a0c718d.diff (#6991 Support PHP 7.3 without dropping support for 5.6/7.0)

TL;DR
So we need to update our build script to get this fixed. We'll need to do a 2.16.1 release to fix this for all users. For now, there are two workarounds:

• Switch back to PHP 7.2
• Replace continue with break in vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:2636 and vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:2665 until we get this fixed.

Thanks for reporting this bug!

[mautibot]

This issue has been mentioned on Mautic Community Forums. There might be relevant details there:

https://forum.mautic.org/t/got-error-php-message-php-warning-continue-targeting-switch-is-equivalent-to-break-did-you-mean-to-use-continue-2/12805/6

[RCheesley]

Great work @dennisameling thanks for tracking down the 🐛 !

[heathdutton]

Good catch @dennisameling I regret having to use that patching method. Hopefully we can avoid it in the future.

[WizardKid]

I have spend an entire day trying to get this software up and running. All the solutions on this thread didnt work for me. It is a great software but I have moved on now.

[WizardKid]

I have spend an entire day trying to get this software up and running. All the solutions on this thread didnt work for me. It is a great software but I have moved on now.

After long hours of going through the workarounds, I fixed my issue by removing all files, installed an old version and upgrading it from the dashboard to the recent version. It is working fine now.

[dennisameling]

We can't simply remove the --no-scripts part in our build process since it will run scripts that aren't needed when creating a release package. This is a known issue in the composer-patches lib we're using: cweagans/composer-patches#273

[dennisameling]

Found a way to fix this issue. We can't get composer to run plugin scripts manually (cweagans/composer-patches in this case, which applies patches) without also triggering other scripts in our composer.json.

So, I added the patched UnitOfWork.php to the build folder, which is then copied to the release package during the build process. It's not very pretty, but since we're already on the latest release of Doctrine ORM 2.5 (there won't be any more updates due to it being EOL), we should be good to go.