Skip to content

Fix compatibility with PHP 7.3/nightly#2086

Merged
gsherwood merged 1 commit intosquizlabs:masterfrom
jrfnl:feature/compatibility-with-php-7.3-nightly
Jul 9, 2018
Merged

Fix compatibility with PHP 7.3/nightly#2086
gsherwood merged 1 commit intosquizlabs:masterfrom
jrfnl:feature/compatibility-with-php-7.3-nightly

Conversation

@jrfnl
Copy link
Copy Markdown
Contributor

@jrfnl jrfnl commented Jul 8, 2018

As of PHP 7.3, using continue to target a switch control structure will throw an E_WARNING.

Applying continue to a switch is equivalent to using break and more often than not, a continue targeting a higher level control structure is actually intended.

To target the higher level control structure, a numeric argument has to be passed to continue.

Refs:

This change has been recently committed to PHP itself and will break the build against nightly for the current master already.
See: https://travis-ci.org/jrfnl/PHP_CodeSniffer/jobs/401533444 (this branch, but without commits, so equal to the current PHPCS master).

This PR fixes the instances of this in PHPCS so the build against nightly will pass again.

Notes:

  • All these switches are nested in other control structures, but end the switch at the scope closer of the outer control structure, so using break in these cases is equivalent to using continue 2.

FYI: The PHPCompatibility standard will include a sniff to detect this in the near future. This PR is the result of some tests I've been running on the WIP PHPCompatibility sniff 😉

@gsherwood
Copy link
Copy Markdown
Member

Applying continue to a switch is equivalent to using break and more often than not, a continue targeting a higher level control structure is actually intended.

And I'm sure that's what I intended in all these cases. It's just lucky that there is no code after the switch in all of them.

I know that using break here is the equivalent of what the code actually does, but it's not what the intention of the developer was. So I think using continue(2) might be the way to go here so that I can later differentiate between times I wanted to only break out of the switch, and times when I really wanted the loop to continue.

@jrfnl jrfnl force-pushed the feature/compatibility-with-php-7.3-nightly branch from 65eca84 to 16c771d Compare July 9, 2018 01:42
@jrfnl
Copy link
Copy Markdown
Contributor Author

jrfnl commented Jul 9, 2018

@gsherwood I've updated (amended) the PR to reflect your remark. Should be good to go.

As of PHP 7.3, using `continue` to target a `switch` control structure will throw an `E_WARNING`.

Applying `continue` to a `switch` is equivalent to using `break` and more often than not, a `continue` targeting a higher level control structure is actually intended.

To target the higher level control structure, a numeric argument has to be passed to `continue`.

Refs:
* php/php-src#3364
* https://wiki.php.net/rfc/continue_on_switch_deprecation

This change has been recently committed to PHP itself and will break the build against `nightly` for the current `master` already. See: https://travis-ci.org/jrfnl/PHP_CodeSniffer/jobs/401533444 (this branch, but without commits, so equal to the current `master`).

This PR fixes the instances of this in PHPCS.
All these switches are nested in other control structures, but end the switch end the scope closer of the outer control structure, so using `break` in these cases would be equivalent to using `continue 2`.

FYI: The PHPCompatibility standard will include a sniff to detect this in the near future.
@gsherwood gsherwood added this to the 3.3.1 milestone Jul 9, 2018
@gsherwood gsherwood merged commit 16c771d into squizlabs:master Jul 9, 2018
@gsherwood
Copy link
Copy Markdown
Member

Thanks a lot for finding and fixing these. I think that PHPCompat sniff is going to come in handy.

@jrfnl jrfnl deleted the feature/compatibility-with-php-7.3-nightly branch July 9, 2018 14:13
@jrfnl
Copy link
Copy Markdown
Contributor Author

jrfnl commented Jul 9, 2018

Thanks a lot for finding and fixing these. I think that PHPCompat sniff is going to come in handy.

You're welcome ;-)

The PR for the PHPCompatibility sniff has been opened: PHPCompatibility/PHPCompatibility#687 (in case other people are looking for it),

szepeviktor referenced this pull request in phpstan/phpstan Jul 25, 2018
On php:master there's no xdebug
@jrfnl jrfnl mentioned this pull request Sep 15, 2018
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