Correct analyze clone expression (#3382)

* Correct analyze clone, add PossibleInvalidClone issue type

* Infer mixed type when possible incorrect clone

* Remove unused variable

config.xsd

@@ -301,6 +301,7 @@
             <xs:element name="PossiblyInvalidArrayAssignment" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidArrayOffset" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidCast" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="PossiblyInvalidClone" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidFunctionCall" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidIterator" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidMethodCall" type="IssueHandlerType" minOccurs="0" />

docs/running_psalm/error_levels.md

@@ -147,6 +147,7 @@ These issues are treated as errors at level 3 and below.
  - [PossiblyInvalidArrayAssignment](issues/PossiblyInvalidArrayAssignment.md)
  - [PossiblyInvalidArrayOffset](issues/PossiblyInvalidArrayOffset.md)
  - [PossiblyInvalidCast](issues/PossiblyInvalidCast.md)
+ - [PossiblyInvalidClone](issues/PossiblyInvalidClone.md)
  - [PossiblyInvalidFunctionCall](issues/PossiblyInvalidFunctionCall.md)
  - [PossiblyInvalidIterator](issues/PossiblyInvalidIterator.md)
  - [PossiblyInvalidMethodCall](issues/PossiblyInvalidMethodCall.md)

docs/running_psalm/issues/PossiblyInvalidClone.md

@@ -0,0 +1,16 @@
+# PossiblyInvalidClone
+
+Emitted when trying to clone a value that's possibly not cloneable
+
+```php
+<?php
+
+class A {}
+
+/**
+ * @param A|string $a
+ */
+function foo($a) {
+    return clone $a;
+}
+```

src/Psalm/Internal/Analyzer/Statements/Expression/CloneAnalyzer.php

@@ -2,11 +2,13 @@
 namespace Psalm\Internal\Analyzer\Statements\Expression;
 
 use PhpParser;
+use Psalm\Internal\Analyzer\MethodAnalyzer;
 use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\CodeLocation;
 use Psalm\Context;
 use Psalm\Issue\InvalidClone;
+use Psalm\Issue\PossiblyInvalidClone;
 use Psalm\IssueBuffer;
 use Psalm\Type;
 use Psalm\Type\Atomic\TTemplateParam;
@@ -21,6 +23,7 @@ class CloneAnalyzer
         PhpParser\Node\Expr\Clone_ $stmt,
         Context $context
     ) : bool {
+        $codebase_methods = $statements_analyzer->getCodebase()->methods;
         if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->expr, $context) === false) {
             return false;
         }
@@ -32,12 +35,42 @@ class CloneAnalyzer
 
             $immutable_cloned = false;
 
-            foreach ($clone_type->getAtomicTypes() as $clone_type_part) {
+            $invalid_clones = [];
-                if (!$clone_type_part instanceof TNamedObject
+            $possibly_valid = false;
-                    && !$clone_type_part instanceof TObject
+            $atomic_types = $clone_type->getAtomicTypes();
-                    && !$clone_type_part instanceof TMixed
+            while ($atomic_types) {
-                    && !$clone_type_part instanceof TTemplateParam
+                $clone_type_part = \array_pop($atomic_types);
+
+                if ($clone_type_part instanceof TMixed ||
+                    $clone_type_part instanceof TObject
                 ) {
+                    $invalid_clones[] = $clone_type_part->getId();
+                    $possibly_valid = true;
+                } elseif ($clone_type_part instanceof TNamedObject) {
+                    $clone_method_id = new \Psalm\Internal\MethodIdentifier(
+                        $clone_type_part->value,
+                        '__clone'
+                    );
+
+                    $does_method_exist = $codebase_methods->methodExists(
+                        $clone_method_id,
+                        $context->calling_method_id,
+                        new CodeLocation($statements_analyzer->getSource(), $stmt)
+                    );
+                    $is_method_visible = MethodAnalyzer::isMethodVisible(
+                        $clone_method_id,
+                        $context,
+                        $statements_analyzer->getSource()
+                    );
+                    if ($does_method_exist && !$is_method_visible) {
+                        $invalid_clones[] = $clone_type_part->getId();
+                    } else {
+                        $possibly_valid = true;
+                        $immutable_cloned = true;
+                    }
+                } elseif ($clone_type_part instanceof TTemplateParam) {
+                    $atomic_types = array_merge($atomic_types, $clone_type_part->as->getAtomicTypes());
+                } else {
                     if ($clone_type_part instanceof Type\Atomic\TFalse
                         && $clone_type->ignore_falsable_issues
                     ) {
@@ -50,22 +83,34 @@ class CloneAnalyzer
                         continue;
                     }
 
+                    $invalid_clones[] = $clone_type_part->getId();
+                }
+            }
+
+            if ($invalid_clones) {
+                if ($possibly_valid) {
                     if (IssueBuffer::accepts(
-                        new InvalidClone(
+                        new PossiblyInvalidClone(
-                            'Cannot clone ' . $clone_type_part,
+                            'Cannot clone ' . $invalid_clones[0],
                             new CodeLocation($statements_analyzer->getSource(), $stmt)
                         ),
                         $statements_analyzer->getSuppressedIssues()
                     )) {
-                        return false;
+                        // fall through
+                    }
+                } else {
+                    if (IssueBuffer::accepts(
+                        new InvalidClone(
+                            'Cannot clone ' . $invalid_clones[0],
+                            new CodeLocation($statements_analyzer->getSource(), $stmt)
+                        ),
+                        $statements_analyzer->getSuppressedIssues()
+                    )) {
+                        // fall through
                     }
-
-                    return true;
                 }
 
-                if ($clone_type_part instanceof TNamedObject) {
+                return true;
-                    $immutable_cloned = true;
-                }
             }
 
             $statements_analyzer->node_data->setType($stmt, $stmt_expr_type);

src/Psalm/Issue/PossiblyInvalidClone.php

@@ -0,0 +1,8 @@
+<?php
+namespace Psalm\Issue;
+
+class PossiblyInvalidClone extends CodeIssue
+{
+    const ERROR_LEVEL = 3;
+    const SHORTCODE = 226;
+}

tests/CloneTest.php

@@ -0,0 +1,114 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\Tests;
+
+class CloneTest extends TestCase
+{
+    use Traits\InvalidCodeAnalysisTestTrait;
+    use Traits\ValidCodeAnalysisTestTrait;
+
+    /**
+     * @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
+     */
+    public function providerValidCodeParse()
+    {
+        return [
+            'cloneCorrect' => [
+                '<?php
+                    class A {}
+                    function foo(A $a) : A {
+                        return clone $a;
+                    }
+                    $a = foo(new A());',
+            ],
+            'cloneCorrectWithPublicMethod' => [
+                '<?php
+                    class A {
+                        public function __clone() {}
+                    }
+                    function foo(A $a) : A {
+                        return clone $a;
+                    }
+                    foo(new A());',
+            ],
+            'clonePrivateInternally' => [
+                '<?php
+                    class A {
+                        private function __clone() {}
+                        public function foo(): self {
+                            return clone $this;
+                        }
+                    }',
+            ],
+        ];
+    }
+
+    /**
+     * @return iterable<string,array{string,error_message:string,2?:string[],3?:bool,4?:string}>
+     */
+    public function providerInvalidCodeParse()
+    {
+        return [
+            'invalidIntClone' => [
+                '<?php
+                    $a = 5;
+                    clone $a;',
+                'error_message' => 'InvalidClone',
+            ],
+            'invalidMixedClone' => [
+                '<?php
+                    /** @var mixed $a */
+                    $a = 5;
+                    clone $a;',
+                'error_message' => 'PossiblyInvalidClone',
+            ],
+            'notVisibleCloneMethod' => [
+                '<?php
+                    class A {
+                        private function __clone() {}
+                    }
+                    $a = new A();
+                    clone $a;',
+                'error_message' => 'InvalidClone',
+            ],
+            'invalidGenericClone' => [
+                '<?php
+                    /**
+                     * @template T as int|string
+                     * @param T $a
+                     */
+                    function foo($a): void {
+                        clone $a;
+                    }',
+                'error_message' => 'InvalidClone',
+            ],
+            'possiblyInvalidGenericClone' => [
+                '<?php
+                    /**
+                     * @template T
+                     * @param T $a
+                     */
+                    function foo($a): void {
+                        clone $a;
+                    }',
+                'error_message' => 'PossiblyInvalidClone',
+            ],
+            'mixedTypeInferredIfErrors' => [
+                '<?php
+                    class A {}
+                    /**
+                     * @param A|string $a
+                     */
+                    function foo($a): void {
+                        /**
+                         * @psalm-suppress PossiblyInvalidClone
+                         */
+                        $cloned = clone $a;
+                    }',
+                'error_message' => 'MixedAssignment',
+            ]
+        ];
+    }
+}