Merge pull request #6997 from zoonru/list_values_paradox

vimeo · Nov 28, 2021 · 28a7565 · 28a7565
2 parents 8bc71fc + 925df22
commit 28a7565
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 38 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
@@ -130,28 +130,26 @@ public static function analyze(
             $if_clauses = [];
         }
 
-        $if_clauses = array_values(
-            array_map(
-                /**
-                 * @return Clause
-                 */
-                function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
-                    $keys = array_keys($c->possibilities);
-
-                    $mixed_var_ids = \array_diff($mixed_var_ids, $keys);
-
-                    foreach ($keys as $key) {
-                        foreach ($mixed_var_ids as $mixed_var_id) {
-                            if (preg_match('/^' . preg_quote($mixed_var_id, '/') . '(\[|-)/', $key)) {
-                                return new Clause([], $cond_object_id, $cond_object_id, true);
-                            }
+        $if_clauses = array_map(
+            /**
+             * @return Clause
+             */
+            function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
+                $keys = array_keys($c->possibilities);
+
+                $mixed_var_ids = \array_diff($mixed_var_ids, $keys);
+
+                foreach ($keys as $key) {
+                    foreach ($mixed_var_ids as $mixed_var_id) {
+                        if (preg_match('/^' . preg_quote($mixed_var_id, '/') . '(\[|-)/', $key)) {
+                            return new Clause([], $cond_object_id, $cond_object_id, true);
                         }
                     }
+                }
 
-                    return $c;
-                },
-                $if_clauses
-            )
+                return $c;
+            },
+            $if_clauses
         );
 
         $entry_clauses = $context->clauses;

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php
@@ -364,6 +364,39 @@ function (array $_) : bool {
             return;
         }
 
+        if ($first_arg && $function_id === 'array_values') {
+            $first_arg_type = $statements_analyzer->node_data->getType($first_arg->value);
+
+            if ($first_arg_type
+                && UnionTypeComparator::isContainedBy(
+                    $codebase,
+                    $first_arg_type,
+                    Type::getList()
+                )
+            ) {
+                if ($first_arg_type->from_docblock) {
+                    if (IssueBuffer::accepts(
+                        new \Psalm\Issue\RedundantCastGivenDocblockType(
+                            'The call to array_values is unnecessary given the list docblock type '.$first_arg_type,
+                            new CodeLocation($statements_analyzer, $function_name)
+                        ),
+                        $statements_analyzer->getSuppressedIssues()
+                    )) {
+                        // fall through
+                    }
+                } else {
+                    if (IssueBuffer::accepts(
+                        new \Psalm\Issue\RedundantCast(
+                            'The call to array_values is unnecessary, '.$first_arg_type.' is already a list',
+                            new CodeLocation($statements_analyzer, $function_name)
+                        ),
+                        $statements_analyzer->getSuppressedIssues()
+                    )) {
+                        // fall through
+                    }
+                }
+            }
+        }
         if ($first_arg && $function_id === 'strtolower') {
             $first_arg_type = $statements_analyzer->node_data->getType($first_arg->value);
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php
@@ -209,7 +209,7 @@ public static function analyze(
             }
 
             $all_match_condition = self::convertCondsToConditional(
-                \array_values($all_conds),
+                $all_conds,
                 $match_condition,
                 $match_condition->getAttributes()
             );

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
@@ -83,28 +83,26 @@ public static function analyze(
             }
         }
 
-        $if_clauses = array_values(
-            array_map(
+        $if_clauses = array_map(
                 /**
                  * @return \Psalm\Internal\Clause
                  */
-                function (\Psalm\Internal\Clause $c) use ($mixed_var_ids, $cond_id): \Psalm\Internal\Clause {
-                    $keys = array_keys($c->possibilities);
+            function (\Psalm\Internal\Clause $c) use ($mixed_var_ids, $cond_id): \Psalm\Internal\Clause {
+                $keys = array_keys($c->possibilities);
 
-                    $mixed_var_ids = \array_diff($mixed_var_ids, $keys);
+                $mixed_var_ids = \array_diff($mixed_var_ids, $keys);
 
-                    foreach ($keys as $key) {
-                        foreach ($mixed_var_ids as $mixed_var_id) {
-                            if (preg_match('/^' . preg_quote($mixed_var_id, '/') . '(\[|-)/', $key)) {
-                                return new \Psalm\Internal\Clause([], $cond_id, $cond_id, true);
-                            }
+                foreach ($keys as $key) {
+                    foreach ($mixed_var_ids as $mixed_var_id) {
+                        if (preg_match('/^' . preg_quote($mixed_var_id, '/') . '(\[|-)/', $key)) {
+                            return new \Psalm\Internal\Clause([], $cond_id, $cond_id, true);
                         }
                     }
+                }
 
-                    return $c;
-                },
-                $if_clauses
-            )
+                return $c;
+            },
+            $if_clauses
         );
 
         // this will see whether any of the clauses in set A conflict with the clauses in set B

diff --git a/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php
@@ -7,6 +7,8 @@
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Type;
 
+use function count;
+
 class UnsetAnalyzer
 {
     public static function analyze(
@@ -50,7 +52,11 @@ public static function analyze(
                                 || $var->dim instanceof PhpParser\Node\Scalar\LNumber
                             ) {
                                 if (isset($atomic_root_type->properties[$var->dim->value])) {
-                                    $atomic_root_type->is_list = false;
+                                    if ($atomic_root_type->is_list
+                                        && $var->dim->value !== count($atomic_root_type->properties)-1
+                                    ) {
+                                        $atomic_root_type->is_list = false;
+                                    }
                                     unset($atomic_root_type->properties[$var->dim->value]);
                                     $root_type->bustCache(); //remove id cache
                                 }

diff --git a/src/Psalm/Internal/Fork/Pool.php b/src/Psalm/Internal/Fork/Pool.php
@@ -387,7 +387,7 @@ private function readResultsFromChildren(): array
             }
         }
 
-        return array_values($terminationMessages);
+        return $terminationMessages;
     }
 
     /**

diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php
@@ -831,6 +831,7 @@ public function offsetSet($offset, $value): void
             'keyedIntOffsetArrayValues' => [
                 '<?php
                     $a = ["hello", 5];
+                    /** @psalm-suppress RedundantCast */
                     $a_values = array_values($a);
                     $a_keys = array_keys($a);',
                 'assertions' => [
@@ -2007,6 +2008,17 @@ function baz(array $bar) : void {
                     }',
                 'error_message' => 'RedundantCast',
             ],
+            'arrayValuesOnList' => [
+                '<?php
+                    /**
+                     * @param list<int> $a
+                     * @return list<int>
+                     */
+                    function foo(array $a) : array {
+                        return array_values($a);
+                    }',
+                'error_message' => 'RedundantCast',
+            ],
             'assignToListWithUpdatedForeachKey' => [
                 '<?php
                     /**

diff --git a/tests/ReportOutputTest.php b/tests/ReportOutputTest.php
@@ -13,7 +13,6 @@
 use Psalm\Report\ReportOptions;
 use Psalm\Tests\Internal\Provider;
 
-use function array_values;
 use function file_get_contents;
 use function json_decode;
 use function ob_end_clean;
@@ -815,7 +814,7 @@ public function testJsonReport(): void
         $json_report_options = ProjectAnalyzer::getFileReportOptions([__DIR__ . '/test-report.json'])[0];
 
         $this->assertSame(
-            array_values($issue_data),
+            $issue_data,
             json_decode(IssueBuffer::getOutput(IssueBuffer::getIssuesData(), $json_report_options), true)
         );
     }

diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php
@@ -2287,6 +2287,7 @@ public function __construct(array $elements) {
                          * @return static<U>
                          */
                         public function map(callable $callback) {
+                            /** @psalm-suppress RedundantCast */
                             return new static(array_values(array_map($callback, $this->elements)));
                         }
                     }