Immutable unions (#8627)

* Immutable CodeLocation * Remove excess clones * Remove external clones * Remove leftover clones * Fix final clone issue * Immutable storages * Refactoring * Fixes * Fixes * Fix * Fix * Fixes * Simplify * Fixes * Fix * Fixes * Update * Fix * Cache global types * Fix * Update * Update * Fixes * Fixes * Refactor * Fixes * Fix * Fix * More caching * Fix * Fix * Update * Update * Fix * Fixes * Update * Refactor * Update * Fixes * Break one more test * Fix * FIx * Fix * Fix * Fix * Fix * Improve performance and readability * Equivalent logic * Fixes * Revert * Revert "Revert" This reverts commit f917510. * Fix * Fix reference bug * Make default TypeVisitor immutable * Bugfix * Remove clones * Partial refactoring * Refactoring * Fixes * Fix * Fixes * Fixes * cs-fix * Fix final bugs * Add test * Misc fixes * Update * Fixes * Experiment with removing different property * revert "Experiment with removing different property" This reverts commit ac1156e. * Uniform naming * Uniform naming * Hack hotfix * Clean up $_FILES ref #8621 * Undo hack, try fixing properly * Helper method * Remove redundant call * Partially fix bugs * Cleanup * Change defaults * Fix bug * Fix (?, hope this doesn't break anything else) * cs-fix * Review fixes * Bugfix * Bugfix * Improve logic * Update
vimeo · Nov 4, 2022 · d0be59e · d0be59e
1 parent 51838a5
commit d0be59e
Show file tree

Hide file tree

Showing 202 changed files with 2,388 additions and 1,603 deletions.
diff --git a/composer.json b/composer.json
@@ -50,7 +50,7 @@
         "phpspec/prophecy": "^1.15",
         "phpspec/prophecy-phpunit": "^2.0",
         "phpunit/phpunit": "^9.5",
-        "psalm/plugin-phpunit": "^0.16",
+        "psalm/plugin-phpunit": "^0.18",
         "slevomat/coding-standard": "^7.0",
         "phpstan/phpdoc-parser": "1.6.4",
         "squizlabs/php_codesniffer": "^3.6",

diff --git a/examples/plugins/ClassUnqualifier.php b/examples/plugins/ClassUnqualifier.php
@@ -37,6 +37,7 @@ public static function afterClassLikeExistenceCheck(
                     $type_token[0] = $aliases[strtolower($fq_class_name)];
                 }
             }
+            unset($type_token);
 
             $new_candidate_type = implode(
                 '',

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -46,6 +46,10 @@
     </PossiblyUndefinedIntArrayOffset>
   </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php">
+    <ConflictingReferenceConstraint occurrences="2">
+      <code>if (AtomicTypeComparator::isContainedBy(</code>
+      <code>if (AtomicTypeComparator::isContainedBy(</code>
+    </ConflictingReferenceConstraint>
     <PossiblyUndefinedIntArrayOffset occurrences="1">
       <code>$iterator_atomic_type-&gt;type_params[1]</code>
     </PossiblyUndefinedIntArrayOffset>
@@ -150,6 +154,18 @@
       <code>$callable_arg-&gt;items[1]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php">
+    <ReferenceConstraintViolation occurrences="3">
+      <code>$stmt_type</code>
+      <code>$stmt_type</code>
+      <code>$stmt_type</code>
+    </ReferenceConstraintViolation>
+  </file>
+  <file src="src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php">
+    <ReferenceConstraintViolation occurrences="1">
+      <code>$stmt_type</code>
+    </ReferenceConstraintViolation>
+  </file>
   <file src="src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php">
     <PossiblyUndefinedIntArrayOffset occurrences="1">
       <code>$invalid_fetch_types[0]</code>
@@ -293,16 +309,10 @@
     </MoreSpecificReturnType>
   </file>
   <file src="src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php">
-    <ImpureMethodCall occurrences="5">
-      <code>get</code>
-      <code>get</code>
-      <code>get</code>
+    <ImpureMethodCall occurrences="2">
       <code>getClassTemplateTypes</code>
       <code>has</code>
     </ImpureMethodCall>
-    <ImpurePropertyAssignment occurrences="1">
-      <code>$candidate_param_type-&gt;from_template_default</code>
-    </ImpurePropertyAssignment>
   </file>
   <file src="src/Psalm/Internal/Type/TypeCombiner.php">
     <PossiblyUndefinedIntArrayOffset occurrences="6">
@@ -326,8 +336,21 @@
       <code>array_keys($template_type_map[$template_param_name])[0]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Storage/ClassConstantStorage.php">
+    <MutableDependency occurrences="1">
+      <code>CustomMetadataTrait</code>
+    </MutableDependency>
+  </file>
+  <file src="src/Psalm/Storage/FunctionLikeParameter.php">
+    <ImpureMethodCall occurrences="4">
+      <code>traverse</code>
+      <code>traverse</code>
+      <code>traverse</code>
+      <code>traverse</code>
+    </ImpureMethodCall>
+  </file>
   <file src="src/Psalm/Type/Atomic.php">
-    <ImpureMethodCall occurrences="10">
+    <ImpureMethodCall occurrences="12">
       <code>classExtendsOrImplements</code>
       <code>classExtendsOrImplements</code>
       <code>classExtendsOrImplements</code>
@@ -338,10 +361,15 @@
       <code>interfaceExtends</code>
       <code>interfaceExtends</code>
       <code>interfaceExtends</code>
+      <code>traverse</code>
+      <code>traverse</code>
     </ImpureMethodCall>
     <PossiblyUndefinedIntArrayOffset occurrences="1">
       <code>array_keys($template_type_map[$value])[0]</code>
     </PossiblyUndefinedIntArrayOffset>
+    <ReferenceConstraintViolation occurrences="1">
+      <code>$value</code>
+    </ReferenceConstraintViolation>
   </file>
   <file src="src/Psalm/Type/Atomic/CallableTrait.php">
     <ImpureMethodCall occurrences="4">
@@ -400,10 +428,8 @@
       <code>replace</code>
       <code>replace</code>
     </ImpureMethodCall>
-    <ImpurePropertyAssignment occurrences="3">
+    <ImpurePropertyAssignment occurrences="1">
       <code>$key_type-&gt;possibly_undefined</code>
-      <code>$value_type-&gt;possibly_undefined</code>
-      <code>$value_type-&gt;possibly_undefined</code>
     </ImpurePropertyAssignment>
   </file>
   <file src="src/Psalm/Type/Atomic/TList.php">
@@ -420,10 +446,6 @@
       <code>replace</code>
       <code>replace</code>
     </ImpureMethodCall>
-    <ImpurePropertyFetch occurrences="2">
-      <code>$type-&gt;possibly_undefined</code>
-      <code>$type-&gt;possibly_undefined</code>
-    </ImpurePropertyFetch>
   </file>
   <file src="src/Psalm/Type/Atomic/TTemplateKeyOf.php">
     <ImpureMethodCall occurrences="1">
@@ -457,12 +479,28 @@
       <code>$type[0][0]</code>
     </PossiblyUndefinedIntArrayOffset>
   </file>
+  <file src="src/Psalm/Type/TypeNode.php">
+    <ReferenceConstraintViolation occurrences="1">
+      <code>$node</code>
+    </ReferenceConstraintViolation>
+  </file>
+  <file src="src/Psalm/Type/TypeVisitor.php">
+    <ImpureMethodCall occurrences="1">
+      <code>visit</code>
+    </ImpureMethodCall>
+  </file>
   <file src="src/Psalm/Type/Union.php">
     <PossiblyUnusedProperty occurrences="1">
       <code>$ignore_isset</code>
     </PossiblyUnusedProperty>
   </file>
   <file src="src/Psalm/Type/UnionTrait.php">
+    <ImpureMethodCall occurrences="4">
+      <code>traverse</code>
+      <code>traverse</code>
+      <code>traverseArray</code>
+      <code>traverseArray</code>
+    </ImpureMethodCall>
     <PossiblyUnusedMethod occurrences="2">
       <code>allFloatLiterals</code>
       <code>allFloatLiterals</code>

diff --git a/src/Psalm/Aliases.php b/src/Psalm/Aliases.php
@@ -54,6 +54,8 @@ final class Aliases
      * @param array<lowercase-string, string> $functions_flipped
      * @param array<string, string> $constants_flipped
      * @internal
+     *
+     * @psalm-mutation-free
      */
     public function __construct(
         ?string $namespace = null,

diff --git a/src/Psalm/CodeLocation.php b/src/Psalm/CodeLocation.php
@@ -7,6 +7,7 @@
 use PhpParser;
 use Psalm\Internal\Analyzer\CommentAnalyzer;
 use Psalm\Internal\Analyzer\ProjectAnalyzer;
+use Psalm\Storage\ImmutableNonCloneableTrait;
 use UnexpectedValueException;
 
 use function explode;
@@ -25,8 +26,13 @@
 
 use const PREG_OFFSET_CAPTURE;
 
+/**
+ * @psalm-immutable
+ */
 class CodeLocation
 {
+    use ImmutableNonCloneableTrait;
+
     /** @var string */
     public $file_path;
 
@@ -85,7 +91,7 @@ class CodeLocation
     private $docblock_start_line_number;
 
     /** @var int|null */
-    private $docblock_line_number;
+    protected $docblock_line_number;
 
     /** @var null|int */
     private $regex_type;
@@ -111,9 +117,12 @@ public function __construct(
         ?CodeLocation $previous_location = null,
         bool $single_line = false,
         ?int $regex_type = null,
-        ?string $selected_text = null
+        ?string $selected_text = null,
+        ?int $comment_line = null
     ) {
+        /** @psalm-suppress ImpureMethodCall Actually mutation-free just not marked */
         $this->file_start = (int)$stmt->getAttribute('startFilePos');
+        /** @psalm-suppress ImpureMethodCall Actually mutation-free just not marked */
         $this->file_end = (int)$stmt->getAttribute('endFilePos');
         $this->raw_file_start = $this->file_start;
         $this->raw_file_end = $this->file_end;
@@ -124,21 +133,38 @@ public function __construct(
         $this->previous_location = $previous_location;
         $this->text = $selected_text;
 
+        /** @psalm-suppress ImpureMethodCall Actually mutation-free just not marked */
         $doc_comment = $stmt->getDocComment();
 
         $this->docblock_start = $doc_comment ? $doc_comment->getStartFilePos() : null;
         $this->docblock_start_line_number = $doc_comment ? $doc_comment->getStartLine() : null;
 
         $this->preview_start = $this->docblock_start ?: $this->file_start;
 
+        /** @psalm-suppress ImpureMethodCall Actually mutation-free just not marked */
         $this->raw_line_number = $stmt->getLine();
+
+        $this->docblock_line_number = $comment_line;
     }
 
-    public function setCommentLine(int $line): void
+    /**
+     * @psalm-suppress PossiblyUnusedMethod Part of public API
+     * @return static
+     */
+    public function setCommentLine(?int $line): self
     {
-        $this->docblock_line_number = $line;
+        if ($line === $this->docblock_line_number) {
+            return $this;
+        }
+        $cloned = clone $this;
+        $cloned->docblock_line_number = $line;
+        return $cloned;
     }
 
+    /**
+     * @psalm-external-mutation-free
+     * @psalm-suppress InaccessibleProperty Mainly used for caching
+     */
     private function calculateRealLocation(): void
     {
         if ($this->have_recalculated) {

diff --git a/src/Psalm/CodeLocation/DocblockTypeLocation.php b/src/Psalm/CodeLocation/DocblockTypeLocation.php
@@ -5,6 +5,7 @@
 use Psalm\CodeLocation;
 use Psalm\FileSource;
 
+/** @psalm-immutable */
 class DocblockTypeLocation extends CodeLocation
 {
     public function __construct(
@@ -26,5 +27,7 @@ public function __construct(
         $this->single_line = false;
 
         $this->preview_start = $this->file_start;
+
+        $this->docblock_line_number = $line_number;
     }
 }
diff --git a/src/Psalm/CodeLocation/ParseErrorLocation.php b/src/Psalm/CodeLocation/ParseErrorLocation.php
@@ -8,6 +8,7 @@
 use function substr;
 use function substr_count;
 
+/** @psalm-immutable */
 class ParseErrorLocation extends CodeLocation
 {
     public function __construct(
@@ -16,9 +17,9 @@ public function __construct(
         string $file_path,
         string $file_name
     ) {
-        /** @psalm-suppress PossiblyUndefinedStringArrayOffset */
+        /** @psalm-suppress PossiblyUndefinedStringArrayOffset, ImpureMethodCall */
         $this->file_start = (int)$error->getAttributes()['startFilePos'];
-        /** @psalm-suppress PossiblyUndefinedStringArrayOffset */
+        /** @psalm-suppress PossiblyUndefinedStringArrayOffset, ImpureMethodCall */
         $this->file_end = (int)$error->getAttributes()['endFilePos'];
         $this->raw_file_start = $this->file_start;
         $this->raw_file_end = $this->file_end;

diff --git a/src/Psalm/CodeLocation/Raw.php b/src/Psalm/CodeLocation/Raw.php
@@ -7,6 +7,7 @@
 use function substr;
 use function substr_count;
 
+/** @psalm-immutable */
 class Raw extends CodeLocation
 {
     public function __construct(

diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php
@@ -507,6 +507,7 @@ public function scanFiles(int $threads = 1): void
         }
     }
 
+    /** @psalm-mutation-free */
     public function getFileContents(string $file_path): string
     {
         return $this->file_provider->getContents($file_path);
@@ -1951,9 +1952,9 @@ public function addTaintSource(
         string $taint_id,
         array $taints = TaintKindGroup::ALL_INPUT,
         ?CodeLocation $code_location = null
-    ): void {
+    ): Union {
         if (!$this->taint_flow_graph) {
-            return;
+            return $expr_type;
         }
 
         $source = new TaintSource(
@@ -1966,7 +1967,7 @@ public function addTaintSource(
 
         $this->taint_flow_graph->addSource($source);
 
-        $expr_type->parent_nodes[$source->id] = $source;
+        return $expr_type->addParentNodes([$source->id => $source]);
     }
 
     /**

diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
@@ -432,17 +432,6 @@ public function __destruct()
         $this->case_scope = null;
     }
 
-    public function __clone()
-    {
-        foreach ($this->clauses as &$clause) {
-            $clause = clone $clause;
-        }
-
-        foreach ($this->constants as &$constant) {
-            $constant = clone $constant;
-        }
-    }
-
     /**
      * Updates the parent context, looking at the changes within a block and then applying those changes, where
      * necessary, to the parent context
@@ -471,15 +460,13 @@ public function update(
 
                 if (!$existing_type) {
                     if ($new_type) {
-                        $this->vars_in_scope[$var_id] = clone $new_type;
+                        $this->vars_in_scope[$var_id] = $new_type;
                         $updated_vars[$var_id] = true;
                     }
 
                     continue;
                 }
 
-                $existing_type = clone $existing_type;
-
                 // if the type changed within the block of statements, process the replacement
                 // also never allow ourselves to remove all types from a union
                 if ((!$new_type || !$old_type->equals($new_type))
@@ -543,7 +530,12 @@ public function getRedefinedVars(array $new_vars_in_scope, bool $include_new_var
 
             $new_type = $new_vars_in_scope[$var_id];
 
-            if (!$this_type->equals($new_type)) {
+            if (!$this_type->equals(
+                $new_type,
+                true,
+                !($this_type->propagate_parent_nodes || $new_type->propagate_parent_nodes)
+            )
+            ) {
                 $redefined_vars[$var_id] = $this_type;
             }
         }
@@ -720,7 +712,7 @@ public static function filterClauses(
 
                     $result_type = AssertionReconciler::reconcile(
                         $assertion,
-                        clone $new_type,
+                        $new_type,
                         null,
                         $statements_analyzer,
                         false,

diff --git a/src/Psalm/ErrorBaseline.php b/src/Psalm/ErrorBaseline.php
@@ -230,6 +230,7 @@ static function (array $carry, IssueData $issue): array {
         foreach ($groupedIssues as &$issues) {
             ksort($issues);
         }
+        unset($issues);
 
         return $groupedIssues;
     }