Fix #2866 - prevent use of impure __toString via concatenation in pur…

…e contexts
vimeo · Feb 24, 2020 · 0a8bb32 · 0a8bb32
1 parent 3f226e2
commit 0a8bb32
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 7 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -929,6 +929,7 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
         if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Equal
             && $stmt_left_type
             && $stmt_right_type
+            && $context->mutation_free
         ) {
             if ($stmt_left_type->hasString() && $stmt_right_type->hasObjectType()) {
                 foreach ($stmt_right_type->getAtomicTypes() as $atomic_type) {
@@ -944,9 +945,7 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
                             continue;
                         }
 
-                        if ($context->mutation_free
-                            && !$storage->mutation_free
-                        ) {
+                        if (!$storage->mutation_free) {
                             if (IssueBuffer::accepts(
                                 new ImpureMethodCall(
                                     'Cannot call a possibly-mutating method '
@@ -974,9 +973,7 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
                             continue;
                         }
 
-                        if ($context->mutation_free
-                            && !$storage->mutation_free
-                        ) {
+                        if (!$storage->mutation_free) {
                             if (IssueBuffer::accepts(
                                 new ImpureMethodCall(
                                     'Cannot call a possibly-mutating method '
@@ -1834,6 +1831,36 @@ public static function analyzeConcatOp(
                         // fall through
                     }
                 }
+
+                if ($context->mutation_free) {
+                    foreach ($left_type->getAtomicTypes() as $atomic_type) {
+                        if ($atomic_type instanceof TNamedObject) {
+                            try {
+                                $storage = $codebase->methods->getStorage(
+                                    new \Psalm\Internal\MethodIdentifier(
+                                        $atomic_type->value,
+                                        '__tostring'
+                                    )
+                                );
+                            } catch (\UnexpectedValueException $e) {
+                                continue;
+                            }
+
+                            if (!$storage->mutation_free) {
+                                if (IssueBuffer::accepts(
+                                    new ImpureMethodCall(
+                                        'Cannot call a possibly-mutating method '
+                                            . $atomic_type->value . '::__toString from a pure context',
+                                        new CodeLocation($statements_analyzer, $left)
+                                    ),
+                                    $statements_analyzer->getSuppressedIssues()
+                                )) {
+                                    // fall through
+                                }
+                            }
+                        }
+                    }
+                }
             }
 
             foreach ($right_type->getAtomicTypes() as $right_type_part) {
@@ -1880,6 +1907,36 @@ public static function analyzeConcatOp(
                         // fall through
                     }
                 }
+
+                if ($context->mutation_free) {
+                    foreach ($right_type->getAtomicTypes() as $atomic_type) {
+                        if ($atomic_type instanceof TNamedObject) {
+                            try {
+                                $storage = $codebase->methods->getStorage(
+                                    new \Psalm\Internal\MethodIdentifier(
+                                        $atomic_type->value,
+                                        '__tostring'
+                                    )
+                                );
+                            } catch (\UnexpectedValueException $e) {
+                                continue;
+                            }
+
+                            if (!$storage->mutation_free) {
+                                if (IssueBuffer::accepts(
+                                    new ImpureMethodCall(
+                                        'Cannot call a possibly-mutating method '
+                                            . $atomic_type->value . '::__toString from a pure context',
+                                        new CodeLocation($statements_analyzer, $right)
+                                    ),
+                                    $statements_analyzer->getSuppressedIssues()
+                                )) {
+                                    // fall through
+                                }
+                            }
+                        }
+                    }
+                }
             }
 
             if (!$left_type_match

diff --git a/tests/PureAnnotationTest.php b/tests/PureAnnotationTest.php
@@ -400,7 +400,7 @@ public static function zero(): string {
                     }',
                 'error_message' => 'ImpureStaticProperty',
             ],
-            'preventImpureToString' => [
+            'preventImpureToStringViaComparison' => [
                 '<?php
                     class A {
                         public function __toString() {
@@ -418,6 +418,23 @@ function foo(string $s, A $a) : string {
                     }',
                 'error_message' => 'ImpureMethodCall'
             ],
+            'preventImpureToStringViaConcatenation' => [
+                '<?php
+                    class A {
+                        public function __toString() {
+                            echo "hi";
+                            return "bar";
+                        }
+                    }
+
+                    /**
+                     * @psalm-pure
+                     */
+                    function foo(string $s, A $a) : string {
+                        return $a . $s;
+                    }',
+                'error_message' => 'ImpureMethodCall'
+            ],
         ];
     }
 }