Merge pull request #7129 from vimeo/muglug-detect-unused-properties-w…

…ith-arrays Detect unused properties that are written to inside arrays
vimeo · Dec 11, 2021 · 858be40 · 858be40
2 parents 19ae9e8 + 4229045
commit 858be40
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 56 deletions.
diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php
@@ -95,14 +95,6 @@ class Codebase
      */
     public $use_referencing_locations = [];
 
-    /**
-     * A map of file names to the classes that they contain explicit references to
-     * used in collaboration with use_referencing_locations
-     *
-     * @var array<string, array<lowercase-string, bool>>
-     */
-    public $use_referencing_files = [];
-
     /**
      * @var FileStorageProvider
      */

diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
@@ -235,11 +235,6 @@ class Context
      */
     public $parent_context;
 
-    /**
-     * @var array<string, Type\Union>
-     */
-    public $possible_param_types = [];
-
     /**
      * A list of vars that have been assigned to
      *

diff --git a/src/Psalm/Internal/Analyzer/CanAlias.php b/src/Psalm/Internal/Analyzer/CanAlias.php
@@ -72,8 +72,6 @@ public function visitUse(PhpParser\Node\Stmt\Use_ $stmt): void
                         // register the path
                         $codebase->use_referencing_locations[$use_path_lc][] =
                             new CodeLocation($this, $use);
-
-                        $codebase->use_referencing_files[$this->getFilePath()][$use_path_lc] = true;
                     }
 
                     if ($codebase->alter_code) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
@@ -459,12 +459,6 @@ function (array $carry, Clause $clause): array {
             }
         }
 
-        if ($if_scope->possible_param_types) {
-            foreach ($if_scope->possible_param_types as $var => $type) {
-                $context->possible_param_types[$var] = $type;
-            }
-        }
-
         if ($if_scope->reasonable_clauses
             && (count($if_scope->reasonable_clauses) > 1 || !$if_scope->reasonable_clauses[0]->wedge)
         ) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
@@ -228,7 +228,7 @@ public static function analyze(
 
         $naive_property_exists = $codebase->properties->propertyExists(
             $property_id,
-            true,
+            !$in_assignment,
             $statements_analyzer,
             $context,
             $codebase->collect_locations ? new CodeLocation($statements_analyzer->getSource(), $stmt) : null
@@ -252,7 +252,7 @@ public static function analyze(
                 if ($new_class_storage
                     && ($codebase->properties->propertyExists(
                         $new_property_id,
-                        true,
+                        !$in_assignment,
                         $statements_analyzer,
                         $context,
                         $codebase->collect_locations

diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
@@ -1285,19 +1285,6 @@ private function visitClassConstDeclaration(
             }
 
             $constant_storage->description = $description;
-
-            foreach ($stmt->attrGroups as $attr_group) {
-                foreach ($attr_group->attrs as $attr) {
-                    $constant_storage->attributes[] = AttributeResolver::resolve(
-                        $this->codebase,
-                        $this->file_scanner,
-                        $this->file_storage,
-                        $this->aliases,
-                        $attr,
-                        $this->storage->name ?? null
-                    );
-                }
-            }
         }
     }
 

diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php
@@ -1368,7 +1368,7 @@ private static function handleTemplates(
     ): array {
         $storage->template_types = [];
 
-        foreach ($docblock_info->templates as $i => $template_map) {
+        foreach ($docblock_info->templates as $template_map) {
             $template_name = $template_map[0];
 
             if ($template_map[1] !== null && $template_map[2] !== null) {
@@ -1416,8 +1416,6 @@ private static function handleTemplates(
                     'fn-' . strtolower($cased_function_id) => $template_type,
                 ];
             }
-
-            $storage->template_covariants[$i] = $template_map[3];
         }
 
         return array_merge($template_types ?: [], $storage->template_types);

diff --git a/src/Psalm/Internal/Scope/IfScope.php b/src/Psalm/Internal/Scope/IfScope.php
@@ -74,13 +74,6 @@ class IfScope
      */
     public $reasonable_clauses = [];
 
-    /**
-     * Variables that were mixed, but are no longer
-     *
-     * @var array<string, Type\Union>|null
-     */
-    public $possible_param_types;
-
     /**
      * @var string[]
      */

diff --git a/src/Psalm/Storage/ClassConstantStorage.php b/src/Psalm/Storage/ClassConstantStorage.php
@@ -38,11 +38,6 @@ class ClassConstantStorage
      */
     public $deprecated = false;
 
-    /**
-     * @var list<AttributeStorage>
-     */
-    public $attributes = [];
-
     /**
      * @var ?string
      */

diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php
@@ -120,11 +120,6 @@ abstract class FunctionLikeStorage
      */
     public $template_types;
 
-    /**
-     * @var array<int, bool>|null
-     */
-    public $template_covariants;
-
     /**
      * @var array<int, Assertion>
      */

diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php
@@ -1195,6 +1195,17 @@ public function validate(): void
                     foobar
                 ',
             ],
+            'usedPropertyAsAssignmentKey' => [
+                '<?php
+                    class A {
+                        public string $foo = "bar";
+                        public array $bar = [];
+                    }
+
+                    $a = new A();
+                    $a->bar[$a->foo] = "bar";
+                    print_r($a->bar);',
+            ],
         ];
     }
 
@@ -1248,6 +1259,30 @@ class A {
                 'error_message' => 'PossiblyUnusedProperty',
                 'error_levels' => ['UnusedVariable'],
             ],
+            'possiblyUnusedPropertyWrittenNeverRead' => [
+                '<?php
+                    class A {
+                        /** @var string */
+                        public $foo = "hello";
+                    }
+
+                    $a = new A();
+                    $a->foo = "bar";',
+                'error_message' => 'PossiblyUnusedProperty',
+                'error_levels' => ['UnusedVariable'],
+            ],
+            'possiblyUnusedPropertyWithArrayWrittenNeverRead' => [
+                '<?php
+                    class A {
+                        /** @var list<string> */
+                        public array $foo = [];
+                    }
+
+                    $a = new A();
+                    $a->foo[] = "bar";',
+                'error_message' => 'PossiblyUnusedProperty',
+                'error_levels' => ['UnusedVariable'],
+            ],
             'unusedProperty' => [
                 '<?php
                     class A {