From a706f4d722ef920f372af48967d09268fd97d412 Mon Sep 17 00:00:00 2001
From: Matthew Brown <github@muglug.com>
Date: Sat, 22 Feb 2020 10:04:38 -0500
Subject: [PATCH] Fix #2242 - warn when using mutable dependencies

---
 config.xsd                                    |  1 +
 docs/running_psalm/issues.md                  | 19 +++++
 src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 27 +++++++
 src/Psalm/Issue/MutableDependency.php         |  7 ++
 tests/ImmutableAnnotationTest.php             | 72 +++++++++++++++++++
 5 files changed, 126 insertions(+)
 create mode 100644 src/Psalm/Issue/MutableDependency.php

diff --git a/config.xsd b/config.xsd
index a0f96bac809..2668b825237 100644
--- a/config.xsd
+++ b/config.xsd
@@ -271,6 +271,7 @@
             <xs:element name="MixedPropertyTypeCoercion" type="PropertyIssueHandlerType" minOccurs="0" />
             <xs:element name="MoreSpecificReturnType" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="MoreSpecificImplementedParamType" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="MutableDependency" type="PropertyIssueHandlerType" minOccurs="0" />
             <xs:element name="NoInterfaceProperties" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="NonStaticSelfCall" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="NullableReturnStatement" type="IssueHandlerType" minOccurs="0" />
diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
index e167bc6f88f..2e39f7173b8 100644
--- a/docs/running_psalm/issues.md
+++ b/docs/running_psalm/issues.md
@@ -1433,6 +1433,25 @@ function foo() : B {
 }
 ```
 
+### MutableDependency
+
+Emitted when an immutable class inherits from a class or trait not marked immutable
+
+```php
+class MutableParent {
+    public int $i = 0;
+
+    public function increment() : void {
+        $this->i++;
+    }
+}
+
+/**
+ * @psalm-immutable
+ */
+final class NotReallyImmutableClass extends MutableParent {}
+```
+
 ### NoValue
 
 Emitted when using the result of a function that never returns.
diff --git a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php
index 0c40b0a2f96..6b8006983cb 100644
--- a/src/Psalm/Internal/Analyzer/ClassAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/ClassAnalyzer.php
@@ -22,6 +22,7 @@
 use Psalm\Issue\MissingImmutableAnnotation;
 use Psalm\Issue\MissingPropertyType;
 use Psalm\Issue\MissingTemplateParam;
+use Psalm\Issue\MutableDependency;
 use Psalm\Issue\OverriddenPropertyAccess;
 use Psalm\Issue\PropertyNotSetInConstructor;
 use Psalm\Issue\ReservedWord;
@@ -329,6 +330,20 @@ public function analyze(
                     }
                 }
 
+                if ($storage->mutation_free
+                    && !$parent_class_storage->mutation_free
+                ) {
+                    if (IssueBuffer::accepts(
+                        new MutableDependency(
+                            $fq_class_name . ' is marked immutable but ' . $parent_fq_class_name . ' is not',
+                            $code_location
+                        ),
+                        $storage->suppressed_issues + $this->getSuppressedIssues()
+                    )) {
+                        // fall through
+                    }
+                }
+
                 if ($codebase->store_node_types) {
                     $codebase->analyzer->addNodeReference(
                         $this->getFilePath(),
@@ -1433,6 +1448,18 @@ private function analyzeTraitUse(
                     }
                 }
 
+                if ($storage->mutation_free && !$trait_storage->mutation_free) {
+                    if (IssueBuffer::accepts(
+                        new MutableDependency(
+                            $storage->name . ' is marked immutable but ' . $fq_trait_name . ' is not',
+                            new CodeLocation($this, $trait_name)
+                        ),
+                        $storage->suppressed_issues + $this->getSuppressedIssues()
+                    )) {
+                        // fall through
+                    }
+                }
+
                 $trait_file_analyzer = $project_analyzer->getFileAnalyzerForClassLike($fq_trait_name_resolved);
                 $trait_node = $codebase->classlikes->getTraitNode($fq_trait_name_resolved);
                 $trait_aliases = $codebase->classlikes->getTraitAliases($fq_trait_name_resolved);
diff --git a/src/Psalm/Issue/MutableDependency.php b/src/Psalm/Issue/MutableDependency.php
new file mode 100644
index 00000000000..ecef8c88317
--- /dev/null
+++ b/src/Psalm/Issue/MutableDependency.php
@@ -0,0 +1,7 @@
+<?php
+namespace Psalm\Issue;
+
+class MutableDependency extends CodeIssue
+{
+    const ERROR_LEVEL = 1;
+}
diff --git a/tests/ImmutableAnnotationTest.php b/tests/ImmutableAnnotationTest.php
index 2e8a94245db..11bce6c9782 100644
--- a/tests/ImmutableAnnotationTest.php
+++ b/tests/ImmutableAnnotationTest.php
@@ -333,6 +333,44 @@ public function get(): int {
                     $item = new Item(5);
                     new Immutable($item);',
             ],
+            'preventNonImmutableTraitInImmutableClass' => [
+                '<?php
+                    /**
+                     * @psalm-immutable
+                     */
+                    trait ImmutableTrait {
+                        public int $i = 0;
+
+                        public function __construct(int $i) {
+                            $this->i = $i;
+                        }
+                    }
+
+                    /**
+                     * @psalm-immutable
+                     */
+                    final class NotReallyImmutableClass {
+                        use ImmutableTrait;
+                    }',
+            ],
+            'preventImmutableClassInheritingMutableParent' => [
+                '<?php
+                    /**
+                     * @psalm-immutable
+                     */
+                    class ImmutableParent {
+                        public int $i = 0;
+
+                        public function __construct(int $i) {
+                            $this->i = $i;
+                        }
+                    }
+
+                    /**
+                     * @psalm-immutable
+                     */
+                    final class ImmutableClass extends ImmutableParent {}',
+            ],
         ];
     }
 
@@ -529,6 +567,40 @@ public function get(): int {
                     new Immutable($item);',
                 'error_message' => 'ImpureArgument',
             ],
+            'preventNonImmutableTraitInImmutableClass' => [
+                '<?php
+                    trait MutableTrait {
+                        public int $i = 0;
+
+                        public function increment() : void {
+                            $this->i++;
+                        }
+                    }
+
+                    /**
+                     * @psalm-immutable
+                     */
+                    final class NotReallyImmutableClass {
+                        use MutableTrait;
+                    }',
+                'error_message' => 'MutableDependency'
+            ],
+            'preventImmutableClassInheritingMutableParent' => [
+                '<?php
+                    class MutableParent {
+                        public int $i = 0;
+
+                        public function increment() : void {
+                            $this->i++;
+                        }
+                    }
+
+                    /**
+                     * @psalm-immutable
+                     */
+                    final class NotReallyImmutableClass extends MutableParent {}',
+                'error_message' => 'MutableDependency'
+            ],
         ];
     }
 }