Fix #2242 - warn when using mutable dependencies

vimeo · Feb 22, 2020 · a706f4d · a706f4d
1 parent a50826d
commit a706f4d
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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'
+            ],
         ];
     }
 }