Skip to content
This repository was archived by the owner on Jan 31, 2020. It is now read-only.

php 7.3 compat: continue targeting switch equaivalent to break; #91

Merged
merged 3 commits into from
Aug 28, 2018

Conversation

samsonasik
Copy link
Contributor

@samsonasik samsonasik commented Jul 8, 2018

Provide a narrative description of what you are trying to accomplish:

in PHP 7.3.0-dev ( nightly )

  • Are you fixing a bug?
    • Detail how the bug is invoked currently.
      It is inside ArrayObject class which the continue code called inside switch inside loop.
    • Detail the original, incorrect behavior.
      It got error: "E_WARNING "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"?"
    • Detail the new, expected behavior.
      using continue 2 using break; to jump to next loop.
    • Base your feature on the master branch, and submit against that branch.
    • Add a regression test that demonstrates the bug, and proves the fix.
      without this patch, the test should failure.
    • Add a CHANGELOG.md entry for the fix.

@samsonasik samsonasik changed the title php 7.3 compat: continue inside switch equaivalent to break; php 7.3 compat: continue targeting switch equaivalent to break; Jul 8, 2018
@samsonasik
Copy link
Contributor Author

actually, break should be enough as it not break the loop, but break the case itself, except, we use break 2 to break the loop itself, ref https://3v4l.org/3W0u4

@@ -425,7 +425,7 @@ public function unserialize($data)
$this->setIteratorClass($v);
break;
case 'protectedProperties':
continue;
continue 2;
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, this is more confusing, as it implies there are two loops, when, in this case case, it's actually a switch inside a foreach.

Let's make this a break instead, please, as that will achieve the same result: continuing on to the next item in the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@weierophinney weierophinney merged commit d0cd1d0 into zendframework:master Aug 28, 2018
weierophinney added a commit that referenced this pull request Aug 28, 2018
weierophinney added a commit that referenced this pull request Aug 28, 2018
weierophinney added a commit that referenced this pull request Aug 28, 2018
@weierophinney
Copy link
Member

Thanks, @samsonasik

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants