Remove PHP < 7.1.3 code #22820

ogizanagi · 2017-05-21T09:16:50Z

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

ogizanagi · 2017-05-21T09:23:34Z

src/Symfony/Component/Finder/Iterator/FilterIterator.php

- *
- * @author Alex Bogomazov
- */
-abstract class FilterIterator extends \FilterIterator


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)?

I think we can just mark is as deprecated, with no notice triggered explicitly + internal also if you want, but that may be redundant

chalasr · 2017-05-21T09:27:31Z

It looks like ArgumentMetadataFactory::$supports* should be removed too https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php#L41..L42

ogizanagi · 2017-05-21T09:34:36Z

Thanks Robin. I'm also looking at similar checks and @requires PHP X in tests to find PHP versions support layers.

This PR was merged into the 3.4 branch. Discussion ---------- [Finder] Deprecate FilterIterator | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | no  | BC breaks? | no | Deprecations? | yes  | Tests pass? | yes | Fixed tickets | #22820 (review) | License | MIT | Doc PR | N/A Commits ------- 946066c [Finder] Deprecate FilterIterator

nicolas-grekas · 2017-05-21T17:41:17Z

rebase + fix needed

ogizanagi · 2017-05-21T18:30:45Z

src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php

-            $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 {


@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 the TypedReference, 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.php index 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 ?: '_'}); }))); }

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 · 2017-05-22T05:40:42Z

Travis & fabbot failures unrelated (AFAIK).

nicolas-grekas · 2017-05-22T06:50:34Z

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  | Tests pass? | yes | Fixed tickets | #22733 | License | MIT | Doc PR | N/A Commits ------- 7091fb4 Remove PHP < 7.1.3 code

@fabpot

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 @ogizanagi Commits ------- c8012f0 Remove redundant sprintf arguments.

carsonbot added Status: Needs Review BC Break labels May 21, 2017

ogizanagi commented May 21, 2017

View reviewed changes

ogizanagi mentioned this pull request May 21, 2017

[Finder] Deprecate FilterIterator #22828

Merged

ogizanagi commented May 21, 2017

View reviewed changes

ogizanagi added 2 commits May 22, 2017 07:41

Remove PHP < 7.1.3 code

e423010

PhpDumper: replace empty closure by nullable typehint

5772f88

nicolas-grekas closed this May 22, 2017

ogizanagi deleted the php7.1.3 branch May 22, 2017 07:10

fabpot mentioned this pull request Oct 19, 2017

Release v4.0.0-BETA1 #24612

Merged

Aliance mentioned this pull request Oct 19, 2017

Remove redundant sprintf argument. #24613

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove PHP < 7.1.3 code #22820

Remove PHP < 7.1.3 code #22820

ogizanagi commented May 21, 2017

ogizanagi May 21, 2017 •

edited

Loading

nicolas-grekas May 21, 2017

ogizanagi May 21, 2017

chalasr commented May 21, 2017

ogizanagi commented May 21, 2017

nicolas-grekas commented May 21, 2017

ogizanagi May 21, 2017 •

edited

Loading

ogizanagi May 22, 2017

ogizanagi commented May 22, 2017

nicolas-grekas commented May 22, 2017

Remove PHP < 7.1.3 code #22820

Remove PHP < 7.1.3 code #22820

Conversation

ogizanagi commented May 21, 2017

ogizanagi May 21, 2017 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas May 21, 2017

Choose a reason for hiding this comment

ogizanagi May 21, 2017

Choose a reason for hiding this comment

chalasr commented May 21, 2017

ogizanagi commented May 21, 2017

nicolas-grekas commented May 21, 2017

ogizanagi May 21, 2017 • edited Loading

Choose a reason for hiding this comment

ogizanagi May 22, 2017

Choose a reason for hiding this comment

ogizanagi commented May 22, 2017

nicolas-grekas commented May 22, 2017

ogizanagi May 21, 2017 •

edited

Loading

ogizanagi May 21, 2017 •

edited

Loading