Skip to content

Commit f3ce137

Browse files
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.
2 parents 3db79f1 + 300aaf5 commit f3ce137

File tree

4 files changed

+7
-2
lines changed

4 files changed

+7
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ CHANGELOG
1010
* Add `absolute_uri` option to surrogate fragment renderers
1111
* Add `ValueResolverInterface` and deprecate `ArgumentValueResolverInterface`
1212
* Add argument `$reflector` to `ArgumentResolverInterface` and `ArgumentMetadataFactoryInterface`
13+
* Deprecate calling `ConfigDataCollector::setKernel()`, `RouterListener::setCurrentRequest()` without arguments
1314

1415
6.1
1516
---

DataCollector/ConfigDataCollector.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ class ConfigDataCollector extends DataCollector implements LateDataCollectorInte
3131
*/
3232
public function setKernel(KernelInterface $kernel = null)
3333
{
34+
if (1 > \func_num_args()) {
35+
trigger_deprecation('symfony/http-kernel', '6.2', 'Calling "%s()" without any arguments is deprecated, pass null explicitly instead.', __METHOD__);
36+
}
37+
3438
$this->kernel = $kernel;
3539
}
3640

EventListener/RouterListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function __construct(UrlMatcherInterface|RequestMatcherInterface $matcher
6868
$this->debug = $debug;
6969
}
7070

71-
private function setCurrentRequest(Request $request = null)
71+
private function setCurrentRequest(?Request $request)
7272
{
7373
if (null !== $request) {
7474
try {

HttpCache/ResponseCacheStrategy.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public function update(Response $response)
110110
$response->headers->set('Age', $this->age);
111111

112112
if ($this->isNotCacheableResponseEmbedded) {
113-
$response->setLastModified();
113+
$response->setLastModified(null);
114114

115115
if ($this->flagDirectives['no-store']) {
116116
$response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate');

0 commit comments

Comments
 (0)