allow containing 0 or similar values except null. #26095

scil · 2018-02-08T15:38:19Z

otherwise, ' 0, false, [] are not be allowed with $containter->set()'.

Q	A
Branch?	4.0
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	yes/no
Tests pass?	yes
License	MIT
Doc PR

otherwise, doc maybe add words like ' 0, false, [] are not be used with $containter->set()'.

stof · 2018-02-08T15:54:22Z

Setting anything else than an object should be forbidden anyway and very likely to break (they should be stored as parameters)

nicolas-grekas

Looks legit to me, does no harm, for 3.4

nicolas-grekas · 2018-02-08T16:07:08Z

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -542,7 +542,7 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
        if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
            return parent::get($id, $invalidBehavior);
        }
-        if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
+        if (!is_null($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE))) {            


if (null !== $service = parent::g...

scil · 2018-02-09T08:25:03Z

I'm develping with swoole, such as LaravelFly, WebSocketFly.

In WebSocketFly, I use symfony/dependency-injection and make a containter for each worker process.
I tried inject worker id and failed, because worker id maybe 0.

public function onWorkerStart(\swoole_server $server, int $workerid)
{
    $workerContainer = clone $serverContainer; // fake code
    $workerContainer->set('id',$workerid); // set an interger value  [0, )
}

https://github.com/scil/WebSocketFly/blob/e54ab563f1bda45cc28172715c9c3e2c70c13cbb/src/WebSocketFly/Server.php#L131

xabbuh · 2018-02-09T08:32:16Z

Setting anything else than an object should be forbidden anyway and very likely to break (they should be stored as parameters)

@stof while I agree with you for Symfony < 4.1 this would break the new ContainerBag which was introduced in #25288 (which makes me think that #25288 actually breaks the contract specified by the ContainerInterface 🤔).

nicolas-grekas · 2018-02-09T08:59:46Z

@xabbuh WDYT? Which part of the contract?
@scil did you close on purpose?

xabbuh · 2018-02-09T10:00:21Z

ContainerInterface::get() demands to return an object which is not (necessarily) the case for parameters, right? Our interfaces differs from the PSR-11 one here.

jvasseur · 2018-02-09T10:17:32Z

@xabbuh that's not true, ContainerInterface::get() has a return type of mixed allowing anything to be stored in the container.

xabbuh · 2018-02-09T10:22:47Z

jvasseur · 2018-02-09T10:27:57Z

Oh, I thought you where speaking of the PSR-11 ContainerInterface, not the Symfony one.

So #25288 doesn't breaks the ContainerInterface contract since it extends the PSR-11 interface and not the Symfony one.

nicolas-grekas · 2018-02-09T10:30:06Z

@xabbuh actually that's only a non-enforced docblock; nothing anywhere prevents returning anything else in practice. And it is also trivial to use regular configuration to return a non object: just use a factory (I used this recently :) )

allow containing 0 or similar values except null.

19a92e4

otherwise, doc maybe add words like ' 0, false, [] are not be used with $containter->set()'.

carsonbot added Status: Needs Review Deprecation labels Feb 8, 2018

nicolas-grekas reviewed Feb 8, 2018

View reviewed changes

use null!==_ instead of !is_null(_)

64f97b6

scil closed this Feb 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow containing 0 or similar values except null. #26095

allow containing 0 or similar values except null. #26095

allow containing 0 or similar values except null. #26095

allow containing 0 or similar values except null. #26095

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment