[FEATURE] Provide a trait to support public access deprecation 28/52928/34
authorElmar Hinz <t3elmar@gmail.com>
Wed, 24 May 2017 08:00:14 +0000 (10:00 +0200)
committerBenni Mack <benni@typo3.org>
Thu, 30 Nov 2017 14:26:39 +0000 (15:26 +0100)
Provides a trait to leverage a smooth migration of public
property access to protected. For the period of deprecation
the access to the protected property is still possible and is
logged by the deprecation log. A unit test is included.

Resolves: #81330
Releases: master
Change-Id: I6293e460053eb38a633271ec877b3bc9a8527342
Reviewed-on: https://review.typo3.org/52928
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/core/Classes/Compatibility/PublicPropertyDeprecationTrait.php [new file with mode: 0644]
typo3/sysext/core/Documentation/Changelog/master/Feature-81330-TraitToMigratePublicAccessToProtectedByDeprecation.rst [new file with mode: 0644]
typo3/sysext/core/Documentation/Changelog/master/Important-81330-DealingWithPropertiesThatAreMigratedToProtected.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit_Deprecated/Compatibility/PublicPropertyDeprecationTraitTest.php [new file with mode: 0644]

diff --git a/typo3/sysext/core/Classes/Compatibility/PublicPropertyDeprecationTrait.php b/typo3/sysext/core/Classes/Compatibility/PublicPropertyDeprecationTrait.php
new file mode 100644 (file)
index 0000000..df2f864
--- /dev/null
@@ -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 (file)
index 0000000..75245c8
--- /dev/null
@@ -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 (file)
index 0000000..3c621f6
--- /dev/null
@@ -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 (file)
index 0000000..4ccd792
--- /dev/null
@@ -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'],
+        ];
+    }
+}