8000 feature #45360 [ErrorHandler] trigger deprecations for ``@final`` pro… · symfony/symfony@6e3d5c1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6e3d5c1

Browse files
nicolas-grekasnicolas-grekas
committed
feature #45360 [ErrorHandler] trigger deprecations for @final properties (nicolas-grekas, fancyweb)
This PR was merged into the 6.1 branch. Discussion ---------- [ErrorHandler] trigger deprecations for ``@final`` properties | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | Fix #43600 | License | MIT | Doc PR | - This PR makes DebugClassLoader trigger a deprecation when a property in a child class is redefined while the same property is ``@final`` on the parent class (the deprecation is silenced when both classes live in the exact same namespace or when the child property is deprecated.) It also makes all properties in the `Symfony\` namespace implicitly ``@final``, unless they are typed. The goal here is to be able to add types to all properties in 7.0, thus fixing #43600. Commits ------- eada4c0 Add more tests 751eea7 [ErrorHandler] trigger deprecations for ``@final`` properties
2 parents fa61d6e + eada4c0 commit 6e3d5c1

File tree

21 files changed

+192
-46
lines changed

21 files changed

+192
-46
lines changed

‎UPGRADE-6.1.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
UPGRADE FROM 6.0 to 6.1
22
=======================
33

4+
All components
5+
--------------
6+
7+
* Public and protected properties are now considered final;
8+
instead of overriding a property, consider setting its value in the constructor
9+
410
Console
511
-------
612

7-
* Deprecate `Command::$defaultName` and `Command::$defaultDescription`, use the `AsCommand` attribute instead.
13+
* Deprecate `Command::$defaultName` and `Command::$defaultDescription`, use the `AsCommand` attribute instead
814

915
Serializer
1016
----------

‎src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,10 @@ public function testTooLongNamespace()
7979

8080
abstract class MaxIdLengthAdapter extends AbstractAdapter
8181
{
82-
protected $maxIdLength = 50;
83-
8482
public function __construct(string $ns)
8583
{
84+
$this->maxIdLength = 50;
85+
8686
parent::__construct($ns);
8787
}
8888
}

‎src/Symfony/Component/Console/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ CHANGELOG
55
---
66

77
* Add method `__toString()` to `InputInterface`
8-
* Deprecate `Command::$defaultName` and `Command::$defaultDescription`, use the `AsCommand` attribute instead.
8+
* Deprecate `Command::$defaultName` and `Command::$defaultDescription`, use the `AsCommand` attribute instead
99

1010
6.0
1111
---

‎src/Symfony/Component/Console/Tests/Command/CommandTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,13 +503,27 @@ class Php8Command2 extends Command
503503

504504
class MyCommand extends Command
505505
{
506+
/**
507+
* @deprecated since Symfony 6.1
508+
*/
506509
protected static $defaultName = 'my:comm 10000 and';
510+
511+
/**
512+
* @deprecated since Symfony 6.1
513+
*/
507514
protected static $defaultDescription = 'This is a command I wrote all by myself';
508515
}
509516

510517
#[AsCommand(name: 'my:command', description: 'This is a command I wrote all by myself')]
511518
class MyAnnotatedCommand extends Command
512519
{
520+
/**
521+
* @deprecated since Symfony 6.1
522+
*/
513523
protected static $defaultName = 'i-shall-be-ignored';
524+
525+
/**
526+
* @deprecated since Symfony 6.1
527+
*/
514528
protected static $defaultDescription = 'This description should be ignored.';
515529
}

‎src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -413,16 +413,6 @@ class ProjectServiceContainer extends Container
413413
public $__foo_bar;
414414
public $__foo_baz;
415415
public $__internal;
416-
protected $privates;
417-
protected $methodMap = [
418-
'bar' => 'getBarService',
419-
'foo_bar' => 'getFooBarService',
420-
'foo.baz' => 'getFoo_BazService',
421-
'circular' => 'getCircularService',
422-
'throw_exception' => 'getThrowExceptionService',
423-
'throws_exception_on_service_configuration' => 'getThrowsExceptionOnServiceConfigurationService',
424-
'internal_dependency' => 'getInternalDependencyService',
425-
];
426416

427417
public function __construct()
428418
{
@@ -434,6 +424,15 @@ public function __construct()
434424
$this->__internal = new \stdClass();
435425
$this->privates = [];
436426
$this->aliases = ['alias' => 'bar'];
427+
$this->methodMap = [
428+
'bar' => 'getBarService',
429+
'foo_bar' => 'getFooBarService',
430+
'foo.baz' => 'getFoo_BazService',
431+
'circular' => 'getCircularService',
432+
'throw_exception' => 'getThrowExceptionService',
433+
'throws_exception_on_service_configuration' => 'getThrowsExceptionOnServiceConfigurationService',
434+
'internal_dependency' => 'getInternalDependencyService',
435+
];
437436
}
438437

439438
protected function getInternalService()

‎src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public function testRegisterClasses()
8989
$container = new ContainerBuilder();
9090
$container->setParameter('sub_dir', 'Sub');
9191
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
92-
$loader->autoRegisterAliasesForSinglyImplementedInterfaces = false;
92+
$loader->noAutoRegisterAliasesForSinglyImplementedInterfaces();
9393

9494
$loader->registerClasses(new Definition(), 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\\', 'Prototype/%sub_dir%/*');
9595
$loader->registerClasses(new Definition(), 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\\', 'Prototype/%sub_dir%/*'); // loading twice should not be an issue
@@ -270,7 +270,10 @@ public function testRegisterClassesWithWhenEnv(?string $env, bool $expected)
270270

271271
class TestFileLoader extends FileLoader
272272
{
273-
public $autoRegisterAliasesForSinglyImplementedInterfaces = true;
273+
public function noAutoRegisterAliasesForSinglyImplementedInterfaces()
274+
{
275+
$this->autoRegisterAliasesForSinglyImplementedInterfaces = false;
276+
}
274277

275278
public function load(mixed $resource, string $type = null): mixed
276279
{

‎src/Symfony/Component/ErrorHandler/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CHANGELOG
44
6.1
55
---
66

7-
* Report overridden `@final` constants
7+
* Report overridden `@final` constants and properties
88

99
5.4
1010
---

‎src/Symfony/Component/ErrorHandler/DebugClassLoader.php

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ class DebugClassLoader
112112
private static array $checkedClasses = [];
113113
private static array $final = [];
114114
private static array $finalMethods = [];
115+
private static array $finalProperties = [];
115116
private static array $finalConstants = [];
116117
private static array $deprecated = [];
117118
private static array $internal = [];
@@ -469,9 +470,10 @@ public function checkAnnotations(\ReflectionClass $refl, string $class): array
469470
self::$finalMethods[$class] = [];
470471
self::$internalMethods[$class] = [];
471472
self::$annotatedParameters[$class] = [];
473+
self::$finalProperties[$class] = [];
472474
self::$finalConstants[$class] = [];
473475
foreach ($parentAndOwnInterfaces as $use) {
474-
foreach (['finalMethods', 'internalMethods', 'annotatedParameters', 'returnTypes', 'finalConstants'] as $property) {
476+
foreach (['finalMethods', 'internalMethods', 'annotatedParameters', 'returnTypes', 'finalProperties', 'finalConstants'] as $property) {
475477
if (isset(self::${$property}[$use])) {
476478
self::${$property}[$class] = self::${$property}[$class] ? self::${$property}[$use] + self::${$property}[$class] : self::${$property}[$use];
477479
}
@@ -626,22 +628,29 @@ public function checkAnnotations(\ReflectionClass $refl, string $class): array
626628
}
627629
}
628630

629-
foreach ($refl->getReflectionConstants(\ReflectionClassConstant::IS_PUBLIC | \ReflectionClassConstant::IS_PROTECTED) as $constant) {
630-
if ($constant->class !== $class) {
631-
continue;
632-
}
631+
$finals = isset(self::$final[$class]) || $refl->isFinal() ? [] : [
632+
'finalConstants' => $refl->getReflectionConstants(\ReflectionClassConstant::IS_PUBLIC | \ReflectionClassConstant::IS_PROTECTED),
633+
'finalProperties' => $refl->getProperties(\ReflectionProperty::IS_PUBLIC | \ReflectionProperty::IS_PROTECTED),
634+
];
635+
foreach ($finals as $type => $reflectors) {
636+
foreach ($reflectors as $r) {
637+
if ($r->class !== $class) {
638+
continue;
639+
}
640+
641+
$doc = $this->parsePhpDoc($r);
633642

634-
foreach ($parentAndOwnInterfaces as $use) {
635-
if (isset(self::$finalConstants[$use][$constant->name])) {
636-
$deprecations[] = sprintf('The "%s::%s" constant is considered final. You should not override it in "%s".', self::$finalConstants[$use][$constant->name], $constant->name, $class);
643+
foreach ($parentAndOwnInterfaces as $use) {
644+
if (isset(self::${$type}[$use][$r->name]) && !isset($doc['deprecated']) && ('finalConstants' === $type || substr($use, 0, strrpos($use, '\\')) !== substr($use, 0, strrpos($class, '\\')))) {
645+
$msg = 'finalConstants' === $type ? '%s" constant' : '$%s" property';
646+
$deprecations[] = sprintf('The "%s::'.$msg.' is considered final. You should not override it in "%s".', self::${$type}[$use][$r->name], $r->name, $class);
647+
}
637648
}
638-
}
639649

640-
if (!($doc = $this->parsePhpDoc($constant)) || !isset($doc['final'])) {
641-
continue;
650+
if (isset($doc['final']) || ('finalProperties' === $type && str_starts_with($class, 'Symfony\\') && !$r->hasType())) {
651+
self::${$type}[$class][$r->name] = $class;
652+
}
642653
}
643-
644-
self::$finalConstants[$class][$constant->name] = $class;
645654
}
646655

647656
return $deprecations;

‎src/Symfony/Component/ErrorHandler/Tests/DebugClassLoaderTest.php

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,13 +401,34 @@ class_exists('Test\\'.ReturnType::class, true);
401401
], $deprecations);
402402
}
403403

404+
public function testOverrideFinalProperty()
405+
{
406+
$deprecations = [];
407+
set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; });
408+
$e = error_reporting(E_USER_DEPRECATED);
409+
410+
class_exists(Fixtures\OverrideFinalProperty::class, true);
411+
class_exists(Fixtures\FinalProperty\OverrideFinalPropertySameNamespace::class, true);
412+
class_exists('Test\\'.OverrideOutsideFinalProperty::class, true);
413+
414+
error_reporting($e);
415+
restore_error_handler();
416+
417+
$this->assertSame([
418+
'The "Symfony\Component\ErrorHandler\Tests\Fixtures\FinalProperty\FinalProperty::$pub" property is considered final. You should not override it in "Symfony\Component\ErrorHandler\Tests\Fixtures\OverrideFinalProperty".',
419+
'The "Symfony\Component\ErrorHandler\Tests\Fixtures\FinalProperty\FinalProperty::$prot" property is considered final. You should not override it in "Symfony\Component\ErrorHandler\Tests\Fixtures\OverrideFinalProperty".',
420+
'The "Symfony\Component\ErrorHandler\Tests\Fixtures\FinalProperty\FinalProperty::$implicitlyFinal" property is considered final. You should not override it in "Symfony\Component\ErrorHandler\Tests\Fixtures\OverrideFinalProperty".',
421+
'The "Test\Symfony\Component\ErrorHandler\Tests\FinalProperty\OutsideFinalProperty::$final" property is considered final. You should not override it in "Test\Symfony\Component\ErrorHandler\Tests\OverrideOutsideFinalProperty".'
422+
], $deprecations);
423+
}
424+
404425
public function testOverrideFinalConstant()
405426
{
406427
$deprecations = [];
407428
set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; });
408429
$e = error_reporting(E_USER_DEPRECATED);
409430

410-
class_exists( Fixtures\FinalConstant\OverrideFinalConstant::class, true);
431+
class_exists(Fixtures\FinalConstant\OverrideFinalConstant::class, true);
411432

412433
error_reporting($e);
413434
restore_error_handler();
@@ -523,6 +544,10 @@ public function ownAbstractBaseMethod() { }
523544
return $fixtureDir.\DIRECTORY_SEPARATOR.'ReturnType.php';
524545
} elseif ('Test\\'.Fixtures\OutsideInterface::class === $class) {
525546
return $fixtureDir.\DIRECTORY_SEPARATOR.'OutsideInterface.php';
547+
} elseif ('Test\\'.OverrideOutsideFinalProperty::class === $class) {
548+
return $fixtureDir.'OverrideOutsideFinalProperty.php';
549+
} elseif ('Test\\Symfony\\Component\\ErrorHandler\\Tests\\FinalProperty\\OutsideFinalProperty' === $class) {
550+
return $fixtureDir.'FinalProperty'.\DIRECTORY_SEPARATOR.'OutsideFinalProperty.php';
526551
}
527552
}
528553
}

‎src/Symfony/Component/ErrorHandler/Tests/Fixtures/FinalProperty/FinalProperty.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Symfony\Component\ErrorHandler\Tests\Fixtures\FinalProperty;
4+
5+
class FinalProperty
6+
{
7+
/**
8+
* @final
9+
*/
10+
public $pub;
11+
12+
/**
13+
* @final
14+
*/
15+
protected $prot;
16+
17+
/**
18+
* @final
19+
*/
20+
private $priv;
21+
22+
/**
23+
* @final
24+
*/
25+
public $notOverriden;
26+
27+
protected $implicitlyFinal;
28+
29+
protected string $typedSoNotFinal;
30+
31+
protected $deprecated;
32+
}

‎src/Symfony/Component/ErrorHandler/Tests/Fixtures/FinalProperty/OutsideFinalProperty.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
namespace Test\Symfony\Component\ErrorHandler\Tests\FinalProperty;
4+
5+
class OutsideFinalProperty
6+
{
 C2EE
7+
/**
8+
* @final
9+
*/
10+
public $final;
11+
12+
protected $notImplicitlyFinalBecauseNotInSymfony;
13+
14+
/**
15+
* @final
16+
*/
17+
public $notOverriden;
18+
19+
/**
20+
* @final
21+
*/
22+
protected $deprecated;
23+
}

‎src/Symfony/Component/ErrorHandler/Tests/Fixtures/FinalProperty/OverrideFinalPropertySameNamespace.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace Symfony\Component\ErrorHandler\Tests\Fixtures\FinalProperty;
4+
5+
class OverrideFinalPropertySameNamespace extends FinalProperty
6+
{
7+
public $pub;
8+
}

‎src/Symfony/Component/ErrorHandler/Tests/Fixtures/OverrideFinalProperty.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
namespace Symfony\Component\ErrorHandler\Tests\Fixtures;
4+
5+
use Symfony\Component\ErrorHandler\Tests\Fixtures\FinalProperty\FinalProperty;
6+
7+
class OverrideFinalProperty extends FinalProperty
8+
{
9+
public $pub;
10+
protected $prot;
11+
private $priv;
12+
protected $implicitlyFinal;
13+
protected string $typedSoNotFinal;
14+
/**
15+
* @deprecated
16+
*/
17+
protected $deprecated;
18+
}

‎src/Symfony/Component/ErrorHandler/Tests/Fixtures/OverrideOutsideFinalProperty.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
namespace Test\Symfony\Component\ErrorHandler\Tests;
4+
5+
use Test\Symfony\Component\ErrorHandler\Tests\FinalProperty\OutsideFinalProperty;
6+
7+
class OverrideOutsideFinalProperty extends OutsideFinalProperty
8+
{
9+
public $final;
10+
protected $notImplicitlyFinalBecauseNotInSymfony;
11+
/**
12+
* @deprecated
13+
*/
14+
protected $deprecated;
15+
}

‎src/Symfony/Component/Form/Tests/VersionAwareTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
trait VersionAwareTest
1515
{
16-
protected static $supportedFeatureSetVersion = 404;
16+
protected static int $supportedFeatureSetVersion = 404;
1717

1818
protected function requiresFeatureSet(int $requiredFeatureSetVersion)
1919
{

‎src/Symfony/Component/Ldap/Adapter/ExtLdap/Query.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ class Query extends AbstractQuery
2626
{
2727
public const PAGINATION_OID = \LDAP_CONTROL_PAGEDRESULTS;
2828

29-
/** @var Connection */
30-
protected $connection;
31-
3229
/** @var resource[]|Result[] */
3330
private array $results;
3431

‎src/Symfony/Component/Validator/Test/ConstraintValidatorTestCase.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
* A test case to ease testing Constraint Validators.
4040
*
4141
* @author Bernhard Schussek <bschussek@gmail.com>
42+
*
43+
* @template T of ConstraintValidatorInterface
4244
*/
4345
abstract class ConstraintValidatorTestCase extends TestCase
4446
{
@@ -48,7 +50,7 @@ abstract class ConstraintValidatorTestCase extends TestCase
4850
protected $context;
4951

5052
/**
51-
* @var ConstraintValidatorInterface
53+
* @var T
5254
*/
5355
protected $validator;
5456

‎src/Symfony/Component/Validator/Tests/Constraints/ImageValidatorTest.php

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,11 @@
1818

1919
/**
2020
* @requires extension fileinfo
21+
*
22+
* @extends ConstraintValidatorTestCase<ImageValidator>
2123
*/
2224
class ImageValidatorTest extends ConstraintValidatorTestCase
2325
{
24-
protected $context;
25-
26-
/**
27-
* @var ImageValidator
28-
*/
29-
protected $validator;
30-
3126
protected $path;
3227
protected $image;
3328
protected $imageLandscape;

0 commit comments

Comments
 (0)
0