From 47faea8ca3220e36f7fabcec29cadbad23435c13 Mon Sep 17 00:00:00 2001
From: Brown <github@muglug.com>
Date: Sun, 23 Aug 2020 10:57:24 -0400
Subject: [PATCH] =?UTF-8?q?Prohibit=20property=20fetches=20from=20pure=20c?=
 =?UTF-8?q?ontexts=20except=20when=20they=E2=80=99re=20on=20immutable=20ob?=
 =?UTF-8?q?jects?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../Fetch/InstancePropertyFetchAnalyzer.php   | 57 +++++++++++++++-
 src/Psalm/Issue/ImpurePropertyFetch.php       |  8 +++
 .../PureAnnotationAdditionTest.php            | 21 ++++++
 tests/PureAnnotationTest.php                  | 68 ++++++++++++++++---
 4 files changed, 144 insertions(+), 10 deletions(-)
 create mode 100644 src/Psalm/Issue/ImpurePropertyFetch.php

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php
index aaaf830bb..afdc7936e 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php
@@ -13,6 +13,7 @@ use Psalm\Internal\Type\TemplateResult;
 use Psalm\CodeLocation;
 use Psalm\Context;
 use Psalm\Issue\DeprecatedProperty;
+use Psalm\Issue\ImpurePropertyFetch;
 use Psalm\Issue\InvalidPropertyFetch;
 use Psalm\Issue\InternalProperty;
 use Psalm\Issue\MissingPropertyType;
@@ -179,12 +180,14 @@ class InstancePropertyFetchAnalyzer
 
                         $property_id = $lhs_type_part->value . '::$' . $stmt->name->name;
 
+                        $class_storage = $codebase->classlike_storage_provider->get($lhs_type_part->value);
+
                         self::processTaints(
                             $statements_analyzer,
                             $stmt,
                             $stmt_type,
                             $property_id,
-                            $codebase->classlike_storage_provider->get($lhs_type_part->value),
+                            $class_storage,
                             $in_assignment
                         );
 
@@ -208,6 +211,32 @@ class InstancePropertyFetchAnalyzer
                                 $property_id
                             );
                         }
+
+                        if (!$context->collect_mutations
+                            && !$context->collect_initializations
+                            && !($class_storage->external_mutation_free
+                                && $stmt_type->allow_mutations)
+                        ) {
+                            $project_analyzer = $statements_analyzer->getProjectAnalyzer();
+
+                            if ($context->pure) {
+                                if (IssueBuffer::accepts(
+                                    new ImpurePropertyFetch(
+                                        'Cannot access a property on a mutable object from a pure context',
+                                        new CodeLocation($statements_analyzer, $stmt)
+                                    ),
+                                    $statements_analyzer->getSuppressedIssues()
+                                )) {
+                                    // fall through
+                                }
+                            } elseif ($codebase->alter_code
+                                && isset($project_analyzer->getIssuesToFix()['MissingPureAnnotation'])
+                                && $statements_analyzer->getSource()
+                                    instanceof \Psalm\Internal\Analyzer\FunctionLikeAnalyzer
+                            ) {
+                                $statements_analyzer->getSource()->inferred_impure = true;
+                            }
+                        }
                     }
                 }
             }
@@ -956,6 +985,32 @@ class InstancePropertyFetchAnalyzer
                 }
             }
 
+            if (!$context->collect_mutations
+                && !$context->collect_initializations
+                && !($class_storage->external_mutation_free
+                    && $class_property_type->allow_mutations)
+            ) {
+                $project_analyzer = $statements_analyzer->getProjectAnalyzer();
+
+                if ($context->pure) {
+                    if (IssueBuffer::accepts(
+                        new ImpurePropertyFetch(
+                            'Cannot access a property on a mutable object from a pure context',
+                            new CodeLocation($statements_analyzer, $stmt)
+                        ),
+                        $statements_analyzer->getSuppressedIssues()
+                    )) {
+                        // fall through
+                    }
+                } elseif ($codebase->alter_code
+                    && isset($project_analyzer->getIssuesToFix()['MissingPureAnnotation'])
+                    && $statements_analyzer->getSource()
+                        instanceof \Psalm\Internal\Analyzer\FunctionLikeAnalyzer
+                ) {
+                    $statements_analyzer->getSource()->inferred_impure = true;
+                }
+            }
+
             self::processTaints(
                 $statements_analyzer,
                 $stmt,
diff --git a/src/Psalm/Issue/ImpurePropertyFetch.php b/src/Psalm/Issue/ImpurePropertyFetch.php
new file mode 100644
index 000000000..906ef46df
--- /dev/null
+++ b/src/Psalm/Issue/ImpurePropertyFetch.php
@@ -0,0 +1,8 @@
+<?php
+namespace Psalm\Issue;
+
+class ImpurePropertyFetch extends CodeIssue
+{
+    const ERROR_LEVEL = -1;
+    const SHORTCODE = 234;
+}
diff --git a/tests/FileManipulation/PureAnnotationAdditionTest.php b/tests/FileManipulation/PureAnnotationAdditionTest.php
index ca3d0338e..89144c9e1 100644
--- a/tests/FileManipulation/PureAnnotationAdditionTest.php
+++ b/tests/FileManipulation/PureAnnotationAdditionTest.php
@@ -40,6 +40,27 @@ class PureAnnotationAdditionTest extends FileManipulationTest
                 ['MissingPureAnnotation'],
                 true,
             ],
+            'dontAddPureAnnotationToMutationFreeMethod' => [
+                '<?php
+                    class A {
+                        public string $foo = "hello";
+
+                        public function getFoo() : string {
+                            return $this->foo;
+                        }
+                    }',
+                '<?php
+                    class A {
+                        public string $foo = "hello";
+
+                        public function getFoo() : string {
+                            return $this->foo;
+                        }
+                    }',
+                '7.4',
+                ['MissingPureAnnotation'],
+                true,
+            ],
         ];
     }
 }
diff --git a/tests/PureAnnotationTest.php b/tests/PureAnnotationTest.php
index 2c6e15f30..745e57744 100644
--- a/tests/PureAnnotationTest.php
+++ b/tests/PureAnnotationTest.php
@@ -18,18 +18,12 @@ class PureAnnotationTest extends TestCase
                 '<?php
                     namespace Bar;
 
-                    class A {
-                        public int $a = 5;
-                    }
-
                     /** @psalm-pure */
-                    function filterOdd(int $i, A $a) : ?int {
-                        if ($i % 2 === 0 || $a->a === 2) {
+                    function filterOdd(int $i) : ?int {
+                        if ($i % 2 === 0) {
                             return $i;
                         }
 
-                        $a = new A();
-
                         return null;
                     }',
             ],
@@ -345,6 +339,28 @@ class PureAnnotationTest extends TestCase
                         }
                     }'
             ],
+            'allowPropertyAccessOnImmutableClass' => [
+                '<?php
+                    namespace Bar;
+
+                    /** @psalm-immutable */
+                    class A {
+                        public int $a;
+
+                        public function __construct(int $a) {
+                            $this->a = $a;
+                        }
+                    }
+
+                    /** @psalm-pure */
+                    function filterOdd(A $a) : bool {
+                        if ($a->a % 2 === 0) {
+                            return true;
+                        }
+
+                        return false;
+                    }',
+            ],
         ];
     }
 
@@ -364,7 +380,7 @@ class PureAnnotationTest extends TestCase
 
                     /** @psalm-pure */
                     function filterOdd(int $i, A $a) : ?int {
-                        $a->a++;
+                        $a->a = $i;
 
                         if ($i % 2 === 0 || $a->a === 2) {
                             return $i;
@@ -598,6 +614,40 @@ class PureAnnotationTest extends TestCase
                     }',
                 'error_message' => 'ImpureFunctionCall',
             ],
+            'propertyFetchIsNotPure' => [
+                '<?php
+                    class A {
+                        public string $foo = "hello";
+
+                        /** @psalm-pure */
+                        public function getFoo() : string {
+                            return $this->foo;
+                        }
+                    }',
+                'error_message' => 'ImpurePropertyFetch',
+            ],
+            'preventPropertyAccessOnMutableClass' => [
+                '<?php
+                    namespace Bar;
+
+                    class A {
+                        public int $a;
+
+                        public function __construct(int $a) {
+                            $this->a = $a;
+                        }
+                    }
+
+                    /** @psalm-pure */
+                    function filterOdd(A $a) : bool {
+                        if ($a->a % 2 === 0) {
+                            return true;
+                        }
+
+                        return false;
+                    }',
+                'error_message' => 'ImpurePropertyFetch',
+            ],
         ];
     }
 }