Fix #2729 - allow mutation of otherwise-readonly properties

vimeo · Feb 2, 2020 · 93fc1b1 · 93fc1b1
1 parent 8d7fb2b
commit 93fc1b1
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 2 deletions.
diff --git a/src/Psalm/DocComment.php b/src/Psalm/DocComment.php
@@ -151,7 +151,7 @@ public static function parse($docblock, $line_number = null, $preserve_format =
                         'ignore-variable-method', 'ignore-variable-property', 'internal',
                         'taint-sink', 'taint-source', 'assert-untainted', 'scope-this',
                         'mutation-free', 'external-mutation-free', 'immutable', 'readonly',
-                        'remove-taint',
+                        'remove-taint', 'allow-private-mutation',
                     ],
                     true
                 )) {
@@ -275,7 +275,7 @@ public static function parsePreservingLength(\PhpParser\Comment\Doc $docblock)
                         'ignore-variable-method', 'ignore-variable-property', 'internal',
                         'taint-sink', 'taint-source', 'assert-untainted', 'scope-this',
                         'mutation-free', 'external-mutation-free', 'immutable', 'readonly',
-                        'remove-taint',
+                        'remove-taint', 'allow-private-mutation',
                     ],
                     true
                 )) {

diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php
@@ -182,6 +182,8 @@ public static function arrayToDocblocks(
                 $var_comment->internal = isset($parsed_docblock['specials']['internal']);
                 $var_comment->readonly = isset($parsed_docblock['specials']['readonly'])
                     || isset($parsed_docblock['specials']['psalm-readonly']);
+                $var_comment->allow_private_mutation
+                    = isset($parsed_docblock['specials']['psalm-allow-private-mutation']);
                 $var_comment->remove_taint = isset($parsed_docblock['specials']['psalm-remove-taint']);
 
                 if (isset($parsed_docblock['specials']['psalm-internal'])) {
@@ -214,6 +216,7 @@ public static function arrayToDocblocks(
             $var_comment->internal = isset($parsed_docblock['specials']['internal']);
             $var_comment->readonly = isset($parsed_docblock['specials']['readonly'])
                 || isset($parsed_docblock['specials']['psalm-readonly']);
+            $var_comment->allow_private_mutation = isset($parsed_docblock['specials']['psalm-allow-private-mutation']);
             $var_comment->remove_taint = isset($parsed_docblock['specials']['psalm-remove-taint']);
 
             $var_comments[] = $var_comment;

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
@@ -659,6 +659,7 @@ public static function analyzeInstance(
                                 && (!$context->calling_function_id
                                     || \strpos($context->calling_function_id, '::__construct')
                                     || \strpos($context->calling_function_id, '::unserialize')
+                                    || $property_storage->allow_private_mutation
                                     || $property_pure_compatible)
                             )
                         ) {

diff --git a/src/Psalm/Internal/Scanner/VarDocblockComment.php b/src/Psalm/Internal/Scanner/VarDocblockComment.php
@@ -66,6 +66,13 @@ class VarDocblockComment
      */
     public $readonly = false;
 
+    /**
+     * Whether or not to allow mutation by internal methods
+     *
+     * @var bool
+     */
+    public $allow_private_mutation = false;
+
     /**
      * @var bool
      */

diff --git a/src/Psalm/Internal/Visitor/ReflectorVisitor.php b/src/Psalm/Internal/Visitor/ReflectorVisitor.php
@@ -3214,6 +3214,7 @@ private function visitPropertyDeclaration(
             $property_storage->internal = $var_comment ? $var_comment->internal : false;
             $property_storage->psalm_internal = $var_comment ? $var_comment->psalm_internal : null;
             $property_storage->readonly = $var_comment ? $var_comment->readonly : false;
+            $property_storage->allow_private_mutation = $var_comment ? $var_comment->allow_private_mutation : false;
 
             if (!$signature_type && !$doc_var_group_type) {
                 if ($property->default) {

diff --git a/src/Psalm/Storage/PropertyStorage.php b/src/Psalm/Storage/PropertyStorage.php
@@ -74,6 +74,13 @@ class PropertyStorage
      */
     public $readonly = false;
 
+    /**
+     * Whether or not to allow mutation by internal methods
+     *
+     * @var bool
+     */
+    public $allow_private_mutation = false;
+
     /**
      * @var null|string
      */

diff --git a/tests/PropertyTypeTest.php b/tests/PropertyTypeTest.php
@@ -1698,6 +1698,22 @@ public function __construct() {
 
                     echo (new A)->bar;'
             ],
+            'readonlyWithPrivateMutationsAllowedPropertySetInAnotherMEthod' => [
+                '<?php
+                    class A {
+                        /**
+                         * @readonly
+                         * @psalm-allow-private-mutation
+                         */
+                        public ?string $bar = null;
+
+                        public function setBar(string $s) : void {
+                            $this->bar = $s;
+                        }
+                    }
+
+                    echo (new A)->bar;'
+            ],
             'readonlyPropertySetChildClass' => [
                 '<?php
                     abstract class A {
@@ -2821,6 +2837,28 @@ public function __construct() {
                     $a->bar = "goodbye";',
                 'error_message' => 'InaccessibleProperty',
             ],
+            'readonlyPropertySetInConstructorAndAlsoOutsideClass' => [
+                '<?php
+                    class A {
+                        /**
+                         * @readonly
+                         * @psalm-allow-private-mutation
+                         */
+                        public string $bar;
+
+                        public function __construct() {
+                            $this->bar = "hello";
+                        }
+
+                        public function setAgain() : void {
+                            $this->bar = "hello";
+                        }
+                    }
+
+                    $a = new A();
+                    $a->bar = "goodbye";',
+                'error_message' => 'InaccessibleProperty - src/somefile.php:19:21',
+            ],
             'addNullToMixedAfterNullablePropertyFetch' => [
                 '<?php
                     class A {