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

@@ -5,6 +5,7 @@ CHANGELOG
 ---
 
  * The HTTP cache store uses the `xxh128` algorithm
+ * Deprecate calling `JsonResponse::setCallback()`, `Response::setExpires/setLastModified/setEtag()`, `MockArraySessionStorage/NativeSessionStorage::setMetadataBag()`, `NativeSessionStorage::setSaveHandler()` without arguments
 
 6.1
 ---

JsonResponse.php

@@ -79,6 +79,9 @@ public static function fromJsonString(string $data, int $status = 200, array $he
      */
     public function setCallback(string $callback = null): static
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
+        }
         if (null !== $callback) {
             // partially taken from https://geekality.net/2011/08/03/valid-javascript-identifier/
             // partially taken from https://github.com/willdurand/JsonpCallbackValidator

Response.php

@@ -720,6 +720,9 @@ public function getExpires(): ?\DateTimeInterface
      */
     public function setExpires(\DateTimeInterface $date = null): static
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
+        }
         if (null === $date) {
             $this->headers->remove('Expires');
 
@@ -899,6 +902,9 @@ public function getLastModified(): ?\DateTimeInterface
      */
     public function setLastModified(\DateTimeInterface $date = null): static
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
+        }
         if (null === $date) {
             $this->headers->remove('Last-Modified');
 
@@ -937,6 +943,9 @@ public function getEtag(): ?string
      */
     public function setEtag(string $etag = null, bool $weak = false): static
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
+        }
         if (null === $etag) {
             $this->headers->remove('Etag');
         } else {

Session/Storage/MockArraySessionStorage.php

@@ -173,6 +173,9 @@ public function isStarted(): bool
 
     public function setMetadataBag(MetadataBag $bag = null)
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
+        }
         if (null === $bag) {
             $bag = new MetadataBag();
         }

Session/Storage/NativeSessionStorage.php

@@ -301,6 +301,9 @@ public function getBag(string $name): SessionBagInterface
 
     public function setMetadataBag(MetadataBag $metaBag = null)
     {
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
+        }
         if (null === $metaBag) {
             $metaBag = new MetadataBag();
         }
@@ -377,10 +380,8 @@ public function setOptions(array $options)
      */
     public function setSaveHandler(AbstractProxy|\SessionHandlerInterface $saveHandler = null)
     {
-        if (!$saveHandler instanceof AbstractProxy &&
-            !$saveHandler instanceof \SessionHandlerInterface &&
-            null !== $saveHandler) {
-            throw new \InvalidArgumentException('Must be instance of AbstractProxy; implement \SessionHandlerInterface; or be null.');
+        if (1 > \func_num_args()) {
+            trigger_deprecation('symfony/http-foundation', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
         }
 
         // Wrap $saveHandler in proxy and prevent double wrapping of proxy

Tests/ResponseTest.php

@@ -995,7 +995,7 @@ public function testHasVary()
     public function testSetEtag()
     {
         $response = new Response('', 200, ['ETag' => '"12345"']);
-        $response->setEtag();
+        $response->setEtag(null);
 
         $this->assertNull($response->headers->get('Etag'), '->setEtag() removes Etags when call with null');
     }

Tests/Session/Storage/NativeSessionStorageTest.php

@@ -206,8 +206,6 @@ public function testSetSaveHandler()
     {
         $this->iniSet('session.save_handler', 'files');
         $storage = $this->getStorage();
-        $storage->setSaveHandler();
-        $this->assertInstanceOf(SessionHandlerProxy::class, $storage->getSaveHandler());
         $storage->setSaveHandler(null);
         $this->assertInstanceOf(SessionHandlerProxy::class, $storage->getSaveHandler());
         $storage->setSaveHandler(new SessionHandlerProxy(new NativeFileSessionHandler()));