feature #38996 Remove the default values from setters with a nullable parameter (derrabus, nicolas-grekas)

This PR was merged into the 6.2 branch.

Discussion
----------

Remove the default values from setters with a nullable parameter

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | N/A
| License       | MIT
| Doc PR        | N/A

Setters with a default value are a bit odd: I want to set a value, but I won't tell you which one!

We do have this kind of setters, like this example from `TokenStorageInterface`:

```php
public function setToken(TokenInterface $token = null);
```

This means that those to calls are equivalent:

```php
$tokenStorage->setToken(null);
$tokenStorage->setToken();
```

The reason for this is actually a php 5 quirk: On php 5, we did not have nullable parameter type declarations – those we added in php 7.1. The only workaround was to declare `null` as default value because then php would accept passing null despite the type declaration demanding an instance of a specific class/interface.

Because the days of php 5 are over, I'd like to change this. Our method signature would then look like this.

```php
public function setToken(?TokenInterface $token);
```

We can do this for interfaces and abstract methods because an implementation may add a default value. Thus, removing the default value from the interface alone is not a BC break.

For the implementations of that interface, this is a different story because removing the default breaks calling code that omits the parameter entirely. This is why for the implementations I trigger a deprecation if the method is called without arguments. This enables us to remove the default in Symfony 6.

This PR performs the suggested changes for `TokenStorageInterface` and `ContainerAwareInterface`, but we have a few more setters like this. But before I continue, I'd like to collect some feedback if this is something you would want to change.

Commits
-------

78803b2962 Remove all "nullable-by-default-value" setters
917ffde141 Remove the default values from setters with a nullable parameter.

Author: nicolas-grekas

CHANGELOG.md

@@ -9,6 +9,8 @@ CHANGELOG
  * Add `enum` env var processor
  * Add `shuffle` env var processor
  * Allow #[When] to be extended
+ * Change the signature of `ContainerAwareInterface::setContainer()` to `setContainer(?ContainerInterface)`
+ * Deprecate calling `ContainerAwareTrait::setContainer()` without arguments
 
 6.1
 ---

ContainerAwareInterface.php

@@ -21,5 +21,5 @@ interface ContainerAwareInterface
     /**
      * Sets the container.
      */
-    public function setContainer(ContainerInterface $container = null);
+    public function setContainer(?ContainerInterface $container);
 }

ContainerAwareTrait.php

@@ -25,6 +25,10 @@ trait ContainerAwareTrait
 
     public function setContainer(ContainerInterface $container = null)
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/dependency-injection', '6.2', 'Calling "%s::%s()" without any arguments is deprecated, pass null explicitly instead.', __CLASS__, __FUNCTION__);
+        }
+
         $this->container = $container;
     }
 }

Tests/ContainerAwareTraitTest.php

@@ -0,0 +1,64 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Tests;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerAwareTrait;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+class ContainerAwareTraitTest extends TestCase
+{
+    use ExpectDeprecationTrait;
+
+    /**
+     * @group legacy
+     */
+    public function testSetContainerLegacy()
+    {
+        $container = $this->createMock(ContainerInterface::class);
+
+        $dummy = new ContainerAwareDummy();
+        $dummy->setContainer($container);
+
+        self::assertSame($container, $dummy->getContainer());
+
+        $this->expectDeprecation('Since symfony/dependency-injection 6.2: Calling "Symfony\Component\DependencyInjection\Tests\ContainerAwareDummy::setContainer()" without any arguments is deprecated, pass null explicitly instead.');
+
+        $dummy->setContainer();
+        self::assertNull($dummy->getContainer());
+    }
+
+    public function testSetContainer()
+    {
+        $container = $this->createMock(ContainerInterface::class);
+
+        $dummy = new ContainerAwareDummy();
+        $dummy->setContainer($container);
+
+        self::assertSame($container, $dummy->getContainer());
+
+        $dummy->setContainer(null);
+        self::assertNull($dummy->getContainer());
+    }
+}
+
+class ContainerAwareDummy implements ContainerAwareInterface
+{
+    use ContainerAwareTrait;
+
+    public function getContainer(): ?ContainerInterface
+    {
+        return $this->container;
+    }
+}