Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Remove PHP < 7.1.3 code#22820
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Remove PHP < 7.1.3 code#22820
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ogizanagi commentedMay 21, 2017
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | yes |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #22733 |
| License | MIT |
| Doc PR | N/A |
| * | ||
| * @author Alex Bogomazov | ||
| */ | ||
| abstractclass FilterIteratorextends \FilterIterator |
ogizanagiMay 21, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I removed this class as it was not needed anymore using php 7, but the class isn't marked as internal. So theorically, someone might have used it in its own code.
What about marking it@internal in 3.4, and triggering a deprecation if a non-symfony class is extending it?
Or should we just keep it empty in 4.0, and deprecate it in 4.1 (because there is not upgrade path for 3.x branch, appart from upgrading to php 7)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think we can just mark is as deprecated, with no notice triggered explicitly + internal also if you want, but that may be redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
See#22828
chalasr commentedMay 21, 2017
It looks like |
ogizanagi commentedMay 21, 2017
Thanks Robin. I'm also looking at similar checks and |
This PR was merged into the 3.4 branch.Discussion----------[Finder] Deprecate FilterIterator| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | no| New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files -->| BC breaks? | no| Deprecations? | yes <!-- don't forget updating UPGRADE-*.md files -->| Tests pass? | yes| Fixed tickets |#22820 (review)| License | MIT| Doc PR | N/ACommits-------946066c [Finder] Deprecate FilterIterator
nicolas-grekas commentedMay 21, 2017
rebase + fix needed |
| $f =function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition$v) {return$v; };return$f(${($_ =isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ?$this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] :$this->get('Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber')) &&false ?:'_'}); | ||
| },'baz' =>function () { | ||
| $f =function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition$v) {return$v; };return$f(${($_ =isset($this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ?$this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] :$this->getAutowired_Symfony_Component_DependencyInjection_Tests_Fixtures_CustomDefinitionService()) &&false ?:'_'}); | ||
| return$this->services['foo_service'] =new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber(new \Symfony\Component\DependencyInjection\ServiceLocator(array('Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' =>function (): ?\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition { |
ogizanagiMay 21, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nicolas-grekas : I've dug some more and played a bit with registered passes, but I cant' find the reason I don't get the proper invalid behavior in theTypedReference, which prevents the typehint to be nullable as expected.
Anyway, if this output looks good to you, the fix is actually needed in 3.3 branch, where I would have expected the following:
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.phpindex df56035724..e7e8b00049 100644--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php@@ -95,13 +95,13 @@ class ProjectServiceContainer extends Container protected function getFooServiceService() { return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber(new \Symfony\Component\DependencyInjection\ServiceLocator(array('Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => function () {- $f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v) { return $v; }; return $f(${($_ = isset($this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->getAutowired_Symfony_Component_DependencyInjection_Tests_Fixtures_CustomDefinitionService()) && false ?: '_'});+ $f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v = null) { return $v; }; return $f(${($_ = isset($this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->getAutowired_Symfony_Component_DependencyInjection_Tests_Fixtures_CustomDefinitionService()) && false ?: '_'}); }, 'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\TestServiceSubscriber' => function () { $f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber $v) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] : $this->get('Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber')) && false ?: '_'}); }, 'bar' => function () { $f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] : $this->get('Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber')) && false ?: '_'}); }, 'baz' => function () {- $f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v) { return $v; }; return $f(${($_ = isset($this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->getAutowired_Symfony_Component_DependencyInjection_Tests_Fixtures_CustomDefinitionService()) && false ?: '_'});+ $f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v = null) { return $v; }; return $f(${($_ = isset($this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['autowired.Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->getAutowired_Symfony_Component_DependencyInjection_Tests_Fixtures_CustomDefinitionService()) && false ?: '_'}); }))); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I've updated back the output to match actual test's one, since this output is consistent with the one from 3.3 branch. If a fix is actually required, it'll have to be first in 3.3.
Actually, maybe there is no bug, and the PhpDumper is only clever enough to know the services always exist here anyway.
ogizanagi commentedMay 22, 2017
Travis & fabbot failures unrelated (AFAIK). |
nicolas-grekas commentedMay 22, 2017
Thank you@ogizanagi. |
This PR was squashed before being merged into the 4.0-dev branch (closes#22820).Discussion----------Remove PHP < 7.1.3 code| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | yes| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->| Tests pass? | yes| Fixed tickets |#22733| License | MIT| Doc PR | N/ACommits-------7091fb4 Remove PHP < 7.1.3 code
This PR was merged into the 2.7 branch.Discussion----------Remove redundant sprintf argument.| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#22820| License | MIT| Doc PR | –cc@fabpot@ogizanagiCommits-------c8012f0 Remove redundant sprintf arguments.