8000 [DI] Implement PSR-11 by greg0ire · Pull Request #21265 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Implement PSR-11 #21265

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 8, 2017
Merged

[DI] Implement PSR-11 #21265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
8000
Diff view
Diff view
2 changes: 2 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"doctrine/common": "~2.4",
"twig/twig": "~1.28|~2.0",
"psr/cache": "~1.0",
"psr/container": "^1.0",
"psr/log": "~1.0",
"psr/simple-cache": "^1.0",
"symfony/polyfill-intl-icu": "~1.0",
Expand Down Expand 8000 Up @@ -102,6 +103,7 @@
},
"provide": {
"psr/cache-implementation": "1.0",
"psr/container-implementation": "1.0",
"psr/simple-cache-implementation": "1.0"
},
"autoload": {
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
<argument type="collection" />
</service>

<service id="Psr\Container\ContainerInterface" alias="service_container" public="false" />
<service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container" public="false" />
<service id="Symfony\Component\DependencyInjection\Container" alias="service_container" public="false" />

Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/DependencyInjection/ContainerInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\DependencyInjection;

use Psr\Container\ContainerInterface as PsrContainerInterface;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
Expand All @@ -21,7 +22,7 @@
* @author Fabien Potencier <fabien@symfony.com>
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
interface ContainerInterface
interface ContainerInterface extends PsrContainerInterface
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose to have ContainerInterface extend PsrContainerInterface, so that any interface is marked as psr-compliant. Not sure it is the right thing to do though, because php cannot enforce all the contracts defined in the interface docs (like what exception to throw when). Tell me if you feel I should remove this and add implements ContainerInterface in the main implementation.

Copy link
Member

Choose a reason for hiding this comment

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

If it works for PHP, fine.

Copy link
Member

Choose a reason for hiding this comment

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

that basically means in whole framework you will not use Psr version, but Symfony version.
Then, one will be able to use concrete class that implements Symfony version as parameter for method expecting Psr version, but not the other way around. So, if I will have some package that follows Psr, I won't be able to use it directly in Symfony project, because everywhere the framework will require Symfony version instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

everywhere the framework will require Symfony

Not sure I get what you mean, but IMO once this is merged, we should hunt for the Symfony version everywhere, see what calls are made there, and if we can replace use Psr version as type hinting there.

Copy link
Member
@nicolas-grekas nicolas-grekas Jan 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@greg0ire without breaking BC, with the benefit of reducing the feature-set for the shake of PSR11 compliance? I doubt this is ever going to happen...

Copy link
Member

Choose a reason for hiding this comment

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

@jvasseur yes it does
the issue is that $foo instanceof A implies that calling $foo->test($anotherInstanceOfA) is allowed.
Since $anInstanceofB instanceof A, calling $anInstanceofB->test($anotherInstanceOfAButNotB) must thus be allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So… I found a bug in php?

Copy link
Contributor
@jvasseur jvasseur Jan 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It's a know limitation of the PHP engine : https://bugs.php.net/bug.php?id=69612#1431343525

Copy link
Contributor

Choose a reason for hiding this comment

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

But your example is wrong anyway, functions parameters could be contravariant (they are invariant in PHP) meaning they can accept a wider type. In your example they are covariant instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh you're right, my logic was wrong indeed! Which means this would indeed be a BC-break, sorry I got confused.

{
const EXCEPTION_ON_INVALID_REFERENCE = 1;
const NULL_ON_INVALID_REFERENCE = 2;
Expand Down
4 changes: 3 additions & 1 deletion src/Symfony/Component/DependencyInjection/Exception/ExceptionInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@

namespace Symfony\Component\DependencyInjection\Exception;

use Psr\Container\ContainerExceptionInterface;

/**
* Base ExceptionInterface for Dependency Injection component.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Bulat Shakirzyanov <bulat@theopenskyproject.com>
*/
interface ExceptionInterface
interface ExceptionInterface extends ContainerExceptionInterface
{
}
4 changes: 3 additions & 1 deletion src/Symfony/Component/DependencyInjection/Exception/ServiceNotFoundException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@

namespace Symfony\Component\DependencyInjection\Exception;

use Psr\Container\NotFoundExceptionInterface;

/**
* This exception is thrown when a non-existent service is requested.
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class ServiceNotFoundException extends InvalidArgumentException
class ServiceNotFoundException extends InvalidArgumentException implements NotFoundExceptionInterface
{
private $id;
private $sourceId;
Expand Down
6 changes: 5 additions & 1 deletion src/Symfony/Component/DependencyInjection/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +1 75DA 6,8 @@
}
],
"require": {
"php": ">=5.5.9"
"php": ">=5.5.9",
"psr/container": "^1.0"
},
"require-dev": {
"symfony/yaml": "~3.2",
Expand All @@ -32,6 +33,9 @@
"conflict": {
"symfony/yaml": "<3.2"
},
"provide": {
"psr/container-implementation": "1.0"
},
"autoload": {
"psr-4": { "Symfony\\Component\\DependencyInjection\\": "" },
"exclude-from-classmap": [
Expand Down
0