diff --git a/typo3/sysext/core/Classes/Compatibility/PublicPropertyDeprecationTrait.php b/typo3/sysext/core/Classes/Compatibility/PublicPropertyDeprecationTrait.php new file mode 100644 index 0000000000000000000000000000000000000000..df2f8647303debfc7a1ae9b8434a4ba0835185b0 --- /dev/null +++ b/typo3/sysext/core/Classes/Compatibility/PublicPropertyDeprecationTrait.php @@ -0,0 +1,136 @@ +<?php +declare(strict_types=1); +namespace TYPO3\CMS\Core\Compatibility; + +/* + * This file is part of the TYPO3 CMS project. + * + * It is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License, either version 2 + * of the License, or any later version. + * + * For the full copyright and license information, please read the + * LICENSE.txt file that was distributed with this source code. + * + * The TYPO3 project - inspiring people to share! + */ + +/** + * Trait to support the logging of deprecation of public properties. + * + * This is useful due to the long list of PHP4 properties have been set to + * public previously, which should be removed or moved to "protected" / "private". + * + * Usage: + * + * - Use this trait for the class with the properties to change the visibility status or to be removed. + * - Set internal class properties to protected. + * - Add the phpDoc tag "private" to the property (so IDEs understand that). + * - Remove this tag with the next major version. + * - Remove trait after last deprecation. + * + * Note: + * + * Use this trait for classes only that do not make use of magic accessors otherwise. + * + * Example usage: + * + * + * class MyControllerClass { + * use PublicPropertyDeprecationTrait; + * + * /** + * * List previously publically accessible variables + * * @var array + * *... + * private $deprecatedPublicProperties = [ + * 'myProperty' => 'Using myProperty is deprecated and will not be possible anymore in TYPO3 v10. Use getMyProperty() instead.' + * ]; + * + * /** + * * This is my property. + * * + * * @var bool + * * @deprecated (if deprecated) + * * @private (if switched to private) + * / + * protected $myProperty = true; + * } + */ + +/** + * This trait has no public properties by default, ensure to add a $deprecatedPublicProperties to your class + * when using this trait. + */ +trait PublicPropertyDeprecationTrait +{ + /** + * Checks if the property of the given name is set. + * + * Unmarked protected properties must return false as usual. + * Marked properties are evaluated by isset(). + * + * This method is not called for public properties. + * + * @param string $propertyName + * @return bool + */ + public function __isset(string $propertyName) + { + if (isset($this->deprecatedPublicProperties[$propertyName])) { + trigger_error($this->deprecatedPublicProperties[$propertyName], E_USER_DEPRECATED); + return isset($this->$propertyName); + } + return false; + } + + /** + * Gets the value of the property of the given name if tagged. + * + * The evaluation is done in the assumption that this method is never + * reached for a public property. + * + * @param string $propertyName + * @return mixed + */ + public function __get(string $propertyName) + { + if (isset($this->deprecatedPublicProperties[$propertyName])) { + trigger_error($this->deprecatedPublicProperties[$propertyName], E_USER_DEPRECATED); + } + return $this->$propertyName; + } + + /** + * Sets the property of the given name if tagged. + * + * Additionally it's allowed to set unknown properties. + * + * The evaluation is done in the assumption that this method is never + * reached for a public property. + * + * @param string $propertyName + * @param mixed $propertyValue + */ + public function __set(string $propertyName, $propertyValue) + { + // It's allowed to set an unknown property as public, the check is thus necessary + if (property_exists($this, $propertyName) && isset($this->deprecatedPublicProperties[$propertyName])) { + trigger_error($this->deprecatedPublicProperties[$propertyName], E_USER_DEPRECATED); + } + $this->$propertyName = $propertyValue; + } + + /** + * Unsets the property of the given name if tagged. + * + * @param string $propertyName + */ + public function __unset(string $propertyName) + { + if (isset($this->deprecatedPublicProperties[$propertyName])) { + trigger_error($this->deprecatedPublicProperties[$propertyName], E_USER_DEPRECATED); + } + unset($this->$propertyName); + } +} diff --git a/typo3/sysext/core/Documentation/Changelog/master/Feature-81330-TraitToMigratePublicAccessToProtectedByDeprecation.rst b/typo3/sysext/core/Documentation/Changelog/master/Feature-81330-TraitToMigratePublicAccessToProtectedByDeprecation.rst new file mode 100644 index 0000000000000000000000000000000000000000..75245c86c8abca03bd57928277bb03133c811160 --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/master/Feature-81330-TraitToMigratePublicAccessToProtectedByDeprecation.rst @@ -0,0 +1,25 @@ +.. include:: ../../Includes.txt + +============================================================================ +Feature: #81330 - Trait to migrate public access to protected by deprecation +============================================================================ + +See :issue:`81330` +See Important-81330-DealingWithPropertiesThatAreMigratedToProtected.rst + +Description +=========== + +A new PHP Trait (PublicPropertyDeprecationTrait) is added to support the smooth migration of public properties to +a protected or private state of a property. By using this trait, deprecation warnings are thrown until the next +major TYPO3 version. + +Impact +====== + +Instead of creating a breaking change by setting a public property to protected, the migration can now by done by the +softer path of deprecation. This will encourage the encapsulation of core classes that still have public properties. +By reaching encapsulation, refactoring becomes a lot more easy. The core can be modernized more quickly with less +issues for developers. + +.. index:: PHP-API diff --git a/typo3/sysext/core/Documentation/Changelog/master/Important-81330-DealingWithPropertiesThatAreMigratedToProtected.rst b/typo3/sysext/core/Documentation/Changelog/master/Important-81330-DealingWithPropertiesThatAreMigratedToProtected.rst new file mode 100644 index 0000000000000000000000000000000000000000..3c621f6a16fca8a46856d186b93e632eb35615ab --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/master/Important-81330-DealingWithPropertiesThatAreMigratedToProtected.rst @@ -0,0 +1,71 @@ +.. include:: ../../Includes.txt + +=============================================================== +Feature: #81330 - Dealing with properties that become protected +=============================================================== + +See :issue:`81330` +See Feature-81330-TraitToMigratePublicAccessToProtectedByDeprecation.rst + +Intro +===== + +Still a lot of classes of the core have public properties that are also used from within extensions. To reach full +encapsulation of classes, the public access to properties needs to be removed. The property access shall be done by +public methods only. + +Public properties are migrated to protected and setters and getters are provided as needed. During a phase of +deprecation entries into the deprecation log are triggered, when an extension accesses a previously public property. +The code still keeps working until the next major release, when the deprecation tags are removed from the code. + +What options do you have to act, if such an entry is triggered by your extension? + +Types of public properties +========================== + +The remaining public properties can be classified into two types. The first type serves as public API to the internal +state. The second type has the character of fully internal functionality. + +The first type are accessors to configure a class, to inject components, to access results, etc. When these properties +are migrated to protected, methods are provided accordingly, like getters, setters or injectors. That's the new API +to use. + +The second type, properties that are of fully internal functionality, typically has never been called from outside of +the class. For this type no setters and getters are provided. If an extension is accessing this type, it's most likely +an ugly hack that is asking for clean solution. + +Strategies to migrate extensions +================================ + +Using the public API of methods +------------------------------- + +Refactor the extension to use the new API of public accessor methods to access the internal state. + +Finding a better design +----------------------- + +If you were accessing a property of the second type, the fully internal one, it's time to improve the design of +your extension. If you think the flaw of design is on side of the core, review the class. Provide your suggestions by +using the bug tracker or commit patches. + +Claiming getters and setters +---------------------------- + +Your extension may provide a valid use case for a public accessor that nobody was thinking of. Adding getters and +setters is no big deal and we like to see your extension working. Please raise your hand early during the period of +deprecation. Nothing needs to break. + +Using reflection +---------------- + +You could consider to force public access to the property by reflection. This is ugly and not recommended. +You could do this as a quick and dirty workaround, for example when you didn't act early enough. + +The second case to use reflection is to write unit tests. In the ideal world there should be no reason to access +protected properties by unit tests. In the real world there are good reasons now and then to do so. + +There will be no warning if protected properties are changed as they are internal. Be aware, that your extension or +your unit test may break suddenly, when you use this kind of workaround. + +.. index:: PHP-API diff --git a/typo3/sysext/core/Tests/Unit_Deprecated/Compatibility/PublicPropertyDeprecationTraitTest.php b/typo3/sysext/core/Tests/Unit_Deprecated/Compatibility/PublicPropertyDeprecationTraitTest.php new file mode 100644 index 0000000000000000000000000000000000000000..4ccd792f030ae40e88f7a190f21916e61e3cc31e --- /dev/null +++ b/typo3/sysext/core/Tests/Unit_Deprecated/Compatibility/PublicPropertyDeprecationTraitTest.php @@ -0,0 +1,143 @@ +<?php +declare(strict_types=1); +namespace TYPO3\CMS\Core\Tests\Unit\Compatibility; + +/* + * This file is part of the TYPO3 CMS project. + * + * It is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License, either version 2 + * of the License, or any later version. + * + * For the full copyright and license information, please read the + * LICENSE.txt file that was distributed with this source code. + * + * The TYPO3 project - inspiring people to share! + */ + +use TYPO3\CMS\Core\Compatibility\PublicPropertyDeprecationTrait; +use TYPO3\TestingFramework\Core\Unit\UnitTestCase; + +class PublicPropertyDeprecationTraitTest extends UnitTestCase +{ + /** + * @var object Test fixture (anonymous class) + * @see PublicAccessDeprecationTraitTest::setUp() + */ + protected $fixture; + + /** + * Setup + * + * Creating the test fixture, an anonymous class with different kinds + * of properties to test access for. + */ + protected function setUp() + { + $this->fixture = new class { + use PublicPropertyDeprecationTrait; + private $deprecatedPublicProperties = [ + 'taggedProperty' => 'taggedProperty is deprecated', + 'unsetTaggedProperty' => 'unsetTaggedProperty is deprecated' + ]; + + public $publicProperty = 'publicProperty'; + + public $unsetPublicProperty; + + /** + * @deprecatedPublic + */ + protected $taggedProperty = 'taggedProperty'; + + /** + * @deprecatedPublic + */ + protected $unsetTaggedProperty; + + protected $untaggedProperty = 'untaggedProperty'; + }; + } + + /** + * @return array [[$expected, $property],] + */ + public function issetDataProvider(): array + { + return [ + 'public property' => [true, 'publicProperty'], + 'unset public property' => [false, 'unsetPublicProperty'], + 'tagged property' => [true, 'taggedProperty'], + 'unset tagged property' => [false, 'unsetTaggedProperty'], + 'untagged property' => [false, 'untaggedProperty'], + 'unknown property' => [false, 'unknownProperty'], + ]; + } + + /** + * @dataProvider issetDataProvider + * @test + * @param bool $expected + * @param string $property + */ + public function issetWorksAsExpected(bool $expected, string $property) + { + $this->assertSame($expected, isset($this->fixture->$property)); + } + + /** + * @test + */ + public function unknownPropertyCanBeHandledAsUsual() + { + // Uses __isset() + $this->assertFalse(isset($this->fixture->unknownProperty)); + // Uses __set() + $this->fixture->unknownProperty = 23; + // Don't uses __isset() + $this->assertTrue(isset($this->fixture->unknownProperty)); + // Don't uses __get() + $this->assertSame(23, $this->fixture->unknownProperty); + // Don't uses __unset() + unset($this->fixture->unknownProperty); + // Uses __isset() + $this->assertFalse(isset($this->fixture->unknownProperty)); + } + + /** + * @test + */ + public function publicPropertyCanBeHandledAsUsual() + { + $this->assertFalse(isset($this->fixture->unsetPublicProperty)); + $this->fixture->unsetPublicProperty = 23; + $this->assertTrue(isset($this->fixture->unsetPublicProperty)); + $this->assertSame(23, $this->fixture->unsetPublicProperty); + unset($this->fixture->unsetPublicProperty); + $this->assertFalse(isset($this->fixture->unsetPublicProperty)); + } + + /** + * @test + */ + public function taggedPropertyCanBeHandledLikePublicProperty() + { + $this->assertFalse(isset($this->fixture->unsetTaggedProperty)); + $this->fixture->unsetTaggedProperty = 23; + $this->assertTrue(isset($this->fixture->unsetTaggedProperty)); + $this->assertSame(23, $this->fixture->unsetTaggedProperty); + unset($this->fixture->unsetTaggedProperty); + $this->assertFalse(isset($this->fixture->unsetTaggedProperty)); + } + + /** + * @return array [[$property],] + */ + public function invalidPropertiesDataProvider(): array + { + return [ + 'untagged' => ['untaggedProperty'], + 'unknown' => ['unknownProperty'], + ]; + } +}