Skip to content

Commit c657962

Browse files
[ErrorHandler] trigger deprecations for @final properties
1 parent 855fc71 commit c657962

File tree

5 files changed

+74
-15
lines changed

5 files changed

+74
-15
lines changed

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
---

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;

Tests/DebugClassLoaderTest.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,13 +401,30 @@ 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+
412+
error_reporting($e);
413+
restore_error_handler();
414+
415+
$this->assertSame([
416+
'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".',
417+
'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".',
418+
], $deprecations);
419+
}
420+
404421
public function testOverrideFinalConstant()
405422
{
406423
$deprecations = [];
407424
set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; });
408425
$e = error_reporting(E_USER_DEPRECATED);
409426

410-
class_exists( Fixtures\FinalConstant\OverrideFinalConstant::class, true);
427+
class_exists(Fixtures\FinalConstant\OverrideFinalConstant::class, true);
411428

412429
error_reporting($e);
413430
restore_error_handler();
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
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+
}

0 commit comments

Comments
 (0)